D8454: phabricator: ensure that `phabsend` is given a contiguous, linear commit range

mharbison72 (Matt Harbison) phabricator at mercurial-scm.org
Fri Apr 17 00:02:10 UTC 2020


mharbison72 created this revision.
Herald added subscribers: mercurial-devel, Kwan.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  Supplying a non-linear range was another orphan factory.  While in theory there
  could be a use case for skipping over garbage commits (like adding debugging)
  and getting the valuable commits extracted out at the same time as posting a
  review, it seems far more likely that specifying a non-linear range is a user
  error.  This is another case of issue6045, but predates both 0680b8a1992a <https://phab.mercurial-scm.org/rHG0680b8a1992a74360b4f1a20bebf46c6c6962be8> and
  601ce5392cb0 <https://phab.mercurial-scm.org/rHG601ce5392cb092df601bef8c83fc250726f99c3d>.
  
  Neither the `--no-amend` case nor resubmitting a previously submitted commit
  would cause orphans.  But for the sake of simplicity and to keep the parents
  tracked on Phabricator in the proper state, ban missing commits unconditionally.

REPOSITORY
  rHG Mercurial

BRANCH
  stable

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

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

CHANGE DETAILS

diff --git a/tests/test-phabricator.t b/tests/test-phabricator.t
--- a/tests/test-phabricator.t
+++ b/tests/test-phabricator.t
@@ -589,6 +589,12 @@
   applying patch from D7917
   applying patch from D7918
 
+Phabsend requires a linear range of commits
+
+  $ hg phabsend -r 0+3
+  abort: cannot phabsend non-linear revisions
+  [255]
+
 Validate arguments with --fold
 
   $ hg phabsend --fold -r 1
@@ -597,9 +603,6 @@
   $ hg phabsend --fold --no-amend -r 1::
   abort: cannot fold with --no-amend
   [255]
-  $ hg phabsend --fold -r 0+3
-  abort: cannot fold non-linear revisions
-  [255]
   $ hg phabsend --fold -r 1::
   abort: cannot fold revisions with different DREV values
   [255]
diff --git a/hgext/phabricator.py b/hgext/phabricator.py
--- a/hgext/phabricator.py
+++ b/hgext/phabricator.py
@@ -1307,6 +1307,16 @@
     if any(c for c in ctxs if c.obsolete()):
         raise error.Abort(_(b"obsolete commits cannot be posted for review"))
 
+    # Ensure the local commits are an unbroken range.  The semantics of the
+    # --fold option implies this, and the auto restacking of orphans requires
+    # it.  Otherwise A+C in A->B->C will cause B to be orphaned, and C' to
+    # get A' as a parent.
+    revrange = repo.revs(b'(first(%ld)::last(%ld))', revs, revs)
+    if any(r for r in revs if r not in revrange) or any(
+        r for r in revrange if r not in revs
+    ):
+        raise error.Abort(_(b"cannot phabsend non-linear revisions"))
+
     fold = opts.get(b'fold')
     if fold:
         if len(revs) == 1:
@@ -1322,13 +1332,6 @@
         if not opts.get(b"amend"):
             raise error.Abort(_(b"cannot fold with --no-amend"))
 
-        # Ensure the local commits are an unbroken range
-        revrange = repo.revs(b'(first(%ld)::last(%ld))', revs, revs)
-        if any(r for r in revs if r not in revrange) or any(
-            r for r in revrange if r not in revs
-        ):
-            raise error.Abort(_(b"cannot fold non-linear revisions"))
-
         # It might be possible to bucketize the revisions by the DREV value, and
         # iterate over those groups when posting, and then again when amending.
         # But for simplicity, require all selected revisions to be for the same



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


More information about the Mercurial-devel mailing list