[PATCH 1 of 5 V2] config: add option to control creation of empty successors during rewrite

classic Classic list List threaded Threaded
6 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH 1 of 5 V2] config: add option to control creation of empty successors during rewrite

Manuel Jacob
# HG changeset patch
# User Manuel Jacob <[hidden email]>
# Date 1594504407 -7200
#      Sat Jul 11 23:53:27 2020 +0200
# Node ID 8263ac153567482d8794d76a897eb47a0f116e2d
# Parent  52fad88f31b2a3dc16ea8654147bb6ff31482d26
# EXP-Topic keep_empty_successor
config: add option to control creation of empty successors during rewrite

The default for many history-rewriting commands (e.g. rebase and absorb) is
that changesets which would become empty are not created in the target branch.
This makes sense if the source branch consists of small fix-up changes. For
more advanced workflows that make heavy use of history-editing to create
curated patch series, dropping empty changesets is not as important or even
undesirable.

Some users want to keep the meta-history, e.g. to make finding comments in a
code review tool easier or to avoid that divergent bookmarks are created. For
that, obsmarkers from the (to-be) empty changeset to the changeset(s) that
already made the changes should be added. If a to-be empty changeset is pruned
without a successor, adding the obsmarkers is hard because the changeset has to
be found within the hidden part of the history.

If rebasing in TortoiseHg, it’s easy to miss the fact that the to-be empty
changeset was pruned. An empty changeset will function as a reminder that
obsmarkers should be added.

Martin von Zweigbergk mentioned another advantage. Stripping the successor will
de-obsolete the predecessor. If no (empty) successor is created, this won’t be
possible.

In the future, we may want to consider other behaviors, like e.g. creating the
empty successor, but pruning it right away. Therefore this configuration
accepts 'skip' and 'keep' instead of being a boolean configuration.

diff --git a/mercurial/configitems.py b/mercurial/configitems.py
--- a/mercurial/configitems.py
+++ b/mercurial/configitems.py
@@ -1068,6 +1068,9 @@
     b'rewrite', b'update-timestamp', default=False,
 )
 coreconfigitem(
+    b'rewrite', b'empty-successor', default=b'skip', experimental=True,
+)
+coreconfigitem(
     b'storage', b'new-repo-backend', default=b'revlogv1', experimental=True,
 )
 coreconfigitem(
diff --git a/mercurial/helptext/config.txt b/mercurial/helptext/config.txt
--- a/mercurial/helptext/config.txt
+++ b/mercurial/helptext/config.txt
@@ -1890,6 +1890,14 @@
     applicable for `hg amend`, `hg commit --amend` and `hg uncommit` in the
     current version.
 
+``empty-successor``
+
+    Control what happens with empty successors that are the result of rewrite
+    operations. If set to ``skip``, the successor is not created. If set to
+    ``keep``, the empty successor is created and kept.
+
+    Currently, no command considers this configuration. (EXPERIMENTAL)
+
 ``storage``
 -----------
 
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

[PATCH 2 of 5 V2] rewriteutil: add utility to check whether empty successors should be skipped

Manuel Jacob
# HG changeset patch
# User Manuel Jacob <[hidden email]>
# Date 1594526766 -7200
#      Sun Jul 12 06:06:06 2020 +0200
# Node ID 216360d175b1dbbda8498bd8d4341b315e977ad1
# Parent  8263ac153567482d8794d76a897eb47a0f116e2d
# EXP-Topic keep_empty_successor
rewriteutil: add utility to check whether empty successors should be skipped

diff --git a/mercurial/rewriteutil.py b/mercurial/rewriteutil.py
--- a/mercurial/rewriteutil.py
+++ b/mercurial/rewriteutil.py
@@ -53,3 +53,20 @@
     if allowunstable:
         return revset.baseset()
     return repo.revs(b"(%ld::) - %ld", revs, revs)
+
+
+def skip_empty_successor(ui, command):
+    empty_successor = ui.config(b'rewrite', b'empty-successor')
+    if empty_successor == b'skip':
+        return True
+    elif empty_successor == b'keep':
+        return False
+    else:
+        raise error.ConfigError(
+            _(
+                b"%s doesn't know how to handle config "
+                b"rewrite.empty-successor=%s (only 'skip' and 'keep' are "
+                b"supported)"
+            )
+            % (command, empty_successor)
+        )

_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

[PATCH 3 of 5 V2] rebase: consider rewrite.empty-successor configuration

Manuel Jacob
In reply to this post by Manuel Jacob
# HG changeset patch
# User Manuel Jacob <[hidden email]>
# Date 1590993522 -7200
#      Mon Jun 01 08:38:42 2020 +0200
# Node ID 06400a64a1239a4a3bcc64d61e8e6d6a44119628
# Parent  216360d175b1dbbda8498bd8d4341b315e977ad1
# EXP-Topic keep_empty_successor
rebase: consider rewrite.empty-successor configuration

This adds support for the recently added rewrite.empty-successor configuration.

diff --git a/hgext/rebase.py b/hgext/rebase.py
--- a/hgext/rebase.py
+++ b/hgext/rebase.py
@@ -206,6 +206,9 @@
         self.backupf = ui.configbool(b'rewrite', b'backup-bundle')
         self.keepf = opts.get(b'keep', False)
         self.keepbranchesf = opts.get(b'keepbranches', False)
+        self.skipemptysuccessorf = rewriteutil.skip_empty_successor(
+            repo.ui, b'rebase'
+        )
         self.obsoletenotrebased = {}
         self.obsoletewithoutsuccessorindestination = set()
         self.inmemory = inmemory
@@ -530,7 +533,10 @@
         for c in self.extrafns:
             c(ctx, extra)
         destphase = max(ctx.phase(), phases.draft)
-        overrides = {(b'phases', b'new-commit'): destphase}
+        overrides = {
+            (b'phases', b'new-commit'): destphase,
+            (b'ui', b'allowemptycommit'): not self.skipemptysuccessorf,
+        }
         with repo.ui.configoverride(overrides, b'rebase'):
             if self.inmemory:
                 newnode = commitmemorynode(
@@ -650,6 +656,14 @@
             if newnode is not None:
                 self.state[rev] = repo[newnode].rev()
                 ui.debug(b'rebased as %s\n' % short(newnode))
+                if repo[newnode].isempty():
+                    ui.warn(
+                        _(
+                            b'note: created empty successor for %s, its '
+                            b'destination already has all its changes\n'
+                        )
+                        % desc
+                    )
             else:
                 if not self.collapsef:
                     ui.warn(
diff --git a/mercurial/helptext/config.txt b/mercurial/helptext/config.txt
--- a/mercurial/helptext/config.txt
+++ b/mercurial/helptext/config.txt
@@ -1896,7 +1896,8 @@
     operations. If set to ``skip``, the successor is not created. If set to
     ``keep``, the empty successor is created and kept.
 
-    Currently, no command considers this configuration. (EXPERIMENTAL)
+    Currently, only the rebase command considers this configuration.
+    (EXPERIMENTAL)
 
 ``storage``
 -----------
diff --git a/tests/test-rebase-empty-successor.t b/tests/test-rebase-empty-successor.t
new file mode 100644
--- /dev/null
+++ b/tests/test-rebase-empty-successor.t
@@ -0,0 +1,44 @@
+  $ cat << EOF >> $HGRCPATH
+  > [extensions]
+  > rebase=
+  > [alias]
+  > tglog = log -G -T "{rev} '{desc}'\n"
+  > EOF
+
+  $ hg init
+
+  $ echo a > a; hg add a; hg ci -m a
+  $ echo b > b; hg add b; hg ci -m b1
+  $ hg up 0 -q
+  $ echo b > b; hg add b; hg ci -m b2 -q
+
+  $ hg tglog
+  @  2 'b2'
+  |
+  | o  1 'b1'
+  |/
+  o  0 'a'
+  
+
+With rewrite.empty-successor=skip, b2 is skipped because it would become empty.
+
+  $ hg rebase -s 2 -d 1 --config rewrite.empty-successor=skip --dry-run
+  starting dry-run rebase; repository will not be changed
+  rebasing 2:6e2aad5e0f3c "b2" (tip)
+  note: not rebasing 2:6e2aad5e0f3c "b2" (tip), its destination already has all its changes
+  dry-run rebase completed successfully; run without -n/--dry-run to perform this rebase
+
+With rewrite.empty-successor=keep, b2 will be recreated although it became empty.
+
+  $ hg rebase -s 2 -d 1 --config rewrite.empty-successor=keep
+  rebasing 2:6e2aad5e0f3c "b2" (tip)
+  note: created empty successor for 2:6e2aad5e0f3c "b2" (tip), its destination already has all its changes
+  saved backup bundle to $TESTTMP/.hg/strip-backup/6e2aad5e0f3c-7d7c8801-rebase.hg
+
+  $ hg tglog
+  @  2 'b2'
+  |
+  o  1 'b1'
+  |
+  o  0 'a'
+  

_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

[PATCH 4 of 5 V2] absorb: consider rewrite.empty-successor configuration

Manuel Jacob
In reply to this post by Manuel Jacob
# HG changeset patch
# User Manuel Jacob <[hidden email]>
# Date 1590997361 -7200
#      Mon Jun 01 09:42:41 2020 +0200
# Node ID 0d5a6db2d8e19b5c8cb3e78af2b2080b0f0f04e4
# Parent  06400a64a1239a4a3bcc64d61e8e6d6a44119628
# EXP-Topic keep_empty_successor
absorb: consider rewrite.empty-successor configuration

This adds support for the recently added rewrite.empty-successor configuration.

diff --git a/hgext/absorb.py b/hgext/absorb.py
--- a/hgext/absorb.py
+++ b/hgext/absorb.py
@@ -50,6 +50,7 @@
     phases,
     pycompat,
     registrar,
+    rewriteutil,
     scmutil,
     util,
 )
@@ -782,8 +783,10 @@
                 # nothing changed, nothing commited
                 nextp1 = ctx
                 continue
-            if ctx.files() and self._willbecomenoop(
-                memworkingcopy, ctx, nextp1
+            if (
+                self.skip_empty_successor
+                and ctx.files()
+                and self._willbecomenoop(memworkingcopy, ctx, nextp1)
             ):
                 # changeset is no longer necessary
                 self.replacemap[ctx.node()] = None
@@ -935,6 +938,10 @@
                 self.repo, replacements, operation=b'absorb', fixphase=True
             )
 
+    @util.propertycache
+    def skip_empty_successor(self):
+        return rewriteutil.skip_empty_successor(self.ui, b'absorb')
+
 
 def _parsechunk(hunk):
     """(crecord.uihunk or patch.recordhunk) -> (path, (a1, a2, [bline]))"""
diff --git a/mercurial/helptext/config.txt b/mercurial/helptext/config.txt
--- a/mercurial/helptext/config.txt
+++ b/mercurial/helptext/config.txt
@@ -1896,7 +1896,7 @@
     operations. If set to ``skip``, the successor is not created. If set to
     ``keep``, the empty successor is created and kept.
 
-    Currently, only the rebase command considers this configuration.
+    Currently, only the rebase and absorb commands consider this configuration.
     (EXPERIMENTAL)
 
 ``storage``
diff --git a/tests/test-absorb.t b/tests/test-absorb.t
--- a/tests/test-absorb.t
+++ b/tests/test-absorb.t
@@ -490,6 +490,71 @@
      +3
   
 
+Setting config rewrite.empty-successor=keep causes empty changesets to get committed:
+
+  $ cd ..
+  $ hg init repo4a
+  $ cd repo4a
+  $ cat > a <<EOF
+  > 1
+  > 2
+  > EOF
+  $ hg commit -m a12 -A a
+  $ cat > b <<EOF
+  > 1
+  > 2
+  > EOF
+  $ hg commit -m b12 -A b
+  $ echo 3 >> b
+  $ hg commit -m b3
+  $ echo 4 >> b
+  $ hg commit -m b4
+  $ echo 1 > b
+  $ echo 3 >> a
+  $ hg absorb -pn
+  showing changes for a
+          @@ -2,0 +2,1 @@
+  bfafb49 +3
+  showing changes for b
+          @@ -1,3 +1,0 @@
+  1154859 -2
+  30970db -3
+  a393a58 -4
+  
+  4 changesets affected
+  a393a58 b4
+  30970db b3
+  1154859 b12
+  bfafb49 a12
+  $ hg absorb -av --config rewrite.empty-successor=keep | grep became
+  0:bfafb49242db: 1 file(s) changed, became 4:1a2de97fc652
+  1:115485984805: 2 file(s) changed, became 5:0c930dfab74c
+  2:30970dbf7b40: 2 file(s) changed, became 6:df6574ae635c
+  3:a393a58b9a85: 2 file(s) changed, became 7:ad4bd3462c9e
+  $ hg log -T '{rev} {desc}\n' -Gp
+  @  7 b4
+  |
+  o  6 b3
+  |
+  o  5 b12
+  |  diff --git a/b b/b
+  |  new file mode 100644
+  |  --- /dev/null
+  |  +++ b/b
+  |  @@ -0,0 +1,1 @@
+  |  +1
+  |
+  o  4 a12
+     diff --git a/a b/a
+     new file mode 100644
+     --- /dev/null
+     +++ b/a
+     @@ -0,0 +1,3 @@
+     +1
+     +2
+     +3
+  
+
 Use revert to make the current change and its parent disappear.
 This should move us to the non-obsolete ancestor.
 

_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

[PATCH 5 of 5 V2] absorb: make it explicit if empty changeset was created

Manuel Jacob
In reply to this post by Manuel Jacob
# HG changeset patch
# User Manuel Jacob <[hidden email]>
# Date 1590998131 -7200
#      Mon Jun 01 09:55:31 2020 +0200
# Node ID f0d1ae4b67fb74a63d62d8fc639c53ec83481702
# Parent  0d5a6db2d8e19b5c8cb3e78af2b2080b0f0f04e4
# EXP-Topic keep_empty_successor
absorb: make it explicit if empty changeset was created

If the config rewrite.empty-successor=skip is set, a message "became empty and
was dropped" is shown if the changeset became empty. If the config
rewrite.empty-successor=keep is set, absorb may create changesets even if they
became empty. It’s probably a good idea to make that explicit. Therefore the
message is changed to be a combination of both: "became empty and became ...".

Repeating the word "became" is not very elegant. This results from the fact
that "became" was and is overloaded to indicate both the change from non-empty
to empty and the successor relation. In the combinated message, both meanings
are used in one sentence.

diff --git a/hgext/absorb.py b/hgext/absorb.py
--- a/hgext/absorb.py
+++ b/hgext/absorb.py
@@ -783,11 +783,10 @@
                 # nothing changed, nothing commited
                 nextp1 = ctx
                 continue
-            if (
-                self.skip_empty_successor
-                and ctx.files()
-                and self._willbecomenoop(memworkingcopy, ctx, nextp1)
-            ):
+            willbecomenoop = ctx.files() and self._willbecomenoop(
+                memworkingcopy, ctx, nextp1
+            )
+            if self.skip_empty_successor and willbecomenoop:
                 # changeset is no longer necessary
                 self.replacemap[ctx.node()] = None
                 msg = _(b'became empty and was dropped')
@@ -798,7 +797,14 @@
                 nextp1 = lastcommitted
                 self.replacemap[ctx.node()] = lastcommitted.node()
                 if memworkingcopy:
-                    msg = _(b'%d file(s) changed, became %s') % (
+                    if willbecomenoop:
+                        msg = _(
+                            b'%d file(s) changed, became empty '
+                            b'and became %s'
+                        )
+                    else:
+                        msg = _(b'%d file(s) changed, became %s')
+                    msg = msg % (
                         len(memworkingcopy),
                         self._ctx2str(lastcommitted),
                     )
diff --git a/tests/test-absorb.t b/tests/test-absorb.t
--- a/tests/test-absorb.t
+++ b/tests/test-absorb.t
@@ -509,6 +509,7 @@
   $ hg commit -m b3
   $ echo 4 >> b
   $ hg commit -m b4
+  $ hg commit -m empty --config ui.allowemptycommit=True
   $ echo 1 > b
   $ echo 3 >> a
   $ hg absorb -pn
@@ -527,16 +528,19 @@
   1154859 b12
   bfafb49 a12
   $ hg absorb -av --config rewrite.empty-successor=keep | grep became
-  0:bfafb49242db: 1 file(s) changed, became 4:1a2de97fc652
-  1:115485984805: 2 file(s) changed, became 5:0c930dfab74c
-  2:30970dbf7b40: 2 file(s) changed, became 6:df6574ae635c
-  3:a393a58b9a85: 2 file(s) changed, became 7:ad4bd3462c9e
+  0:bfafb49242db: 1 file(s) changed, became 5:1a2de97fc652
+  1:115485984805: 2 file(s) changed, became 6:0c930dfab74c
+  2:30970dbf7b40: 2 file(s) changed, became empty and became 7:df6574ae635c
+  3:a393a58b9a85: 2 file(s) changed, became empty and became 8:ad4bd3462c9e
+  4:1bb0e8cff87a: 2 file(s) changed, became 9:2dbed75af996
   $ hg log -T '{rev} {desc}\n' -Gp
-  @  7 b4
+  @  9 empty
+  |
+  o  8 b4
   |
-  o  6 b3
+  o  7 b3
   |
-  o  5 b12
+  o  6 b12
   |  diff --git a/b b/b
   |  new file mode 100644
   |  --- /dev/null
@@ -544,7 +548,7 @@
   |  @@ -0,0 +1,1 @@
   |  +1
   |
-  o  4 a12
+  o  5 a12
      diff --git a/a b/a
      new file mode 100644
      --- /dev/null
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 5 of 5 V2] absorb: make it explicit if empty changeset was created

Augie Fackler-2
On Sun, Jul 12, 2020 at 07:13:55AM +0200, Manuel Jacob wrote:
> # HG changeset patch
> # User Manuel Jacob <[hidden email]>
> # Date 1590998131 -7200
> #      Mon Jun 01 09:55:31 2020 +0200
> # Node ID f0d1ae4b67fb74a63d62d8fc639c53ec83481702
> # Parent  0d5a6db2d8e19b5c8cb3e78af2b2080b0f0f04e4
> # EXP-Topic keep_empty_successor
> absorb: make it explicit if empty changeset was created

Queued, but consider a follow-up for my suggestion below if you like it.

>
> If the config rewrite.empty-successor=skip is set, a message "became empty and
> was dropped" is shown if the changeset became empty. If the config
> rewrite.empty-successor=keep is set, absorb may create changesets even if they
> became empty. It’s probably a good idea to make that explicit. Therefore the
> message is changed to be a combination of both: "became empty and became ...".
>
> Repeating the word "became" is not very elegant. This results from the fact
> that "became" was and is overloaded to indicate both the change from non-empty
> to empty and the successor relation. In the combinated message, both meanings
> are used in one sentence.
>

[...]

> diff --git a/tests/test-absorb.t b/tests/test-absorb.t
> --- a/tests/test-absorb.t
> +++ b/tests/test-absorb.t
> @@ -509,6 +509,7 @@
>    $ hg commit -m b3
>    $ echo 4 >> b
>    $ hg commit -m b4
> +  $ hg commit -m empty --config ui.allowemptycommit=True
>    $ echo 1 > b
>    $ echo 3 >> a
>    $ hg absorb -pn
> @@ -527,16 +528,19 @@
>    1154859 b12
>    bfafb49 a12
>    $ hg absorb -av --config rewrite.empty-successor=keep | grep became
> -  0:bfafb49242db: 1 file(s) changed, became 4:1a2de97fc652
> -  1:115485984805: 2 file(s) changed, became 5:0c930dfab74c
> -  2:30970dbf7b40: 2 file(s) changed, became 6:df6574ae635c
> -  3:a393a58b9a85: 2 file(s) changed, became 7:ad4bd3462c9e
> +  0:bfafb49242db: 1 file(s) changed, became 5:1a2de97fc652
> +  1:115485984805: 2 file(s) changed, became 6:0c930dfab74c
> +  2:30970dbf7b40: 2 file(s) changed, became empty and became 7:df6574ae635c
> +  3:a393a58b9a85: 2 file(s) changed, became empty and became 8:ad4bd3462c9e

Maybe "N:deadbeef: N file(s) changed, became M:f005ba11 (empty change)" instead?

> +  4:1bb0e8cff87a: 2 file(s) changed, became 9:2dbed75af996
>    $ hg log -T '{rev} {desc}\n' -Gp
> -  @  7 b4
> +  @  9 empty
> +  |
> +  o  8 b4
>    |
> -  o  6 b3
> +  o  7 b3
>    |
> -  o  5 b12
> +  o  6 b12
>    |  diff --git a/b b/b
>    |  new file mode 100644
>    |  --- /dev/null
> @@ -544,7 +548,7 @@
>    |  @@ -0,0 +1,1 @@
>    |  +1
>    |
> -  o  4 a12
> +  o  5 a12
>       diff --git a/a b/a
>       new file mode 100644
>       --- /dev/null
> _______________________________________________
> Mercurial-devel mailing list
> [hidden email]
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel