Patchwork [2,of,2] exchange: check actually missing revs for obsolete / unstable revs (issue6372)

login
register
mail settings
Submitter Manuel Jacob
Date July 17, 2020, 6:27 a.m.
Message ID <91c59e59daae6b4bc0fe.1594967247@tmp>
Download mbox | patch
Permalink /patch/46772/
State New
Headers show

Comments

Manuel Jacob - July 17, 2020, 6:27 a.m.
# HG changeset patch
# User Manuel Jacob <me@manueljacob.de>
# Date 1594966891 -7200
#      Fri Jul 17 08:21:31 2020 +0200
# Node ID 91c59e59daae6b4bc0fe86b09c01fcc2e1aace9d
# Parent  55626b2da7d55766ae667d91652d094c0c4aa86f
# EXP-Topic issue6372
exchange: check actually missing revs for obsolete / unstable revs (issue6372)

The previous code was using `outgoing.missingheads`, which, despite the name,
doesn’t contain the missing heads but all changesets that were passed to the
push via `-r` (or `repo.heads()` by default), i.e. it may contain changesets
that are not heads and / or already on the server.

The actually outgoing changesets are stored in `outgoing.missing`. By checking
all outgoing changesets, we avoid the problem and can show the list of all
obsolete or unstable changesets, which is more helpful for the user.
Augie Fackler - July 20, 2020, 4:19 p.m.
On Fri, Jul 17, 2020 at 08:27:27AM +0200, Manuel Jacob wrote:
> # HG changeset patch
> # User Manuel Jacob <me@manueljacob.de>
> # Date 1594966891 -7200
> #      Fri Jul 17 08:21:31 2020 +0200
> # Node ID 91c59e59daae6b4bc0fe86b09c01fcc2e1aace9d
> # Parent  55626b2da7d55766ae667d91652d094c0c4aa86f
> # EXP-Topic issue6372
> exchange: check actually missing revs for obsolete / unstable revs (issue6372)

These look reasonable, but need rebased.
Manuel Jacob - July 20, 2020, 4:32 p.m.
On 2020-07-20 18:19, Augie Fackler wrote:
> On Fri, Jul 17, 2020 at 08:27:27AM +0200, Manuel Jacob wrote:
>> # HG changeset patch
>> # User Manuel Jacob <me@manueljacob.de>
>> # Date 1594966891 -7200
>> #      Fri Jul 17 08:21:31 2020 +0200
>> # Node ID 91c59e59daae6b4bc0fe86b09c01fcc2e1aace9d
>> # Parent  55626b2da7d55766ae667d91652d094c0c4aa86f
>> # EXP-Topic issue6372
>> exchange: check actually missing revs for obsolete / unstable revs 
>> (issue6372)
> 
> These look reasonable, but need rebased.

I’ve sent a rebased version, which was already committed.

Patch

diff --git a/mercurial/exchange.py b/mercurial/exchange.py
--- a/mercurial/exchange.py
+++ b/mercurial/exchange.py
@@ -905,27 +905,32 @@ 
         # if repo.obsstore == False --> no obsolete
         # then, save the iteration
         if unfi.obsstore:
-            # this message are here for 80 char limit reason
-            mso = _(b"push includes obsolete changeset: %s!")
-            mspd = _(b"push includes phase-divergent changeset: %s!")
-            mscd = _(b"push includes content-divergent changeset: %s!")
-            mst = {
-                b"orphan": _(b"push includes orphan changeset: %s!"),
-                b"phase-divergent": mspd,
-                b"content-divergent": mscd,
-            }
-            # If we are to push if there is at least one
-            # obsolete or unstable changeset in missing, at
-            # least one of the missinghead will be obsolete or
-            # unstable. So checking heads only is ok
-            for node in outgoing.missingheads:
+            obsoletes = []
+            unstables = []
+            for node in outgoing.missing:
                 ctx = unfi[node]
                 if ctx.obsolete():
-                    raise error.Abort(mso % ctx)
+                    obsoletes.append(ctx)
                 elif ctx.isunstable():
-                    # TODO print more than one instability in the abort
-                    # message
-                    raise error.Abort(mst[ctx.instabilities()[0]] % ctx)
+                    unstables.append(ctx)
+            if obsoletes or unstables:
+                msg = b""
+                if obsoletes:
+                    msg += _(b"push includes obsolete changesets:\n")
+                    msg += b"\n".join(b'  %s' % ctx for ctx in obsoletes)
+                if unstables:
+                    if msg:
+                        msg += b"\n"
+                    msg += _(b"push includes unstable changesets:\n")
+                    msg += b"\n".join(
+                        b'  %s (%s)'
+                        % (
+                            ctx,
+                            b", ".join(_(ins) for ins in ctx.instabilities()),
+                        )
+                        for ctx in unstables
+                    )
+                raise error.Abort(msg)
 
         discovery.checkheads(pushop)
     return True
diff --git a/tests/test-obsolete-divergent.t b/tests/test-obsolete-divergent.t
--- a/tests/test-obsolete-divergent.t
+++ b/tests/test-obsolete-divergent.t
@@ -118,7 +118,9 @@ 
   $ hg push ../other
   pushing to ../other
   searching for changes
-  abort: push includes content-divergent changeset: 392fd25390da!
+  abort: push includes unstable changesets:
+    82623d38b9ba (content-divergent)
+    392fd25390da (content-divergent)
   [255]
 
   $ cd ..
diff --git a/tests/test-obsolete.t b/tests/test-obsolete.t
--- a/tests/test-obsolete.t
+++ b/tests/test-obsolete.t
@@ -251,7 +251,8 @@ 
   $ hg push ../tmpa
   pushing to ../tmpa
   searching for changes
-  abort: push includes phase-divergent changeset: 5601fb93a350!
+  abort: push includes unstable changesets:
+    5601fb93a350 (phase-divergent)
   [255]
 
 Fixing "bumped" situation
@@ -616,7 +617,8 @@ 
   $ hg push ../tmpc/ -r 'desc("original_d")'
   pushing to ../tmpc/
   searching for changes
-  abort: push includes obsolete changeset: 94b33453f93b!
+  abort: push includes obsolete changesets:
+    94b33453f93b
   [255]
 
 refuse to push unstable changeset
@@ -624,7 +626,10 @@ 
   $ hg push ../tmpc/
   pushing to ../tmpc/
   searching for changes
-  abort: push includes orphan changeset: cda648ca50f5!
+  abort: push includes obsolete changesets:
+    94b33453f93b
+  push includes unstable changesets:
+    cda648ca50f5 (orphan)
   [255]
 
 with --force it will work anyway
@@ -647,6 +652,26 @@ 
   no changes found
   [1]
 
+pushing should work even if the outgoing changes contain an unrelated changeset
+(neither obsolete nor unstable) (issue6372)
+
+  $ hg up 1 -q
+  $ hg branch new -q
+  $ mkcommit c
+
+  $ hg push ../tmpc/ --new-branch
+  pushing to ../tmpc/
+  searching for changes
+  adding changesets
+  adding manifests
+  adding file changes
+  added 1 changesets with 1 changes to 1 files (+1 heads)
+
+make later tests work unmodified
+
+  $ hg --config extensions.strip= strip tip -q
+  $ hg up 5 -q
+
 Test that extinct changeset are properly detected
 
   $ hg log -r 'extinct()'
@@ -1196,6 +1221,14 @@ 
   phase-divergent: immutable predecessor 245b
   content-divergent: predecessor 245b
 
+  $ hg push  ../tmpf -r 50c51b361e60
+  pushing to ../tmpf
+  searching for changes
+  abort: push includes unstable changesets:
+    50c51b361e60 (orphan, phase-divergent, content-divergent)
+  [255]
+
+
 #if serve
 
   $ hg serve -n test -p $HGPORT -d --pid-file=hg.pid -A access.log -E errors.log