Patchwork D8437: phabricator: prevent posting obsolete commits

login
register
mail settings
Submitter phabricator
Date April 15, 2020, 11:57 p.m.
Message ID <differential-rev-PHID-DREV-pjk7qscngytze5kkwrvi-req@mercurial-scm.org>
Download mbox | patch
Permalink /patch/46141/
State Superseded
Headers show

Comments

phabricator - April 15, 2020, 11:57 p.m.
mharbison72 created this revision.
Herald added subscribers: mercurial-devel, Kwan.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  I don't see why this would be useful in the first place.  But I had a coworker
  submit a single commit that was not a branch head, and the result was to orphan
  its child and keep the original commit visible.  He then did up arrow + Enter,
  and it happily created a new review (since the URL isn't amended into the
  original commit specified on the command line) and a new successor, resulting in
  a local divergence.  I'd like to fix the issue with creating orphans, but this
  is simple enough to prevent on its own.

REPOSITORY
  rHG Mercurial

BRANCH
  default

REVISION DETAIL
  https://phab.mercurial-scm.org/D8437

AFFECTED FILES
  hgext/phabricator.py
  tests/test-phabricator.t

CHANGE DETAILS




To: mharbison72, #hg-reviewers
Cc: Kwan, mercurial-devel
phabricator - April 16, 2020, 1:57 p.m.
Alphare added a comment.
Alphare accepted this revision.


  I think that these kinds of fail-safes are very useful. I might have been bit by this once.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D8437/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D8437

To: mharbison72, #hg-reviewers, Alphare
Cc: Alphare, Kwan, mercurial-devel

Patch

diff --git a/tests/test-phabricator.t b/tests/test-phabricator.t
--- a/tests/test-phabricator.t
+++ b/tests/test-phabricator.t
@@ -229,6 +229,58 @@ 
   o  0   5cbade24e0fa   1970-01-01 00:00 +0000   test
        added
   
+Posting obsolete commits is disallowed
+
+  $ echo "mod3" > file1.txt
+  $ hg ci -m 'modified A'
+  $ echo "mod4" > file1.txt
+  $ hg ci -m 'modified B'
+
+  $ hg up '.^'
+  1 files updated, 0 files merged, 0 files removed, 0 files unresolved
+  $ echo 'obsolete' > file1.txt
+  $ hg amend --config extensions.amend=
+  1 new orphan changesets
+  $ hg log -G
+  @  changeset:   8:8d83edb3cbac
+  |  tag:         tip
+  |  parent:      5:1dff6b051abf
+  |  user:        test
+  |  date:        Thu Jan 01 00:00:00 1970 +0000
+  |  summary:     modified A
+  |
+  | *  changeset:   7:d4ea1b2e3511
+  | |  user:        test
+  | |  date:        Thu Jan 01 00:00:00 1970 +0000
+  | |  instability: orphan
+  | |  summary:     modified B
+  | |
+  | x  changeset:   6:4635d7f0d1ff
+  |/   user:        test
+  |    date:        Thu Jan 01 00:00:00 1970 +0000
+  |    obsolete:    rewritten using amend as 8:8d83edb3cbac
+  |    summary:     modified A
+  |
+  o  changeset:   5:1dff6b051abf
+  |  user:        test
+  |  date:        Thu Jan 01 00:00:00 1970 +0000
+  |  summary:     modified 2
+  |
+  o  changeset:   4:eb3752621d45
+  |  parent:      0:5cbade24e0fa
+  |  user:        test
+  |  date:        Thu Jan 01 00:00:00 1970 +0000
+  |  summary:     modified 1
+  |
+  o  changeset:   0:5cbade24e0fa
+     user:        test
+     date:        Thu Jan 01 00:00:00 1970 +0000
+     summary:     added
+  
+  $ hg phabsend -r 5::
+  abort: obsolete commits cannot be posted for review
+  [255]
+
   $ cd ..
 
 Phabesending a new binary, a modified binary, and a removed binary
diff --git a/hgext/phabricator.py b/hgext/phabricator.py
--- a/hgext/phabricator.py
+++ b/hgext/phabricator.py
@@ -1304,6 +1304,9 @@ 
 
     ctxs = [repo[rev] for rev in revs]
 
+    if any(c for c in ctxs if c.obsolete()):
+        raise error.Abort(_(b"obsolete commits cannot be posted for review"))
+
     fold = opts.get(b'fold')
     if fold:
         if len(revs) == 1: