Patchwork obsolete: add operation metadata to rebase/amend/histedit obsmarkers

login
register
mail settings
Submitter Durham Goode
Date May 9, 2017, 11:34 p.m.
Message ID <22051b0924bcbc5cde6d.1494372888@dev111.prn1.facebook.com>
Download mbox | patch
Permalink /patch/20545/
State Accepted
Headers show

Comments

Durham Goode - May 9, 2017, 11:34 p.m.
# HG changeset patch
# User Durham Goode <durham@fb.com>
# Date 1494372571 25200
#      Tue May 09 16:29:31 2017 -0700
# Node ID 22051b0924bcbc5cde6d25d700dcce6afcaafd98
# Parent  8f1a2b848b52ea7bf3fe2404e3b62924c7aae93f
obsolete: add operation metadata to rebase/amend/histedit obsmarkers

By recording what operation created the obsmarker, we can show very intuitive
messages to the user in various UIs. For instance, log output could have
messages like "Amended as XXX" to show why a commit is old and has an 'x' on it.

     @  ac28e3  durham
    /   First commit
   |
   | o  d4afe7 durham
   | |  Second commit
   | |
   | x  8e9a5d (Amended as ac28e3)  durham
   |/   First commit
   |
via Mercurial-devel - May 9, 2017, 11:55 p.m.
On Tue, May 9, 2017 at 4:34 PM, Durham Goode <durham@fb.com> wrote:
> # HG changeset patch
> # User Durham Goode <durham@fb.com>
> # Date 1494372571 25200
> #      Tue May 09 16:29:31 2017 -0700
> # Node ID 22051b0924bcbc5cde6d25d700dcce6afcaafd98
> # Parent  8f1a2b848b52ea7bf3fe2404e3b62924c7aae93f
> obsolete: add operation metadata to rebase/amend/histedit obsmarkers
>
> By recording what operation created the obsmarker, we can show very intuitive
> messages to the user in various UIs. For instance, log output could have
> messages like "Amended as XXX" to show why a commit is old and has an 'x' on it.

Nice! I had previously assumed that that information was already in
the obsmarker, but then I didn't find it in `hg debugobsolete`. I
didn't realize it would be so easy to add. Seems like an obvious
improvement. So obvious that I'm hesitating to queue it only because
it seems like it should have been proposed before. Any idea why that
hasn't happened?

>
>      @  ac28e3  durham
>     /   First commit
>    |
>    | o  d4afe7 durham
>    | |  Second commit
>    | |
>    | x  8e9a5d (Amended as ac28e3)  durham
>    |/   First commit
>    |
>
> diff --git a/hgext/histedit.py b/hgext/histedit.py
> --- a/hgext/histedit.py
> +++ b/hgext/histedit.py
> @@ -1631,7 +1631,7 @@ def safecleanupnode(ui, repo, name, node
>                               key=repo.changelog.rev)
>          markers = [getmarker(t) for t in sortednodes]
>          if markers:
> -            obsolete.createmarkers(repo, markers)
> +            obsolete.createmarkers(repo, markers, operation='histedit')
>      else:
>          return cleanupnode(ui, repo, name, nodes)
>
> diff --git a/hgext/rebase.py b/hgext/rebase.py
> --- a/hgext/rebase.py
> +++ b/hgext/rebase.py
> @@ -1341,7 +1341,7 @@ def clearrebased(ui, repo, state, skippe
>                      succs = (repo[newrev],)
>                  markers.append((repo[rev], succs))
>          if markers:
> -            obsolete.createmarkers(repo, markers)
> +            obsolete.createmarkers(repo, markers, operation='rebase')
>      else:
>          rebased = [rev for rev in state if state[rev] > nullmerge]
>          if rebased:
> diff --git a/mercurial/cmdutil.py b/mercurial/cmdutil.py
> --- a/mercurial/cmdutil.py
> +++ b/mercurial/cmdutil.py
> @@ -2740,7 +2740,7 @@ def amend(ui, repo, commitfunc, old, ext
>                      if node:
>                          obs.append((ctx, ()))
>
> -                    obsolete.createmarkers(repo, obs)
> +                    obsolete.createmarkers(repo, obs, operation='amend')
>          if not createmarkers and newid != old.node():
>              # Strip the intermediate commit (if there was one) and the amended
>              # commit
> diff --git a/mercurial/obsolete.py b/mercurial/obsolete.py
> --- a/mercurial/obsolete.py
> +++ b/mercurial/obsolete.py
> @@ -1204,7 +1204,8 @@ def _computedivergentset(repo):
>      return divergent
>
>
> -def createmarkers(repo, relations, flag=0, date=None, metadata=None):
> +def createmarkers(repo, relations, flag=0, date=None, metadata=None,
> +                  operation=None):
>      """Add obsolete markers between changesets in a repo
>
>      <relations> must be an iterable of (<old>, (<new>, ...)[,{metadata}])
> @@ -1225,6 +1226,8 @@ def createmarkers(repo, relations, flag=
>          metadata = {}
>      if 'user' not in metadata:
>          metadata['user'] = repo.ui.username()
> +    if operation:
> +        metadata['operation'] = operation
>      tr = repo.transaction('add-obsolescence-marker')
>      try:
>          markerargs = []
> diff --git a/tests/test-histedit-obsolete.t b/tests/test-histedit-obsolete.t
> --- a/tests/test-histedit-obsolete.t
> +++ b/tests/test-histedit-obsolete.t
> @@ -50,10 +50,10 @@ Test that histedit learns about obsolesc
>    o  0:cb9a9f314b8b a
>
>    $ hg debugobsolete
> -  e72d22b19f8ecf4150ab4f91d0973fd9955d3ddf 49d44ab2be1b67a79127568a67c9c99430633b48 0 (*) {'user': 'test'} (glob)
> -  3e30a45cf2f719e96ab3922dfe039cfd047956ce 0 {e72d22b19f8ecf4150ab4f91d0973fd9955d3ddf} (*) {'user': 'test'} (glob)
> -  1b2d564fad96311b45362f17c2aa855150efb35f 46abc7c4d8738e8563e577f7889e1b6db3da4199 0 (*) {'user': 'test'} (glob)
> -  114f4176969ef342759a8a57e6bccefc4234829b 49d44ab2be1b67a79127568a67c9c99430633b48 0 (*) {'user': 'test'} (glob)
> +  e72d22b19f8ecf4150ab4f91d0973fd9955d3ddf 49d44ab2be1b67a79127568a67c9c99430633b48 0 (*) {'operation': 'amend', 'user': 'test'} (glob)
> +  3e30a45cf2f719e96ab3922dfe039cfd047956ce 0 {e72d22b19f8ecf4150ab4f91d0973fd9955d3ddf} (*) {'operation': 'amend', 'user': 'test'} (glob)
> +  1b2d564fad96311b45362f17c2aa855150efb35f 46abc7c4d8738e8563e577f7889e1b6db3da4199 0 (*) {'operation': 'histedit', 'user': 'test'} (glob)
> +  114f4176969ef342759a8a57e6bccefc4234829b 49d44ab2be1b67a79127568a67c9c99430633b48 0 (*) {'operation': 'histedit', 'user': 'test'} (glob)
>
>  With some node gone missing during the edit.
>
> @@ -80,14 +80,14 @@ With some node gone missing during the e
>    o  0:cb9a9f314b8b a
>
>    $ hg debugobsolete
> -  e72d22b19f8ecf4150ab4f91d0973fd9955d3ddf 49d44ab2be1b67a79127568a67c9c99430633b48 0 (*) {'user': 'test'} (glob)
> -  3e30a45cf2f719e96ab3922dfe039cfd047956ce 0 {e72d22b19f8ecf4150ab4f91d0973fd9955d3ddf} (*) {'user': 'test'} (glob)
> -  1b2d564fad96311b45362f17c2aa855150efb35f 46abc7c4d8738e8563e577f7889e1b6db3da4199 0 (*) {'user': 'test'} (glob)
> -  114f4176969ef342759a8a57e6bccefc4234829b 49d44ab2be1b67a79127568a67c9c99430633b48 0 (*) {'user': 'test'} (glob)
> -  76f72745eac0643d16530e56e2f86e36e40631f1 2ca853e48edbd6453a0674dc0fe28a0974c51b9c 0 (*) {'user': 'test'} (glob)
> -  2ca853e48edbd6453a0674dc0fe28a0974c51b9c aba7da93703075eec9fb1dbaf143ff2bc1c49d46 0 (*) {'user': 'test'} (glob)
> -  49d44ab2be1b67a79127568a67c9c99430633b48 273c1f3b86267ed3ec684bb13af1fa4d6ba56e02 0 (*) {'user': 'test'} (glob)
> -  46abc7c4d8738e8563e577f7889e1b6db3da4199 aba7da93703075eec9fb1dbaf143ff2bc1c49d46 0 (*) {'user': 'test'} (glob)
> +  e72d22b19f8ecf4150ab4f91d0973fd9955d3ddf 49d44ab2be1b67a79127568a67c9c99430633b48 0 (*) {'operation': 'amend', 'user': 'test'} (glob)
> +  3e30a45cf2f719e96ab3922dfe039cfd047956ce 0 {e72d22b19f8ecf4150ab4f91d0973fd9955d3ddf} (*) {'operation': 'amend', 'user': 'test'} (glob)
> +  1b2d564fad96311b45362f17c2aa855150efb35f 46abc7c4d8738e8563e577f7889e1b6db3da4199 0 (*) {'operation': 'histedit', 'user': 'test'} (glob)
> +  114f4176969ef342759a8a57e6bccefc4234829b 49d44ab2be1b67a79127568a67c9c99430633b48 0 (*) {'operation': 'histedit', 'user': 'test'} (glob)
> +  76f72745eac0643d16530e56e2f86e36e40631f1 2ca853e48edbd6453a0674dc0fe28a0974c51b9c 0 (*) {'operation': 'amend', 'user': 'test'} (glob)
> +  2ca853e48edbd6453a0674dc0fe28a0974c51b9c aba7da93703075eec9fb1dbaf143ff2bc1c49d46 0 (*) {'operation': 'amend', 'user': 'test'} (glob)
> +  49d44ab2be1b67a79127568a67c9c99430633b48 273c1f3b86267ed3ec684bb13af1fa4d6ba56e02 0 (*) {'operation': 'histedit', 'user': 'test'} (glob)
> +  46abc7c4d8738e8563e577f7889e1b6db3da4199 aba7da93703075eec9fb1dbaf143ff2bc1c49d46 0 (*) {'operation': 'histedit', 'user': 'test'} (glob)
>    $ cd ..
>
>  Base setup for the rest of the testing
> @@ -170,13 +170,13 @@ Base setup for the rest of the testing
>    o  0:cb9a9f314b8b a
>
>    $ hg debugobsolete
> -  96e494a2d553dd05902ba1cee1d94d4cb7b8faed 0 {b346ab9a313db8537ecf96fca3ca3ca984ef3bd7} (*) {'user': 'test'} (glob)
> -  b558abc46d09c30f57ac31e85a8a3d64d2e906e4 0 {96e494a2d553dd05902ba1cee1d94d4cb7b8faed} (*) {'user': 'test'} (glob)
> -  d2ae7f538514cd87c17547b0de4cea71fe1af9fb 0 {cb9a9f314b8b07ba71012fcdbc544b5a4d82ff5b} (*) {'user': 'test'} (glob)
> -  177f92b773850b59254aa5e923436f921b55483b b346ab9a313db8537ecf96fca3ca3ca984ef3bd7 0 (*) {'user': 'test'} (glob)
> -  055a42cdd88768532f9cf79daa407fc8d138de9b 59d9f330561fd6c88b1a6b32f0e45034d88db784 0 (*) {'user': 'test'} (glob)
> -  e860deea161a2f77de56603b340ebbb4536308ae 59d9f330561fd6c88b1a6b32f0e45034d88db784 0 (*) {'user': 'test'} (glob)
> -  652413bf663ef2a641cab26574e46d5f5a64a55a cacdfd884a9321ec4e1de275ef3949fa953a1f83 0 (*) {'user': 'test'} (glob)
> +  96e494a2d553dd05902ba1cee1d94d4cb7b8faed 0 {b346ab9a313db8537ecf96fca3ca3ca984ef3bd7} (*) {'operation': 'histedit', 'user': 'test'} (glob)
> +  b558abc46d09c30f57ac31e85a8a3d64d2e906e4 0 {96e494a2d553dd05902ba1cee1d94d4cb7b8faed} (*) {'operation': 'histedit', 'user': 'test'} (glob)
> +  d2ae7f538514cd87c17547b0de4cea71fe1af9fb 0 {cb9a9f314b8b07ba71012fcdbc544b5a4d82ff5b} (*) {'operation': 'histedit', 'user': 'test'} (glob)
> +  177f92b773850b59254aa5e923436f921b55483b b346ab9a313db8537ecf96fca3ca3ca984ef3bd7 0 (*) {'operation': 'histedit', 'user': 'test'} (glob)
> +  055a42cdd88768532f9cf79daa407fc8d138de9b 59d9f330561fd6c88b1a6b32f0e45034d88db784 0 (*) {'operation': 'histedit', 'user': 'test'} (glob)
> +  e860deea161a2f77de56603b340ebbb4536308ae 59d9f330561fd6c88b1a6b32f0e45034d88db784 0 (*) {'operation': 'histedit', 'user': 'test'} (glob)
> +  652413bf663ef2a641cab26574e46d5f5a64a55a cacdfd884a9321ec4e1de275ef3949fa953a1f83 0 (*) {'operation': 'histedit', 'user': 'test'} (glob)
>
>
>  Ensure hidden revision does not prevent histedit
> diff --git a/tests/test-obsolete.t b/tests/test-obsolete.t
> --- a/tests/test-obsolete.t
> +++ b/tests/test-obsolete.t
> @@ -1247,15 +1247,15 @@ only a subset of those are displayed (be
>    adding d
>    $ hg ci --amend -m dd
>    $ hg debugobsolete --index --rev "3+7"
> -  1 6fdef60fcbabbd3d50e9b9cbc2a240724b91a5e1 d27fb9b066076fd921277a4b9e8b9cb48c95bc6a 0 \(.*\) {'user': 'test'} (re)
> -  3 4715cf767440ed891755448016c2b8cf70760c30 7ae79c5d60f049c7b0dd02f5f25b9d60aaf7b36d 0 \(.*\) {'user': 'test'} (re)
> +  1 6fdef60fcbabbd3d50e9b9cbc2a240724b91a5e1 d27fb9b066076fd921277a4b9e8b9cb48c95bc6a 0 \(.*\) {'operation': 'amend', 'user': 'test'} (re)
> +  3 4715cf767440ed891755448016c2b8cf70760c30 7ae79c5d60f049c7b0dd02f5f25b9d60aaf7b36d 0 \(.*\) {'operation': 'amend', 'user': 'test'} (re)
>    $ hg debugobsolete --index --rev "3+7" -Tjson
>    [
>     {
>      "date": *, (glob)
>      "flag": 0,
>      "index": 1,
> -    "metadata": {"user": "test"},
> +    "metadata": {"operation": "amend", "user": "test"},
>      "precnode": "6fdef60fcbabbd3d50e9b9cbc2a240724b91a5e1",
>      "succnodes": ["d27fb9b066076fd921277a4b9e8b9cb48c95bc6a"]
>     },
> @@ -1263,7 +1263,7 @@ only a subset of those are displayed (be
>      "date": *, (glob)
>      "flag": 0,
>      "index": 3,
> -    "metadata": {"user": "test"},
> +    "metadata": {"operation": "amend", "user": "test"},
>      "precnode": "4715cf767440ed891755448016c2b8cf70760c30",
>      "succnodes": ["7ae79c5d60f049c7b0dd02f5f25b9d60aaf7b36d"]
>     }
> @@ -1271,14 +1271,14 @@ only a subset of those are displayed (be
>
>  Test the --delete option of debugobsolete command
>    $ hg debugobsolete --index
> -  0 cb9a9f314b8b07ba71012fcdbc544b5a4d82ff5b f9bd49731b0b175e42992a3c8fa6c678b2bc11f1 0 \(.*\) {'user': 'test'} (re)
> -  1 6fdef60fcbabbd3d50e9b9cbc2a240724b91a5e1 d27fb9b066076fd921277a4b9e8b9cb48c95bc6a 0 \(.*\) {'user': 'test'} (re)
> -  2 1ab51af8f9b41ef8c7f6f3312d4706d870b1fb74 29346082e4a9e27042b62d2da0e2de211c027621 0 \(.*\) {'user': 'test'} (re)
> -  3 4715cf767440ed891755448016c2b8cf70760c30 7ae79c5d60f049c7b0dd02f5f25b9d60aaf7b36d 0 \(.*\) {'user': 'test'} (re)
> +  0 cb9a9f314b8b07ba71012fcdbc544b5a4d82ff5b f9bd49731b0b175e42992a3c8fa6c678b2bc11f1 0 \(.*\) {'operation': 'amend', 'user': 'test'} (re)
> +  1 6fdef60fcbabbd3d50e9b9cbc2a240724b91a5e1 d27fb9b066076fd921277a4b9e8b9cb48c95bc6a 0 \(.*\) {'operation': 'amend', 'user': 'test'} (re)
> +  2 1ab51af8f9b41ef8c7f6f3312d4706d870b1fb74 29346082e4a9e27042b62d2da0e2de211c027621 0 \(.*\) {'operation': 'amend', 'user': 'test'} (re)
> +  3 4715cf767440ed891755448016c2b8cf70760c30 7ae79c5d60f049c7b0dd02f5f25b9d60aaf7b36d 0 \(.*\) {'operation': 'amend', 'user': 'test'} (re)
>    $ hg debugobsolete --delete 1 --delete 3
>    deleted 2 obsolescence markers
>    $ hg debugobsolete
> -  cb9a9f314b8b07ba71012fcdbc544b5a4d82ff5b f9bd49731b0b175e42992a3c8fa6c678b2bc11f1 0 \(.*\) {'user': 'test'} (re)
> -  1ab51af8f9b41ef8c7f6f3312d4706d870b1fb74 29346082e4a9e27042b62d2da0e2de211c027621 0 \(.*\) {'user': 'test'} (re)
> +  cb9a9f314b8b07ba71012fcdbc544b5a4d82ff5b f9bd49731b0b175e42992a3c8fa6c678b2bc11f1 0 \(.*\) {'operation': 'amend', 'user': 'test'} (re)
> +  1ab51af8f9b41ef8c7f6f3312d4706d870b1fb74 29346082e4a9e27042b62d2da0e2de211c027621 0 \(.*\) {'operation': 'amend', 'user': 'test'} (re)
>    $ cd ..
>
> diff --git a/tests/test-rebase-obsolete.t b/tests/test-rebase-obsolete.t
> --- a/tests/test-rebase-obsolete.t
> +++ b/tests/test-rebase-obsolete.t
> @@ -100,9 +100,9 @@ simple rebase
>    o  0:cd010b8cd998 A
>
>    $ hg debugobsolete
> -  42ccdea3bb16d28e1848c95fe2e44c000f3f21b1 e4e5be0395b2cbd471ed22a26b1b6a1a0658a794 0 (*) {'user': 'test'} (glob)
> -  5fddd98957c8a54a4d436dfe1da9d87f21a1b97b 2327fea05063f39961b14cb69435a9898dc9a245 0 (*) {'user': 'test'} (glob)
> -  32af7686d403cf45b5d95f2d70cebea587ac806a 8eeb3c33ad33d452c89e5dcf611c347f978fb42b 0 (*) {'user': 'test'} (glob)
> +  42ccdea3bb16d28e1848c95fe2e44c000f3f21b1 e4e5be0395b2cbd471ed22a26b1b6a1a0658a794 0 (*) {'operation': 'rebase', 'user': 'test'} (glob)
> +  5fddd98957c8a54a4d436dfe1da9d87f21a1b97b 2327fea05063f39961b14cb69435a9898dc9a245 0 (*) {'operation': 'rebase', 'user': 'test'} (glob)
> +  32af7686d403cf45b5d95f2d70cebea587ac806a 8eeb3c33ad33d452c89e5dcf611c347f978fb42b 0 (*) {'operation': 'rebase', 'user': 'test'} (glob)
>
>
>    $ cd ..
> @@ -170,9 +170,9 @@ set.
>    o  0:cd010b8cd998 A
>
>    $ hg debugobsolete
> -  42ccdea3bb16d28e1848c95fe2e44c000f3f21b1 0 {cd010b8cd998f3981a5a8115f94f8da4ab506089} (*) {'user': 'test'} (glob)
> -  5fddd98957c8a54a4d436dfe1da9d87f21a1b97b 5ae4c968c6aca831df823664e706c9d4aa34473d 0 (*) {'user': 'test'} (glob)
> -  32af7686d403cf45b5d95f2d70cebea587ac806a 0 {5fddd98957c8a54a4d436dfe1da9d87f21a1b97b} (*) {'user': 'test'} (glob)
> +  42ccdea3bb16d28e1848c95fe2e44c000f3f21b1 0 {cd010b8cd998f3981a5a8115f94f8da4ab506089} (*) {'operation': 'rebase', 'user': 'test'} (glob)
> +  5fddd98957c8a54a4d436dfe1da9d87f21a1b97b 5ae4c968c6aca831df823664e706c9d4aa34473d 0 (*) {'operation': 'rebase', 'user': 'test'} (glob)
> +  32af7686d403cf45b5d95f2d70cebea587ac806a 0 {5fddd98957c8a54a4d436dfe1da9d87f21a1b97b} (*) {'operation': 'rebase', 'user': 'test'} (glob)
>
>
>  More complex case where part of the rebase set were already rebased
> @@ -180,10 +180,10 @@ More complex case where part of the reba
>    $ hg rebase --rev 'desc(D)' --dest 'desc(H)'
>    rebasing 9:08483444fef9 "D"
>    $ hg debugobsolete
> -  42ccdea3bb16d28e1848c95fe2e44c000f3f21b1 0 {cd010b8cd998f3981a5a8115f94f8da4ab506089} (*) {'user': 'test'} (glob)
> -  5fddd98957c8a54a4d436dfe1da9d87f21a1b97b 5ae4c968c6aca831df823664e706c9d4aa34473d 0 (*) {'user': 'test'} (glob)
> -  32af7686d403cf45b5d95f2d70cebea587ac806a 0 {5fddd98957c8a54a4d436dfe1da9d87f21a1b97b} (*) {'user': 'test'} (glob)
> -  08483444fef91d6224f6655ee586a65d263ad34c 4596109a6a4328c398bde3a4a3b6737cfade3003 0 (*) {'user': 'test'} (glob)
> +  42ccdea3bb16d28e1848c95fe2e44c000f3f21b1 0 {cd010b8cd998f3981a5a8115f94f8da4ab506089} (*) {'operation': 'rebase', 'user': 'test'} (glob)
> +  5fddd98957c8a54a4d436dfe1da9d87f21a1b97b 5ae4c968c6aca831df823664e706c9d4aa34473d 0 (*) {'operation': 'rebase', 'user': 'test'} (glob)
> +  32af7686d403cf45b5d95f2d70cebea587ac806a 0 {5fddd98957c8a54a4d436dfe1da9d87f21a1b97b} (*) {'operation': 'rebase', 'user': 'test'} (glob)
> +  08483444fef91d6224f6655ee586a65d263ad34c 4596109a6a4328c398bde3a4a3b6737cfade3003 0 (*) {'operation': 'rebase', 'user': 'test'} (glob)
>    $ hg log -G
>    @  11:4596109a6a43 D
>    |
> @@ -208,12 +208,12 @@ More complex case where part of the reba
>    note: not rebasing 9:08483444fef9 "D", already in destination as 11:4596109a6a43 "D"
>    rebasing 10:5ae4c968c6ac "C"
>    $ hg debugobsolete
> -  42ccdea3bb16d28e1848c95fe2e44c000f3f21b1 0 {cd010b8cd998f3981a5a8115f94f8da4ab506089} (*) {'user': 'test'} (glob)
> -  5fddd98957c8a54a4d436dfe1da9d87f21a1b97b 5ae4c968c6aca831df823664e706c9d4aa34473d 0 (*) {'user': 'test'} (glob)
> -  32af7686d403cf45b5d95f2d70cebea587ac806a 0 {5fddd98957c8a54a4d436dfe1da9d87f21a1b97b} (*) {'user': 'test'} (glob)
> -  08483444fef91d6224f6655ee586a65d263ad34c 4596109a6a4328c398bde3a4a3b6737cfade3003 0 (*) {'user': 'test'} (glob)
> -  8877864f1edb05d0e07dc4ba77b67a80a7b86672 462a34d07e599b87ea08676a449373fe4e2e1347 0 (*) {'user': 'test'} (glob)
> -  5ae4c968c6aca831df823664e706c9d4aa34473d 98f6af4ee9539e14da4465128f894c274900b6e5 0 (*) {'user': 'test'} (glob)
> +  42ccdea3bb16d28e1848c95fe2e44c000f3f21b1 0 {cd010b8cd998f3981a5a8115f94f8da4ab506089} (*) {'operation': 'rebase', 'user': 'test'} (glob)
> +  5fddd98957c8a54a4d436dfe1da9d87f21a1b97b 5ae4c968c6aca831df823664e706c9d4aa34473d 0 (*) {'operation': 'rebase', 'user': 'test'} (glob)
> +  32af7686d403cf45b5d95f2d70cebea587ac806a 0 {5fddd98957c8a54a4d436dfe1da9d87f21a1b97b} (*) {'operation': 'rebase', 'user': 'test'} (glob)
> +  08483444fef91d6224f6655ee586a65d263ad34c 4596109a6a4328c398bde3a4a3b6737cfade3003 0 (*) {'operation': 'rebase', 'user': 'test'} (glob)
> +  8877864f1edb05d0e07dc4ba77b67a80a7b86672 462a34d07e599b87ea08676a449373fe4e2e1347 0 (*) {'operation': 'rebase', 'user': 'test'} (glob)
> +  5ae4c968c6aca831df823664e706c9d4aa34473d 98f6af4ee9539e14da4465128f894c274900b6e5 0 (*) {'operation': 'rebase', 'user': 'test'} (glob)
>    $ hg log --rev 'divergent()'
>    $ hg log -G
>    o  13:98f6af4ee953 C
> @@ -349,9 +349,9 @@ collapse rebase
>    $ hg id --debug -r tip
>    4dc2197e807bae9817f09905b50ab288be2dbbcf tip
>    $ hg debugobsolete
> -  42ccdea3bb16d28e1848c95fe2e44c000f3f21b1 4dc2197e807bae9817f09905b50ab288be2dbbcf 0 (*) {'user': 'test'} (glob)
> -  5fddd98957c8a54a4d436dfe1da9d87f21a1b97b 4dc2197e807bae9817f09905b50ab288be2dbbcf 0 (*) {'user': 'test'} (glob)
> -  32af7686d403cf45b5d95f2d70cebea587ac806a 4dc2197e807bae9817f09905b50ab288be2dbbcf 0 (*) {'user': 'test'} (glob)
> +  42ccdea3bb16d28e1848c95fe2e44c000f3f21b1 4dc2197e807bae9817f09905b50ab288be2dbbcf 0 (*) {'operation': 'rebase', 'user': 'test'} (glob)
> +  5fddd98957c8a54a4d436dfe1da9d87f21a1b97b 4dc2197e807bae9817f09905b50ab288be2dbbcf 0 (*) {'operation': 'rebase', 'user': 'test'} (glob)
> +  32af7686d403cf45b5d95f2d70cebea587ac806a 4dc2197e807bae9817f09905b50ab288be2dbbcf 0 (*) {'operation': 'rebase', 'user': 'test'} (glob)
>
>    $ cd ..
>
> @@ -411,9 +411,9 @@ not be rebased.
>    o  0:cd010b8cd998 A
>
>    $ hg debugobsolete
> -  5fddd98957c8a54a4d436dfe1da9d87f21a1b97b e273c5e7d2d29df783dce9f9eaa3ac4adc69c15d 0 (*) {'user': 'test'} (glob)
> -  32af7686d403cf45b5d95f2d70cebea587ac806a cf44d2f5a9f4297a62be94cbdd3dff7c7dc54258 0 (*) {'user': 'test'} (glob)
> -  42ccdea3bb16d28e1848c95fe2e44c000f3f21b1 7c6027df6a99d93f461868e5433f63bde20b6dfb 0 (*) {'user': 'test'} (glob)
> +  5fddd98957c8a54a4d436dfe1da9d87f21a1b97b e273c5e7d2d29df783dce9f9eaa3ac4adc69c15d 0 (*) {'operation': 'rebase', 'user': 'test'} (glob)
> +  32af7686d403cf45b5d95f2d70cebea587ac806a cf44d2f5a9f4297a62be94cbdd3dff7c7dc54258 0 (*) {'operation': 'rebase', 'user': 'test'} (glob)
> +  42ccdea3bb16d28e1848c95fe2e44c000f3f21b1 7c6027df6a99d93f461868e5433f63bde20b6dfb 0 (*) {'operation': 'rebase', 'user': 'test'} (glob)
>
>  Test that rewriting leaving instability behind is allowed
>  ---------------------------------------------------------------------
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Durham Goode - May 10, 2017, 12:07 a.m.
On 5/9/17 4:55 PM, Martin von Zweigbergk wrote:
> On Tue, May 9, 2017 at 4:34 PM, Durham Goode <durham@fb.com> wrote:
>> # HG changeset patch
>> # User Durham Goode <durham@fb.com>
>> # Date 1494372571 25200
>> #      Tue May 09 16:29:31 2017 -0700
>> # Node ID 22051b0924bcbc5cde6d25d700dcce6afcaafd98
>> # Parent  8f1a2b848b52ea7bf3fe2404e3b62924c7aae93f
>> obsolete: add operation metadata to rebase/amend/histedit obsmarkers
>>
>> By recording what operation created the obsmarker, we can show very intuitive
>> messages to the user in various UIs. For instance, log output could have
>> messages like "Amended as XXX" to show why a commit is old and has an 'x' on it.
>
> Nice! I had previously assumed that that information was already in
> the obsmarker, but then I didn't find it in `hg debugobsolete`. I
> didn't realize it would be so easy to add. Seems like an obvious
> improvement. So obvious that I'm hesitating to queue it only because
> it seems like it should have been proposed before. Any idea why that
> hasn't happened?

It increases the size of the obsstore a bit, since I think the metadata 
key is serialized every time. If we wanted to save a few bytes, we could 
use something like 'o' as the key, since this would be a common entry. 
But we don't do that for 'user' and it's a common entry, so... *shrug*

This topic came up once before and Pierre-Yves had comments, so he may 
have comments this time.
Jun Wu - May 10, 2017, 12:18 a.m.
Excerpts from Durham Goode's message of 2017-05-09 17:07:33 -0700:
> On 5/9/17 4:55 PM, Martin von Zweigbergk wrote:
> > On Tue, May 9, 2017 at 4:34 PM, Durham Goode <durham@fb.com> wrote:
> >> # HG changeset patch
> >> # User Durham Goode <durham@fb.com>
> >> # Date 1494372571 25200
> >> #      Tue May 09 16:29:31 2017 -0700
> >> # Node ID 22051b0924bcbc5cde6d25d700dcce6afcaafd98
> >> # Parent  8f1a2b848b52ea7bf3fe2404e3b62924c7aae93f
> >> obsolete: add operation metadata to rebase/amend/histedit obsmarkers
> >>
> >> By recording what operation created the obsmarker, we can show very intuitive
> >> messages to the user in various UIs. For instance, log output could have
> >> messages like "Amended as XXX" to show why a commit is old and has an 'x' on it.
> >
> > Nice! I had previously assumed that that information was already in
> > the obsmarker, but then I didn't find it in `hg debugobsolete`. I
> > didn't realize it would be so easy to add. Seems like an obvious
> > improvement. So obvious that I'm hesitating to queue it only because
> > it seems like it should have been proposed before. Any idea why that
> > hasn't happened?
> 
> It increases the size of the obsstore a bit, since I think the metadata 
> key is serialized every time. If we wanted to save a few bytes, we could 
> use something like 'o' as the key, since this would be a common entry. 
> But we don't do that for 'user' and it's a common entry, so... *shrug*

Another idea is to dedup all strings in obsstore. Existing metadata is also
highly repetitive.

> This topic came up once before and Pierre-Yves had comments, so he may 
> have comments this time.
Pierre-Yves David - May 10, 2017, 10:40 a.m.
On 05/10/2017 01:34 AM, Durham Goode wrote:
> # HG changeset patch
> # User Durham Goode <durham@fb.com>
> # Date 1494372571 25200
> #      Tue May 09 16:29:31 2017 -0700
> # Node ID 22051b0924bcbc5cde6d25d700dcce6afcaafd98
> # Parent  8f1a2b848b52ea7bf3fe2404e3b62924c7aae93f
> obsolete: add operation metadata to rebase/amend/histedit obsmarkers

TL;DR; Storing more information is useful, but the pre-existing plans 
aims at storing slightly different data in a different way. We should 
stick to it.

The idea is to use the obsmarker bitfields to store the data and record 
a pre-defined set of "effects" in each markers. (eg: "content change, 
message change, parent change")

For examples:

1) a `hg amend` that update the diff will create a markers recording the 
"content changes".

2) a `hg amend -e` that only update he commit message will record the 
"message change".

3) a `hg rebase` will record the "parent change".

4) a `hg amend` that update the message, the content and user will 
record these three things (content, message and meta changes).

These data can then be used to have nice template as shown by Durham 
(eg: is X is a successors of Y and the markers as some "content change" 
bit, we can display "amended as Y"; if their are some parent change, we 
can display "rebased as Y", if both are present "amended and rebased as Y").


The above approach as multiple advantages:

   Compact: Since we use the (existing) bitfield, the storage cost is 
free (or minimal if we need to adds an extra byte of bitfield). 
Accessing the data is also very cheap:

   Compoundable: We'll often needs to combine multiple markers to 
reports changesets between two revisions (eg: we do not have the 
intermediate revisions locally). the bit field approach makes it trivial 
to compound the information. We can display display the same final 
information when the same result if obtained from different path (eg: 
from two markers [content change; message change] or from one markers 
[content change | message change]).
   This part is important because simply using operation name makes it 
quite hard to combine the information.

   UI Abstraction: since we record the "effect" information, we abstract 
the command names. This makes use more agile about the actual UI. User 
can use their own "hg myownrewritetool" command and still record 
information useful for the other users.

I've updated the wiki[1] with the above and and updated the existing 
entry in the Roadmap to point to it.

[1] 
https://www.mercurial-scm.org/wiki/ChangesetEvolutionDevel#Record_types_of_operation

Beside the details of what data we store and how, there are useful 
intermediate steps we can take here. The "amended as 134506abcdef" 
message is nice but we won't always have the data to precisely determine 
the web here (eg: old client not collecting data, old markers, etc). So 
starting with a neutral "rewritten as 134506abcdef" would be useful. We 
could get that first, independently of the data collection.

Such change will belong well to the evolve extension first. There are 
multiple test case that can be reused, we can ship it to more user to 
get feedback sooner and we'll likely adjust it regularly at the 
beginning. So please submit it there. Good starting point would be the 
'test-evolve-obshistory.t' file and the 'hg debugobshistory' command.

> By recording what operation created the obsmarker, we can show very intuitive
> messages to the user in various UIs. For instance, log output could have
> messages like "Amended asXXX" to show why a commit is old and has an 'x' on it.
>
>      @  ac28e3  durham
>     /   First commit
>    |
>    | o  d4afe7 durham
>    | |  Second commit
>    | |
>    | x  8e9a5d (Amended as ac28e3)  durham
>    |/   First commit
>    |
>
> diff --git a/hgext/histedit.py b/hgext/histedit.py
> --- a/hgext/histedit.py
> +++ b/hgext/histedit.py
> @@ -1631,7 +1631,7 @@ def safecleanupnode(ui, repo, name, node
>                               key=repo.changelog.rev)
>          markers = [getmarker(t) for t in sortednodes]
>          if markers:
> -            obsolete.createmarkers(repo, markers)
> +            obsolete.createmarkers(repo, markers, operation='histedit')
>      else:
>          return cleanupnode(ui, repo, name, nodes)
>
> diff --git a/hgext/rebase.py b/hgext/rebase.py
> --- a/hgext/rebase.py
> +++ b/hgext/rebase.py
> @@ -1341,7 +1341,7 @@ def clearrebased(ui, repo, state, skippe
>                      succs = (repo[newrev],)
>                  markers.append((repo[rev], succs))
>          if markers:
> -            obsolete.createmarkers(repo, markers)
> +            obsolete.createmarkers(repo, markers, operation='rebase')
>      else:
>          rebased = [rev for rev in state if state[rev] > nullmerge]
>          if rebased:
> diff --git a/mercurial/cmdutil.py b/mercurial/cmdutil.py
> --- a/mercurial/cmdutil.py
> +++ b/mercurial/cmdutil.py
> @@ -2740,7 +2740,7 @@ def amend(ui, repo, commitfunc, old, ext
>                      if node:
>                          obs.append((ctx, ()))
>
> -                    obsolete.createmarkers(repo, obs)
> +                    obsolete.createmarkers(repo, obs, operation='amend')
>          if not createmarkers and newid != old.node():
>              # Strip the intermediate commit (if there was one) and the amended
>              # commit
> diff --git a/mercurial/obsolete.py b/mercurial/obsolete.py
> --- a/mercurial/obsolete.py
> +++ b/mercurial/obsolete.py
> @@ -1204,7 +1204,8 @@ def _computedivergentset(repo):
>      return divergent
>
>
> -def createmarkers(repo, relations, flag=0, date=None, metadata=None):
> +def createmarkers(repo, relations, flag=0, date=None, metadata=None,
> +                  operation=None):
>      """Add obsolete markers between changesets in a repo
>
>      <relations> must be an iterable of (<old>, (<new>, ...)[,{metadata}])
> @@ -1225,6 +1226,8 @@ def createmarkers(repo, relations, flag=
>          metadata = {}
>      if 'user' not in metadata:
>          metadata['user'] = repo.ui.username()
> +    if operation:
> +        metadata['operation'] = operation
>      tr = repo.transaction('add-obsolescence-marker')
>      try:
>          markerargs = []
> diff --git a/tests/test-histedit-obsolete.t b/tests/test-histedit-obsolete.t
> --- a/tests/test-histedit-obsolete.t
> +++ b/tests/test-histedit-obsolete.t
> @@ -50,10 +50,10 @@ Test that histedit learns about obsolesc
>    o  0:cb9a9f314b8b a
>
>    $ hg debugobsolete
> -  e72d22b19f8ecf4150ab4f91d0973fd9955d3ddf 49d44ab2be1b67a79127568a67c9c99430633b48 0 (*) {'user': 'test'} (glob)
> -  3e30a45cf2f719e96ab3922dfe039cfd047956ce 0 {e72d22b19f8ecf4150ab4f91d0973fd9955d3ddf} (*) {'user': 'test'} (glob)
> -  1b2d564fad96311b45362f17c2aa855150efb35f 46abc7c4d8738e8563e577f7889e1b6db3da4199 0 (*) {'user': 'test'} (glob)
> -  114f4176969ef342759a8a57e6bccefc4234829b 49d44ab2be1b67a79127568a67c9c99430633b48 0 (*) {'user': 'test'} (glob)
> +  e72d22b19f8ecf4150ab4f91d0973fd9955d3ddf 49d44ab2be1b67a79127568a67c9c99430633b48 0 (*) {'operation': 'amend', 'user': 'test'} (glob)
> +  3e30a45cf2f719e96ab3922dfe039cfd047956ce 0 {e72d22b19f8ecf4150ab4f91d0973fd9955d3ddf} (*) {'operation': 'amend', 'user': 'test'} (glob)
> +  1b2d564fad96311b45362f17c2aa855150efb35f 46abc7c4d8738e8563e577f7889e1b6db3da4199 0 (*) {'operation': 'histedit', 'user': 'test'} (glob)
> +  114f4176969ef342759a8a57e6bccefc4234829b 49d44ab2be1b67a79127568a67c9c99430633b48 0 (*) {'operation': 'histedit', 'user': 'test'} (glob)
>
>  With some node gone missing during the edit.
>
> @@ -80,14 +80,14 @@ With some node gone missing during the e
>    o  0:cb9a9f314b8b a
>
>    $ hg debugobsolete
> -  e72d22b19f8ecf4150ab4f91d0973fd9955d3ddf 49d44ab2be1b67a79127568a67c9c99430633b48 0 (*) {'user': 'test'} (glob)
> -  3e30a45cf2f719e96ab3922dfe039cfd047956ce 0 {e72d22b19f8ecf4150ab4f91d0973fd9955d3ddf} (*) {'user': 'test'} (glob)
> -  1b2d564fad96311b45362f17c2aa855150efb35f 46abc7c4d8738e8563e577f7889e1b6db3da4199 0 (*) {'user': 'test'} (glob)
> -  114f4176969ef342759a8a57e6bccefc4234829b 49d44ab2be1b67a79127568a67c9c99430633b48 0 (*) {'user': 'test'} (glob)
> -  76f72745eac0643d16530e56e2f86e36e40631f1 2ca853e48edbd6453a0674dc0fe28a0974c51b9c 0 (*) {'user': 'test'} (glob)
> -  2ca853e48edbd6453a0674dc0fe28a0974c51b9c aba7da93703075eec9fb1dbaf143ff2bc1c49d46 0 (*) {'user': 'test'} (glob)
> -  49d44ab2be1b67a79127568a67c9c99430633b48 273c1f3b86267ed3ec684bb13af1fa4d6ba56e02 0 (*) {'user': 'test'} (glob)
> -  46abc7c4d8738e8563e577f7889e1b6db3da4199 aba7da93703075eec9fb1dbaf143ff2bc1c49d46 0 (*) {'user': 'test'} (glob)
> +  e72d22b19f8ecf4150ab4f91d0973fd9955d3ddf 49d44ab2be1b67a79127568a67c9c99430633b48 0 (*) {'operation': 'amend', 'user': 'test'} (glob)
> +  3e30a45cf2f719e96ab3922dfe039cfd047956ce 0 {e72d22b19f8ecf4150ab4f91d0973fd9955d3ddf} (*) {'operation': 'amend', 'user': 'test'} (glob)
> +  1b2d564fad96311b45362f17c2aa855150efb35f 46abc7c4d8738e8563e577f7889e1b6db3da4199 0 (*) {'operation': 'histedit', 'user': 'test'} (glob)
> +  114f4176969ef342759a8a57e6bccefc4234829b 49d44ab2be1b67a79127568a67c9c99430633b48 0 (*) {'operation': 'histedit', 'user': 'test'} (glob)
> +  76f72745eac0643d16530e56e2f86e36e40631f1 2ca853e48edbd6453a0674dc0fe28a0974c51b9c 0 (*) {'operation': 'amend', 'user': 'test'} (glob)
> +  2ca853e48edbd6453a0674dc0fe28a0974c51b9c aba7da93703075eec9fb1dbaf143ff2bc1c49d46 0 (*) {'operation': 'amend', 'user': 'test'} (glob)
> +  49d44ab2be1b67a79127568a67c9c99430633b48 273c1f3b86267ed3ec684bb13af1fa4d6ba56e02 0 (*) {'operation': 'histedit', 'user': 'test'} (glob)
> +  46abc7c4d8738e8563e577f7889e1b6db3da4199 aba7da93703075eec9fb1dbaf143ff2bc1c49d46 0 (*) {'operation': 'histedit', 'user': 'test'} (glob)
>    $ cd ..
>
>  Base setup for the rest of the testing
> @@ -170,13 +170,13 @@ Base setup for the rest of the testing
>    o  0:cb9a9f314b8b a
>
>    $ hg debugobsolete
> -  96e494a2d553dd05902ba1cee1d94d4cb7b8faed 0 {b346ab9a313db8537ecf96fca3ca3ca984ef3bd7} (*) {'user': 'test'} (glob)
> -  b558abc46d09c30f57ac31e85a8a3d64d2e906e4 0 {96e494a2d553dd05902ba1cee1d94d4cb7b8faed} (*) {'user': 'test'} (glob)
> -  d2ae7f538514cd87c17547b0de4cea71fe1af9fb 0 {cb9a9f314b8b07ba71012fcdbc544b5a4d82ff5b} (*) {'user': 'test'} (glob)
> -  177f92b773850b59254aa5e923436f921b55483b b346ab9a313db8537ecf96fca3ca3ca984ef3bd7 0 (*) {'user': 'test'} (glob)
> -  055a42cdd88768532f9cf79daa407fc8d138de9b 59d9f330561fd6c88b1a6b32f0e45034d88db784 0 (*) {'user': 'test'} (glob)
> -  e860deea161a2f77de56603b340ebbb4536308ae 59d9f330561fd6c88b1a6b32f0e45034d88db784 0 (*) {'user': 'test'} (glob)
> -  652413bf663ef2a641cab26574e46d5f5a64a55a cacdfd884a9321ec4e1de275ef3949fa953a1f83 0 (*) {'user': 'test'} (glob)
> +  96e494a2d553dd05902ba1cee1d94d4cb7b8faed 0 {b346ab9a313db8537ecf96fca3ca3ca984ef3bd7} (*) {'operation': 'histedit', 'user': 'test'} (glob)
> +  b558abc46d09c30f57ac31e85a8a3d64d2e906e4 0 {96e494a2d553dd05902ba1cee1d94d4cb7b8faed} (*) {'operation': 'histedit', 'user': 'test'} (glob)
> +  d2ae7f538514cd87c17547b0de4cea71fe1af9fb 0 {cb9a9f314b8b07ba71012fcdbc544b5a4d82ff5b} (*) {'operation': 'histedit', 'user': 'test'} (glob)
> +  177f92b773850b59254aa5e923436f921b55483b b346ab9a313db8537ecf96fca3ca3ca984ef3bd7 0 (*) {'operation': 'histedit', 'user': 'test'} (glob)
> +  055a42cdd88768532f9cf79daa407fc8d138de9b 59d9f330561fd6c88b1a6b32f0e45034d88db784 0 (*) {'operation': 'histedit', 'user': 'test'} (glob)
> +  e860deea161a2f77de56603b340ebbb4536308ae 59d9f330561fd6c88b1a6b32f0e45034d88db784 0 (*) {'operation': 'histedit', 'user': 'test'} (glob)
> +  652413bf663ef2a641cab26574e46d5f5a64a55a cacdfd884a9321ec4e1de275ef3949fa953a1f83 0 (*) {'operation': 'histedit', 'user': 'test'} (glob)
>
>
>  Ensure hidden revision does not prevent histedit
> diff --git a/tests/test-obsolete.t b/tests/test-obsolete.t
> --- a/tests/test-obsolete.t
> +++ b/tests/test-obsolete.t
> @@ -1247,15 +1247,15 @@ only a subset of those are displayed (be
>    adding d
>    $ hg ci --amend -m dd
>    $ hg debugobsolete --index --rev "3+7"
> -  1 6fdef60fcbabbd3d50e9b9cbc2a240724b91a5e1 d27fb9b066076fd921277a4b9e8b9cb48c95bc6a 0 \(.*\) {'user': 'test'} (re)
> -  3 4715cf767440ed891755448016c2b8cf70760c30 7ae79c5d60f049c7b0dd02f5f25b9d60aaf7b36d 0 \(.*\) {'user': 'test'} (re)
> +  1 6fdef60fcbabbd3d50e9b9cbc2a240724b91a5e1 d27fb9b066076fd921277a4b9e8b9cb48c95bc6a 0 \(.*\) {'operation': 'amend', 'user': 'test'} (re)
> +  3 4715cf767440ed891755448016c2b8cf70760c30 7ae79c5d60f049c7b0dd02f5f25b9d60aaf7b36d 0 \(.*\) {'operation': 'amend', 'user': 'test'} (re)
>    $ hg debugobsolete --index --rev "3+7" -Tjson
>    [
>     {
>      "date": *, (glob)
>      "flag": 0,
>      "index": 1,
> -    "metadata": {"user": "test"},
> +    "metadata": {"operation": "amend", "user": "test"},
>      "precnode": "6fdef60fcbabbd3d50e9b9cbc2a240724b91a5e1",
>      "succnodes": ["d27fb9b066076fd921277a4b9e8b9cb48c95bc6a"]
>     },
> @@ -1263,7 +1263,7 @@ only a subset of those are displayed (be
>      "date": *, (glob)
>      "flag": 0,
>      "index": 3,
> -    "metadata": {"user": "test"},
> +    "metadata": {"operation": "amend", "user": "test"},
>      "precnode": "4715cf767440ed891755448016c2b8cf70760c30",
>      "succnodes": ["7ae79c5d60f049c7b0dd02f5f25b9d60aaf7b36d"]
>     }
> @@ -1271,14 +1271,14 @@ only a subset of those are displayed (be
>
>  Test the --delete option of debugobsolete command
>    $ hg debugobsolete --index
> -  0 cb9a9f314b8b07ba71012fcdbc544b5a4d82ff5b f9bd49731b0b175e42992a3c8fa6c678b2bc11f1 0 \(.*\) {'user': 'test'} (re)
> -  1 6fdef60fcbabbd3d50e9b9cbc2a240724b91a5e1 d27fb9b066076fd921277a4b9e8b9cb48c95bc6a 0 \(.*\) {'user': 'test'} (re)
> -  2 1ab51af8f9b41ef8c7f6f3312d4706d870b1fb74 29346082e4a9e27042b62d2da0e2de211c027621 0 \(.*\) {'user': 'test'} (re)
> -  3 4715cf767440ed891755448016c2b8cf70760c30 7ae79c5d60f049c7b0dd02f5f25b9d60aaf7b36d 0 \(.*\) {'user': 'test'} (re)
> +  0 cb9a9f314b8b07ba71012fcdbc544b5a4d82ff5b f9bd49731b0b175e42992a3c8fa6c678b2bc11f1 0 \(.*\) {'operation': 'amend', 'user': 'test'} (re)
> +  1 6fdef60fcbabbd3d50e9b9cbc2a240724b91a5e1 d27fb9b066076fd921277a4b9e8b9cb48c95bc6a 0 \(.*\) {'operation': 'amend', 'user': 'test'} (re)
> +  2 1ab51af8f9b41ef8c7f6f3312d4706d870b1fb74 29346082e4a9e27042b62d2da0e2de211c027621 0 \(.*\) {'operation': 'amend', 'user': 'test'} (re)
> +  3 4715cf767440ed891755448016c2b8cf70760c30 7ae79c5d60f049c7b0dd02f5f25b9d60aaf7b36d 0 \(.*\) {'operation': 'amend', 'user': 'test'} (re)
>    $ hg debugobsolete --delete 1 --delete 3
>    deleted 2 obsolescence markers
>    $ hg debugobsolete
> -  cb9a9f314b8b07ba71012fcdbc544b5a4d82ff5b f9bd49731b0b175e42992a3c8fa6c678b2bc11f1 0 \(.*\) {'user': 'test'} (re)
> -  1ab51af8f9b41ef8c7f6f3312d4706d870b1fb74 29346082e4a9e27042b62d2da0e2de211c027621 0 \(.*\) {'user': 'test'} (re)
> +  cb9a9f314b8b07ba71012fcdbc544b5a4d82ff5b f9bd49731b0b175e42992a3c8fa6c678b2bc11f1 0 \(.*\) {'operation': 'amend', 'user': 'test'} (re)
> +  1ab51af8f9b41ef8c7f6f3312d4706d870b1fb74 29346082e4a9e27042b62d2da0e2de211c027621 0 \(.*\) {'operation': 'amend', 'user': 'test'} (re)
>    $ cd ..
>
> diff --git a/tests/test-rebase-obsolete.t b/tests/test-rebase-obsolete.t
> --- a/tests/test-rebase-obsolete.t
> +++ b/tests/test-rebase-obsolete.t
> @@ -100,9 +100,9 @@ simple rebase
>    o  0:cd010b8cd998 A
>
>    $ hg debugobsolete
> -  42ccdea3bb16d28e1848c95fe2e44c000f3f21b1 e4e5be0395b2cbd471ed22a26b1b6a1a0658a794 0 (*) {'user': 'test'} (glob)
> -  5fddd98957c8a54a4d436dfe1da9d87f21a1b97b 2327fea05063f39961b14cb69435a9898dc9a245 0 (*) {'user': 'test'} (glob)
> -  32af7686d403cf45b5d95f2d70cebea587ac806a 8eeb3c33ad33d452c89e5dcf611c347f978fb42b 0 (*) {'user': 'test'} (glob)
> +  42ccdea3bb16d28e1848c95fe2e44c000f3f21b1 e4e5be0395b2cbd471ed22a26b1b6a1a0658a794 0 (*) {'operation': 'rebase', 'user': 'test'} (glob)
> +  5fddd98957c8a54a4d436dfe1da9d87f21a1b97b 2327fea05063f39961b14cb69435a9898dc9a245 0 (*) {'operation': 'rebase', 'user': 'test'} (glob)
> +  32af7686d403cf45b5d95f2d70cebea587ac806a 8eeb3c33ad33d452c89e5dcf611c347f978fb42b 0 (*) {'operation': 'rebase', 'user': 'test'} (glob)
>
>
>    $ cd ..
> @@ -170,9 +170,9 @@ set.
>    o  0:cd010b8cd998 A
>
>    $ hg debugobsolete
> -  42ccdea3bb16d28e1848c95fe2e44c000f3f21b1 0 {cd010b8cd998f3981a5a8115f94f8da4ab506089} (*) {'user': 'test'} (glob)
> -  5fddd98957c8a54a4d436dfe1da9d87f21a1b97b 5ae4c968c6aca831df823664e706c9d4aa34473d 0 (*) {'user': 'test'} (glob)
> -  32af7686d403cf45b5d95f2d70cebea587ac806a 0 {5fddd98957c8a54a4d436dfe1da9d87f21a1b97b} (*) {'user': 'test'} (glob)
> +  42ccdea3bb16d28e1848c95fe2e44c000f3f21b1 0 {cd010b8cd998f3981a5a8115f94f8da4ab506089} (*) {'operation': 'rebase', 'user': 'test'} (glob)
> +  5fddd98957c8a54a4d436dfe1da9d87f21a1b97b 5ae4c968c6aca831df823664e706c9d4aa34473d 0 (*) {'operation': 'rebase', 'user': 'test'} (glob)
> +  32af7686d403cf45b5d95f2d70cebea587ac806a 0 {5fddd98957c8a54a4d436dfe1da9d87f21a1b97b} (*) {'operation': 'rebase', 'user': 'test'} (glob)
>
>
>  More complex case where part of the rebase set were already rebased
> @@ -180,10 +180,10 @@ More complex case where part of the reba
>    $ hg rebase --rev 'desc(D)' --dest 'desc(H)'
>    rebasing 9:08483444fef9 "D"
>    $ hg debugobsolete
> -  42ccdea3bb16d28e1848c95fe2e44c000f3f21b1 0 {cd010b8cd998f3981a5a8115f94f8da4ab506089} (*) {'user': 'test'} (glob)
> -  5fddd98957c8a54a4d436dfe1da9d87f21a1b97b 5ae4c968c6aca831df823664e706c9d4aa34473d 0 (*) {'user': 'test'} (glob)
> -  32af7686d403cf45b5d95f2d70cebea587ac806a 0 {5fddd98957c8a54a4d436dfe1da9d87f21a1b97b} (*) {'user': 'test'} (glob)
> -  08483444fef91d6224f6655ee586a65d263ad34c 4596109a6a4328c398bde3a4a3b6737cfade3003 0 (*) {'user': 'test'} (glob)
> +  42ccdea3bb16d28e1848c95fe2e44c000f3f21b1 0 {cd010b8cd998f3981a5a8115f94f8da4ab506089} (*) {'operation': 'rebase', 'user': 'test'} (glob)
> +  5fddd98957c8a54a4d436dfe1da9d87f21a1b97b 5ae4c968c6aca831df823664e706c9d4aa34473d 0 (*) {'operation': 'rebase', 'user': 'test'} (glob)
> +  32af7686d403cf45b5d95f2d70cebea587ac806a 0 {5fddd98957c8a54a4d436dfe1da9d87f21a1b97b} (*) {'operation': 'rebase', 'user': 'test'} (glob)
> +  08483444fef91d6224f6655ee586a65d263ad34c 4596109a6a4328c398bde3a4a3b6737cfade3003 0 (*) {'operation': 'rebase', 'user': 'test'} (glob)
>    $ hg log -G
>    @  11:4596109a6a43 D
>    |
> @@ -208,12 +208,12 @@ More complex case where part of the reba
>    note: not rebasing 9:08483444fef9 "D", already in destination as 11:4596109a6a43 "D"
>    rebasing 10:5ae4c968c6ac "C"
>    $ hg debugobsolete
> -  42ccdea3bb16d28e1848c95fe2e44c000f3f21b1 0 {cd010b8cd998f3981a5a8115f94f8da4ab506089} (*) {'user': 'test'} (glob)
> -  5fddd98957c8a54a4d436dfe1da9d87f21a1b97b 5ae4c968c6aca831df823664e706c9d4aa34473d 0 (*) {'user': 'test'} (glob)
> -  32af7686d403cf45b5d95f2d70cebea587ac806a 0 {5fddd98957c8a54a4d436dfe1da9d87f21a1b97b} (*) {'user': 'test'} (glob)
> -  08483444fef91d6224f6655ee586a65d263ad34c 4596109a6a4328c398bde3a4a3b6737cfade3003 0 (*) {'user': 'test'} (glob)
> -  8877864f1edb05d0e07dc4ba77b67a80a7b86672 462a34d07e599b87ea08676a449373fe4e2e1347 0 (*) {'user': 'test'} (glob)
> -  5ae4c968c6aca831df823664e706c9d4aa34473d 98f6af4ee9539e14da4465128f894c274900b6e5 0 (*) {'user': 'test'} (glob)
> +  42ccdea3bb16d28e1848c95fe2e44c000f3f21b1 0 {cd010b8cd998f3981a5a8115f94f8da4ab506089} (*) {'operation': 'rebase', 'user': 'test'} (glob)
> +  5fddd98957c8a54a4d436dfe1da9d87f21a1b97b 5ae4c968c6aca831df823664e706c9d4aa34473d 0 (*) {'operation': 'rebase', 'user': 'test'} (glob)
> +  32af7686d403cf45b5d95f2d70cebea587ac806a 0 {5fddd98957c8a54a4d436dfe1da9d87f21a1b97b} (*) {'operation': 'rebase', 'user': 'test'} (glob)
> +  08483444fef91d6224f6655ee586a65d263ad34c 4596109a6a4328c398bde3a4a3b6737cfade3003 0 (*) {'operation': 'rebase', 'user': 'test'} (glob)
> +  8877864f1edb05d0e07dc4ba77b67a80a7b86672 462a34d07e599b87ea08676a449373fe4e2e1347 0 (*) {'operation': 'rebase', 'user': 'test'} (glob)
> +  5ae4c968c6aca831df823664e706c9d4aa34473d 98f6af4ee9539e14da4465128f894c274900b6e5 0 (*) {'operation': 'rebase', 'user': 'test'} (glob)
>    $ hg log --rev 'divergent()'
>    $ hg log -G
>    o  13:98f6af4ee953 C
> @@ -349,9 +349,9 @@ collapse rebase
>    $ hg id --debug -r tip
>    4dc2197e807bae9817f09905b50ab288be2dbbcf tip
>    $ hg debugobsolete
> -  42ccdea3bb16d28e1848c95fe2e44c000f3f21b1 4dc2197e807bae9817f09905b50ab288be2dbbcf 0 (*) {'user': 'test'} (glob)
> -  5fddd98957c8a54a4d436dfe1da9d87f21a1b97b 4dc2197e807bae9817f09905b50ab288be2dbbcf 0 (*) {'user': 'test'} (glob)
> -  32af7686d403cf45b5d95f2d70cebea587ac806a 4dc2197e807bae9817f09905b50ab288be2dbbcf 0 (*) {'user': 'test'} (glob)
> +  42ccdea3bb16d28e1848c95fe2e44c000f3f21b1 4dc2197e807bae9817f09905b50ab288be2dbbcf 0 (*) {'operation': 'rebase', 'user': 'test'} (glob)
> +  5fddd98957c8a54a4d436dfe1da9d87f21a1b97b 4dc2197e807bae9817f09905b50ab288be2dbbcf 0 (*) {'operation': 'rebase', 'user': 'test'} (glob)
> +  32af7686d403cf45b5d95f2d70cebea587ac806a 4dc2197e807bae9817f09905b50ab288be2dbbcf 0 (*) {'operation': 'rebase', 'user': 'test'} (glob)
>
>    $ cd ..
>
> @@ -411,9 +411,9 @@ not be rebased.
>    o  0:cd010b8cd998 A
>
>    $ hg debugobsolete
> -  5fddd98957c8a54a4d436dfe1da9d87f21a1b97b e273c5e7d2d29df783dce9f9eaa3ac4adc69c15d 0 (*) {'user': 'test'} (glob)
> -  32af7686d403cf45b5d95f2d70cebea587ac806a cf44d2f5a9f4297a62be94cbdd3dff7c7dc54258 0 (*) {'user': 'test'} (glob)
> -  42ccdea3bb16d28e1848c95fe2e44c000f3f21b1 7c6027df6a99d93f461868e5433f63bde20b6dfb 0 (*) {'user': 'test'} (glob)
> +  5fddd98957c8a54a4d436dfe1da9d87f21a1b97b e273c5e7d2d29df783dce9f9eaa3ac4adc69c15d 0 (*) {'operation': 'rebase', 'user': 'test'} (glob)
> +  32af7686d403cf45b5d95f2d70cebea587ac806a cf44d2f5a9f4297a62be94cbdd3dff7c7dc54258 0 (*) {'operation': 'rebase', 'user': 'test'} (glob)
> +  42ccdea3bb16d28e1848c95fe2e44c000f3f21b1 7c6027df6a99d93f461868e5433f63bde20b6dfb 0 (*) {'operation': 'rebase', 'user': 'test'} (glob)
>
>  Test that rewriting leaving instability behind is allowed
>  ---------------------------------------------------------------------
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
>
Durham Goode - May 10, 2017, 3:45 p.m.
On 5/10/17 3:40 AM, Pierre-Yves David wrote:
> On 05/10/2017 01:34 AM, Durham Goode wrote:
>> # HG changeset patch
>> # User Durham Goode <durham@fb.com>
>> # Date 1494372571 25200
>> #      Tue May 09 16:29:31 2017 -0700
>> # Node ID 22051b0924bcbc5cde6d25d700dcce6afcaafd98
>> # Parent  8f1a2b848b52ea7bf3fe2404e3b62924c7aae93f
>> obsolete: add operation metadata to rebase/amend/histedit obsmarkers
>
> TL;DR; Storing more information is useful, but the pre-existing plans
> aims at storing slightly different data in a different way. We should
> stick to it.
>
> The idea is to use the obsmarker bitfields to store the data and record
> a pre-defined set of "effects" in each markers. (eg: "content change,
> message change, parent change")
>
> For examples:
>
> 1) a `hg amend` that update the diff will create a markers recording the
> "content changes".
>
> 2) a `hg amend -e` that only update he commit message will record the
> "message change".
>
> 3) a `hg rebase` will record the "parent change".
>
> 4) a `hg amend` that update the message, the content and user will
> record these three things (content, message and meta changes).
>
> These data can then be used to have nice template as shown by Durham
> (eg: is X is a successors of Y and the markers as some "content change"
> bit, we can display "amended as Y"; if their are some parent change, we
> can display "rebased as Y", if both are present "amended and rebased as
> Y").
>

While having the above data seems nice, I don't know if it justifies not 
going with the current simple approach today. Otherwise we have to wait 
for this large obsstore refactor, which seems like a rather big 
dependency to block an important UX feature we could deliver today.

When the bit field is added in the future we can always change our 
templates and messaging to use that data as well.  And if it's good 
enough, we can drop the 'operation' metadata entirely, with little 
compatibility issue.

> The above approach as multiple advantages:
>
>   Compact: Since we use the (existing) bitfield, the storage cost is
> free (or minimal if we need to adds an extra byte of bitfield).
> Accessing the data is also very cheap:
>
>   Compoundable: We'll often needs to combine multiple markers to reports
> changesets between two revisions (eg: we do not have the intermediate
> revisions locally). the bit field approach makes it trivial to compound
> the information. We can display display the same final information when
> the same result if obtained from different path (eg: from two markers
> [content change; message change] or from one markers [content change |
> message change]).
>   This part is important because simply using operation name makes it
> quite hard to combine the information.

Yes, if there were multiple operations we'd have to union them just as 
operation='modified' or something. Not ideal, but should be an outlier case.

>   UI Abstraction: since we record the "effect" information, we abstract
> the command names. This makes use more agile about the actual UI. User
> can use their own "hg myownrewritetool" command and still record
> information useful for the other users.

I think recording the actual command the user ran is important. The goal 
of showing this in the UI is to let the user know what past action 
caused this commit to become a new commit, so if they run 'hg 
flabbernate' it's much more useful to show them 'fblabbernated to be X' 
than it is to show them 'amended to be X'. So potentially, even if we 
have the bitfield data, it could still be important to record the actual 
command that caused this obsmarker to be created (ex: "amended to be X 
by hg flabbernate")
Augie Fackler - May 10, 2017, 3:51 p.m.
> On May 10, 2017, at 11:45, Durham Goode <durham@fb.com> wrote:
> 
> On 5/10/17 3:40 AM, Pierre-Yves David wrote:
>> On 05/10/2017 01:34 AM, Durham Goode wrote:
>>> # HG changeset patch
>>> # User Durham Goode <durham@fb.com>
>>> # Date 1494372571 25200
>>> #      Tue May 09 16:29:31 2017 -0700
>>> # Node ID 22051b0924bcbc5cde6d25d700dcce6afcaafd98
>>> # Parent  8f1a2b848b52ea7bf3fe2404e3b62924c7aae93f
>>> obsolete: add operation metadata to rebase/amend/histedit obsmarkers
>> 
>> TL;DR; Storing more information is useful, but the pre-existing plans
>> aims at storing slightly different data in a different way. We should
>> stick to it.
>> 
>> The idea is to use the obsmarker bitfields to store the data and record
>> a pre-defined set of "effects" in each markers. (eg: "content change,
>> message change, parent change")
>> 
>> For examples:
>> 
>> 1) a `hg amend` that update the diff will create a markers recording the
>> "content changes".
>> 
>> 2) a `hg amend -e` that only update he commit message will record the
>> "message change".
>> 
>> 3) a `hg rebase` will record the "parent change".
>> 
>> 4) a `hg amend` that update the message, the content and user will
>> record these three things (content, message and meta changes).
>> 
>> These data can then be used to have nice template as shown by Durham
>> (eg: is X is a successors of Y and the markers as some "content change"
>> bit, we can display "amended as Y"; if their are some parent change, we
>> can display "rebased as Y", if both are present "amended and rebased as
>> Y").
>> 
> 
> While having the above data seems nice, I don't know if it justifies not going with the current simple approach today. Otherwise we have to wait for this large obsstore refactor, which seems like a rather big dependency to block an important UX feature we could deliver today.

I agree. This is something that we've observed would be a big help to our git refugees at Google. I'd like to move forward with putting it in extra as a string.

> When the bit field is added in the future we can always change our templates and messaging to use that data as well.  And if it's good enough, we can drop the 'operation' metadata entirely, with little compatibility issue.

Yep. Sounds good to me.

> 
>> The above approach as multiple advantages:
>> 
>>  Compact: Since we use the (existing) bitfield, the storage cost is
>> free (or minimal if we need to adds an extra byte of bitfield).
>> Accessing the data is also very cheap:
>> 
>>  Compoundable: We'll often needs to combine multiple markers to reports
>> changesets between two revisions (eg: we do not have the intermediate
>> revisions locally). the bit field approach makes it trivial to compound
>> the information. We can display display the same final information when
>> the same result if obtained from different path (eg: from two markers
>> [content change; message change] or from one markers [content change |
>> message change]).
>>  This part is important because simply using operation name makes it
>> quite hard to combine the information.
> 
> Yes, if there were multiple operations we'd have to union them just as operation='modified' or something. Not ideal, but should be an outlier case.
> 
>>  UI Abstraction: since we record the "effect" information, we abstract
>> the command names. This makes use more agile about the actual UI. User
>> can use their own "hg myownrewritetool" command and still record
>> information useful for the other users.
> 
> I think recording the actual command the user ran is important. The goal of showing this in the UI is to let the user know what past action caused this commit to become a new commit, so if they run 'hg flabbernate' it's much more useful to show them 'fblabbernated to be X' than it is to show them 'amended to be X'. So potentially, even if we have the bitfield data, it could still be important to record the actual command that caused this obsmarker to be created (ex: "amended to be X by hg flabbernate")

I 100% agree here. The command verbs should map obviously onto the verbs that are saved in the obsmarkers.

AF
via Mercurial-devel - May 10, 2017, 3:52 p.m.
On Wed, May 10, 2017 at 8:45 AM, Durham Goode <durham@fb.com> wrote:
> On 5/10/17 3:40 AM, Pierre-Yves David wrote:
>>
>> On 05/10/2017 01:34 AM, Durham Goode wrote:
>>>
>>> # HG changeset patch
>>> # User Durham Goode <durham@fb.com>
>>> # Date 1494372571 25200
>>> #      Tue May 09 16:29:31 2017 -0700
>>> # Node ID 22051b0924bcbc5cde6d25d700dcce6afcaafd98
>>> # Parent  8f1a2b848b52ea7bf3fe2404e3b62924c7aae93f
>>> obsolete: add operation metadata to rebase/amend/histedit obsmarkers
>>
>>
>> TL;DR; Storing more information is useful, but the pre-existing plans
>> aims at storing slightly different data in a different way. We should
>> stick to it.
>>
>> The idea is to use the obsmarker bitfields to store the data and record
>> a pre-defined set of "effects" in each markers. (eg: "content change,
>> message change, parent change")
>>
>> For examples:
>>
>> 1) a `hg amend` that update the diff will create a markers recording the
>> "content changes".
>>
>> 2) a `hg amend -e` that only update he commit message will record the
>> "message change".
>>
>> 3) a `hg rebase` will record the "parent change".
>>
>> 4) a `hg amend` that update the message, the content and user will
>> record these three things (content, message and meta changes).
>>
>> These data can then be used to have nice template as shown by Durham
>> (eg: is X is a successors of Y and the markers as some "content change"
>> bit, we can display "amended as Y"; if their are some parent change, we
>> can display "rebased as Y", if both are present "amended and rebased as
>> Y").
>>
>
> While having the above data seems nice, I don't know if it justifies not
> going with the current simple approach today. Otherwise we have to wait for
> this large obsstore refactor, which seems like a rather big dependency to
> block an important UX feature we could deliver today.
>
> When the bit field is added in the future we can always change our templates
> and messaging to use that data as well.  And if it's good enough, we can
> drop the 'operation' metadata entirely, with little compatibility issue.
>
>> The above approach as multiple advantages:
>>
>>   Compact: Since we use the (existing) bitfield, the storage cost is
>> free (or minimal if we need to adds an extra byte of bitfield).
>> Accessing the data is also very cheap:
>>
>>   Compoundable: We'll often needs to combine multiple markers to reports
>> changesets between two revisions (eg: we do not have the intermediate
>> revisions locally). the bit field approach makes it trivial to compound
>> the information. We can display display the same final information when
>> the same result if obtained from different path (eg: from two markers
>> [content change; message change] or from one markers [content change |
>> message change]).
>>   This part is important because simply using operation name makes it
>> quite hard to combine the information.
>
>
> Yes, if there were multiple operations we'd have to union them just as
> operation='modified' or something. Not ideal, but should be an outlier case.

With our setup internally, it's not really an outlier case. We tag
revisions that are sent for code review and leave the tags until next
time the change is sent for review. There can thus be many amends and
rebases between the revision sent for review and the current latest
revision. I'm not sure what a good way of presenting that to the user
would be.

>
>>   UI Abstraction: since we record the "effect" information, we abstract
>> the command names. This makes use more agile about the actual UI. User
>> can use their own "hg myownrewritetool" command and still record
>> information useful for the other users.
>
>
> I think recording the actual command the user ran is important. The goal of
> showing this in the UI is to let the user know what past action caused this
> commit to become a new commit, so if they run 'hg flabbernate' it's much
> more useful to show them 'fblabbernated to be X' than it is to show them
> 'amended to be X'. So potentially, even if we have the bitfield data, it
> could still be important to record the actual command that caused this
> obsmarker to be created (ex: "amended to be X by hg flabbernate")
>
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Augie Fackler - May 10, 2017, 3:57 p.m.
> On May 10, 2017, at 11:52, Martin von Zweigbergk <martinvonz@google.com> wrote:
> 
>>> Compoundable: We'll often needs to combine multiple markers to reports
>>> changesets between two revisions (eg: we do not have the intermediate
>>> revisions locally). the bit field approach makes it trivial to compound
>>> the information. We can display display the same final information when
>>> the same result if obtained from different path (eg: from two markers
>>> [content change; message change] or from one markers [content change |
>>> message change]).
>>>  This part is important because simply using operation name makes it
>>> quite hard to combine the information.
>> 
>> 
>> Yes, if there were multiple operations we'd have to union them just as
>> operation='modified' or something. Not ideal, but should be an outlier case.
> 
> With our setup internally, it's not really an outlier case. We tag
> revisions that are sent for code review and leave the tags until next
> time the change is sent for review. There can thus be many amends and
> rebases between the revision sent for review and the current latest
> revision. I'm not sure what a good way of presenting that to the user
> would be.

I think the common case is rebased and amended - couldn't we observe the list of actions in the list and emit something like 

deadbeef rebased, amended to beefdead

in `hg log` output?

I'd rather not let the perfect be the enemy of the good here - we can at least get the data recorded and start experimenting with ways of operating on it. It's already missing from countless thousands of markers, so we'll never be able to assume it exists, so I feel like we might as well start recording it and see if we need to iterate on how we're recording it from there.
Durham Goode - May 10, 2017, 3:59 p.m.
On 5/10/17 8:52 AM, Martin von Zweigbergk wrote:
> On Wed, May 10, 2017 at 8:45 AM, Durham Goode <durham@fb.com> wrote:
>> On 5/10/17 3:40 AM, Pierre-Yves David wrote:
>>>
>>>   Compoundable: We'll often needs to combine multiple markers to reports
>>> changesets between two revisions (eg: we do not have the intermediate
>>> revisions locally). the bit field approach makes it trivial to compound
>>> the information. We can display display the same final information when
>>> the same result if obtained from different path (eg: from two markers
>>> [content change; message change] or from one markers [content change |
>>> message change]).
>>>   This part is important because simply using operation name makes it
>>> quite hard to combine the information.
>>
>>
>> Yes, if there were multiple operations we'd have to union them just as
>> operation='modified' or something. Not ideal, but should be an outlier case.
>
> With our setup internally, it's not really an outlier case. We tag
> revisions that are sent for code review and leave the tags until next
> time the change is sent for review. There can thus be many amends and
> rebases between the revision sent for review and the current latest
> revision. I'm not sure what a good way of presenting that to the user
> would be.

Ah, yes. I was thinking about multiple markers between the same nodes, 
but the chain of commits that are not all visible is a common case (and 
is obviously what Pierre-Yves was talking about when I read closer).

Yes, this case wouldn't be well handled by the operation string 
solution.  One solution we've had internally is to have a smartlog 
output that shows all the successors to a node.  So you can see "X was 
amended to be Y", "Y was rebased to be Z", "Z was landed as V". It makes 
it very easy to visually traverse the history of a node and could allow 
a user to dig in to a generic "X was modified to be V" unioned-message.
via Mercurial-devel - May 10, 2017, 3:59 p.m.
On Wed, May 10, 2017 at 8:57 AM, Augie Fackler <raf@durin42.com> wrote:
>
>> On May 10, 2017, at 11:52, Martin von Zweigbergk <martinvonz@google.com> wrote:
>>
>>>> Compoundable: We'll often needs to combine multiple markers to reports
>>>> changesets between two revisions (eg: we do not have the intermediate
>>>> revisions locally). the bit field approach makes it trivial to compound
>>>> the information. We can display display the same final information when
>>>> the same result if obtained from different path (eg: from two markers
>>>> [content change; message change] or from one markers [content change |
>>>> message change]).
>>>>  This part is important because simply using operation name makes it
>>>> quite hard to combine the information.
>>>
>>>
>>> Yes, if there were multiple operations we'd have to union them just as
>>> operation='modified' or something. Not ideal, but should be an outlier case.
>>
>> With our setup internally, it's not really an outlier case. We tag
>> revisions that are sent for code review and leave the tags until next
>> time the change is sent for review. There can thus be many amends and
>> rebases between the revision sent for review and the current latest
>> revision. I'm not sure what a good way of presenting that to the user
>> would be.
>
> I think the common case is rebased and amended - couldn't we observe the list of actions in the list and emit something like
>
> deadbeef rebased, amended to beefdead
>
> in `hg log` output?
>
> I'd rather not let the perfect be the enemy of the good here - we can at least get the data recorded and start experimenting with ways of operating on it. It's already missing from countless thousands of markers, so we'll never be able to assume it exists, so I feel like we might as well start recording it and see if we need to iterate on how we're recording it from there.

I agree. I didn't mean to use it as an argument against Durham's
patch, just something to think about. And I think agree with you about
how to do it. My initial thought was also to keep a set of all
distinct operations along the chain.
Sean Farley - May 10, 2017, 5:59 p.m.
Augie Fackler <raf@durin42.com> writes:

>> On May 10, 2017, at 11:45, Durham Goode <durham@fb.com> wrote:
>> 
>> On 5/10/17 3:40 AM, Pierre-Yves David wrote:
>>> On 05/10/2017 01:34 AM, Durham Goode wrote:
>>>> # HG changeset patch
>>>> # User Durham Goode <durham@fb.com>
>>>> # Date 1494372571 25200
>>>> #      Tue May 09 16:29:31 2017 -0700
>>>> # Node ID 22051b0924bcbc5cde6d25d700dcce6afcaafd98
>>>> # Parent  8f1a2b848b52ea7bf3fe2404e3b62924c7aae93f
>>>> obsolete: add operation metadata to rebase/amend/histedit obsmarkers
>>> 
>>> TL;DR; Storing more information is useful, but the pre-existing plans
>>> aims at storing slightly different data in a different way. We should
>>> stick to it.
>>> 
>>> The idea is to use the obsmarker bitfields to store the data and record
>>> a pre-defined set of "effects" in each markers. (eg: "content change,
>>> message change, parent change")
>>> 
>>> For examples:
>>> 
>>> 1) a `hg amend` that update the diff will create a markers recording the
>>> "content changes".
>>> 
>>> 2) a `hg amend -e` that only update he commit message will record the
>>> "message change".
>>> 
>>> 3) a `hg rebase` will record the "parent change".
>>> 
>>> 4) a `hg amend` that update the message, the content and user will
>>> record these three things (content, message and meta changes).
>>> 
>>> These data can then be used to have nice template as shown by Durham
>>> (eg: is X is a successors of Y and the markers as some "content change"
>>> bit, we can display "amended as Y"; if their are some parent change, we
>>> can display "rebased as Y", if both are present "amended and rebased as
>>> Y").
>>> 
>> 
>> While having the above data seems nice, I don't know if it justifies not going with the current simple approach today. Otherwise we have to wait for this large obsstore refactor, which seems like a rather big dependency to block an important UX feature we could deliver today.
>
> I agree. This is something that we've observed would be a big help to our git refugees at Google. I'd like to move forward with putting it in extra as a string.
>
>> When the bit field is added in the future we can always change our templates and messaging to use that data as well.  And if it's good enough, we can drop the 'operation' metadata entirely, with little compatibility issue.
>
> Yep. Sounds good to me.
>
>> 
>>> The above approach as multiple advantages:
>>> 
>>>  Compact: Since we use the (existing) bitfield, the storage cost is
>>> free (or minimal if we need to adds an extra byte of bitfield).
>>> Accessing the data is also very cheap:
>>> 
>>>  Compoundable: We'll often needs to combine multiple markers to reports
>>> changesets between two revisions (eg: we do not have the intermediate
>>> revisions locally). the bit field approach makes it trivial to compound
>>> the information. We can display display the same final information when
>>> the same result if obtained from different path (eg: from two markers
>>> [content change; message change] or from one markers [content change |
>>> message change]).
>>>  This part is important because simply using operation name makes it
>>> quite hard to combine the information.
>> 
>> Yes, if there were multiple operations we'd have to union them just as operation='modified' or something. Not ideal, but should be an outlier case.
>> 
>>>  UI Abstraction: since we record the "effect" information, we abstract
>>> the command names. This makes use more agile about the actual UI. User
>>> can use their own "hg myownrewritetool" command and still record
>>> information useful for the other users.
>> 
>> I think recording the actual command the user ran is important. The goal of showing this in the UI is to let the user know what past action caused this commit to become a new commit, so if they run 'hg flabbernate' it's much more useful to show them 'fblabbernated to be X' than it is to show them 'amended to be X'. So potentially, even if we have the bitfield data, it could still be important to record the actual command that caused this obsmarker to be created (ex: "amended to be X by hg flabbernate")
>
> I 100% agree here. The command verbs should map obviously onto the verbs that are saved in the obsmarkers.

Should this include aliases too? My initial gut reaction is 'yes' since
that would be what the user actually types and can help debug easier.
Augie Fackler - May 10, 2017, 6:37 p.m.
> On May 10, 2017, at 13:59, Sean Farley <sean@farley.io> wrote:
> 
> Should this include aliases too? My initial gut reaction is 'yes' since
> that would be what the user actually types and can help debug easier.

Maybe. Typically (not at G and F, but otherwise) users will have a vague sense of what their alias is configured to do.
Sean Farley - May 10, 2017, 6:57 p.m.
Augie Fackler <raf@durin42.com> writes:

>> On May 10, 2017, at 13:59, Sean Farley <sean@farley.io> wrote:
>> 
>> Should this include aliases too? My initial gut reaction is 'yes' since
>> that would be what the user actually types and can help debug easier.
>
> Maybe. Typically (not at G and F, but otherwise) users will have a vague sense of what their alias is configured to do.

My thought is that users (myself included) have aliases for flags that
change behavior to the command, e.g.

amend_with_grandparent = amend -r .^^::

Seeing "amend_with_grandparent" would vastly help convey what I did as a
user. Though, if this is too complicated, I'm totally ok with ditching
it.
Pierre-Yves David - May 10, 2017, 8:08 p.m.
On 05/10/2017 05:45 PM, Durham Goode wrote:
> On 5/10/17 3:40 AM, Pierre-Yves David wrote:
>> On 05/10/2017 01:34 AM, Durham Goode wrote:
>>> # HG changeset patch
>>> # User Durham Goode <durham@fb.com>
>>> # Date 1494372571 25200
>>> #      Tue May 09 16:29:31 2017 -0700
>>> # Node ID 22051b0924bcbc5cde6d25d700dcce6afcaafd98
>>> # Parent  8f1a2b848b52ea7bf3fe2404e3b62924c7aae93f
>>> obsolete: add operation metadata to rebase/amend/histedit obsmarkers
>>
>> TL;DR; Storing more information is useful, but the pre-existing plans
>> aims at storing slightly different data in a different way. We should
>> stick to it.
>>
>> The idea is to use the obsmarker bitfields to store the data and record
>> a pre-defined set of "effects" in each markers. (eg: "content change,
>> message change, parent change")
>>
>> For examples:
>>
>> 1) a `hg amend` that update the diff will create a markers recording the
>> "content changes".
>>
>> 2) a `hg amend -e` that only update he commit message will record the
>> "message change".
>>
>> 3) a `hg rebase` will record the "parent change".
>>
>> 4) a `hg amend` that update the message, the content and user will
>> record these three things (content, message and meta changes).
>>
>> These data can then be used to have nice template as shown by Durham
>> (eg: is X is a successors of Y and the markers as some "content change"
>> bit, we can display "amended as Y"; if their are some parent change, we
>> can display "rebased as Y", if both are present "amended and rebased as
>> Y").
>>
>
> While having the above data seems nice, I don't know if it justifies not
> going with the current simple approach today. Otherwise we have to wait
> for this large obsstore refactor, which seems like a rather big
> dependency to block an important UX feature we could deliver today.

There seems to be a major misunderstanding here.

Obsmarkers -already- has a bitfield (for many years) so are -already- 
able to use them to record actions. There are no delay or blocker here 
everything is already in place for it. There are basically no reason to 
not just do it.

> When the bit field is added in the future we can always change our
> templates and messaging to use that data as well.  And if it's good
> enough, we can drop the 'operation' metadata entirely, with little
> compatibility issue.
>
>> The above approach as multiple advantages:
>>
>>   Compact: Since we use the (existing) bitfield, the storage cost is
>> free (or minimal if we need to adds an extra byte of bitfield).
>> Accessing the data is also very cheap:
>>
>>   Compoundable: We'll often needs to combine multiple markers to reports
>> changesets between two revisions (eg: we do not have the intermediate
>> revisions locally). the bit field approach makes it trivial to compound
>> the information. We can display display the same final information when
>> the same result if obtained from different path (eg: from two markers
>> [content change; message change] or from one markers [content change |
>> message change]).
>>   This part is important because simply using operation name makes it
>> quite hard to combine the information.
>
> Yes, if there were multiple operations we'd have to union them just as
> operation='modified' or something. Not ideal, but should be an outlier
> case.

Having multiple markers between local changesets is the common case once 
people start exchanging draft changesets. We can not dismiss this as an 
outliers, behaving properly here is an actual request from the current 
users.

>>   UI Abstraction: since we record the "effect" information, we abstract
>> the command names. This makes use more agile about the actual UI. User
>> can use their own "hg myownrewritetool" command and still record
>> information useful for the other users.
>
> I think recording the actual command the user ran is important. The goal
> of showing this in the UI is to let the user know what past action
> caused this commit to become a new commit, so if they run 'hg
> flabbernate' it's much more useful to show them 'fblabbernated to be X'
> than it is to show them 'amended to be X'. So potentially, even if we
> have the bitfield data, it could still be important to record the actual
> command that caused this obsmarker to be created (ex: "amended to be X
> by hg flabbernate")

Having more information is always useful, but this looks like something 
to add as a second step.

The action-bit approach has clear advantages (no size concerns, clear 
behavior with marker compositions), so this appears as a clear good 
first step here.

In addition, if that first step turn out to not be enough and we want 
more precise data we might want to look at leveraging the "hg journal" 
local data for this.


I would recommend the following step forward:

1) add a simple "rewritten as" template,
2) record action-bit, use it in the template (and other related output),
3) gather data and look at the best way to issue more details.

By chance step (1) and (2) are on the list of UI improvement founded by 
Unity (1) is about to be done (read: next week) and (2) is likely to be 
scheduled in the next batch (read, this month).

If you want to work in this area, I recommend catching up with the 
recent improvement in evolve and getting in touch with Boris Feld 
(currently writing doing most of the lifting in this area).

Cheers,
Matt Harbison - May 11, 2017, 1:46 a.m.
On Tue, 09 May 2017 19:55:57 -0400, Martin von Zweigbergk via  
Mercurial-devel <mercurial-devel@mercurial-scm.org> wrote:

> On Tue, May 9, 2017 at 4:34 PM, Durham Goode <durham@fb.com> wrote:
>> # HG changeset patch
>> # User Durham Goode <durham@fb.com>
>> # Date 1494372571 25200
>> #      Tue May 09 16:29:31 2017 -0700
>> # Node ID 22051b0924bcbc5cde6d25d700dcce6afcaafd98
>> # Parent  8f1a2b848b52ea7bf3fe2404e3b62924c7aae93f
>> obsolete: add operation metadata to rebase/amend/histedit obsmarkers
>>
>> By recording what operation created the obsmarker, we can show very  
>> intuitive
>> messages to the user in various UIs. For instance, log output could have
>> messages like "Amended as XXX" to show why a commit is old and has an  
>> 'x' on it.
>
> Nice! I had previously assumed that that information was already in
> the obsmarker, but then I didn't find it in `hg debugobsolete`. I
> didn't realize it would be so easy to add. Seems like an obvious
> improvement. So obvious that I'm hesitating to queue it only because
> it seems like it should have been proposed before. Any idea why that
> hasn't happened?

It's not obsolete specific, so maybe not relevant, but operation metadata  
made me recall this thread:

https://www.mercurial-scm.org/pipermail/mercurial-devel/2015-April/068692.html

>>
>>      @  ac28e3  durham
>>     /   First commit
>>    |
>>    | o  d4afe7 durham
>>    | |  Second commit
>>    | |
>>    | x  8e9a5d (Amended as ac28e3)  durham
>>    |/   First commit
>>    |
>>
>> diff --git a/hgext/histedit.py b/hgext/histedit.py
>> --- a/hgext/histedit.py
>> +++ b/hgext/histedit.py
>> @@ -1631,7 +1631,7 @@ def safecleanupnode(ui, repo, name, node
>>                               key=repo.changelog.rev)
>>          markers = [getmarker(t) for t in sortednodes]
>>          if markers:
>> -            obsolete.createmarkers(repo, markers)
>> +            obsolete.createmarkers(repo, markers, operation='histedit')
>>      else:
>>          return cleanupnode(ui, repo, name, nodes)
>>
>> diff --git a/hgext/rebase.py b/hgext/rebase.py
>> --- a/hgext/rebase.py
>> +++ b/hgext/rebase.py
>> @@ -1341,7 +1341,7 @@ def clearrebased(ui, repo, state, skippe
>>                      succs = (repo[newrev],)
>>                  markers.append((repo[rev], succs))
>>          if markers:
>> -            obsolete.createmarkers(repo, markers)
>> +            obsolete.createmarkers(repo, markers, operation='rebase')
>>      else:
>>          rebased = [rev for rev in state if state[rev] > nullmerge]
>>          if rebased:
>> diff --git a/mercurial/cmdutil.py b/mercurial/cmdutil.py
>> --- a/mercurial/cmdutil.py
>> +++ b/mercurial/cmdutil.py
>> @@ -2740,7 +2740,7 @@ def amend(ui, repo, commitfunc, old, ext
>>                      if node:
>>                          obs.append((ctx, ()))
>>
>> -                    obsolete.createmarkers(repo, obs)
>> +                    obsolete.createmarkers(repo, obs,  
>> operation='amend')
>>          if not createmarkers and newid != old.node():
>>              # Strip the intermediate commit (if there was one) and the  
>> amended
>>              # commit
>> diff --git a/mercurial/obsolete.py b/mercurial/obsolete.py
>> --- a/mercurial/obsolete.py
>> +++ b/mercurial/obsolete.py
>> @@ -1204,7 +1204,8 @@ def _computedivergentset(repo):
>>      return divergent
>>
>>
>> -def createmarkers(repo, relations, flag=0, date=None, metadata=None):
>> +def createmarkers(repo, relations, flag=0, date=None, metadata=None,
>> +                  operation=None):
>>      """Add obsolete markers between changesets in a repo
>>
>>      <relations> must be an iterable of (<old>, (<new>,  
>> ...)[,{metadata}])
>> @@ -1225,6 +1226,8 @@ def createmarkers(repo, relations, flag=
>>          metadata = {}
>>      if 'user' not in metadata:
>>          metadata['user'] = repo.ui.username()
>> +    if operation:
>> +        metadata['operation'] = operation
>>      tr = repo.transaction('add-obsolescence-marker')
>>      try:
>>          markerargs = []
>> diff --git a/tests/test-histedit-obsolete.t  
>> b/tests/test-histedit-obsolete.t
>> --- a/tests/test-histedit-obsolete.t
>> +++ b/tests/test-histedit-obsolete.t
>> @@ -50,10 +50,10 @@ Test that histedit learns about obsolesc
>>    o  0:cb9a9f314b8b a
>>
>>    $ hg debugobsolete
>> -  e72d22b19f8ecf4150ab4f91d0973fd9955d3ddf  
>> 49d44ab2be1b67a79127568a67c9c99430633b48 0 (*) {'user': 'test'} (glob)
>> -  3e30a45cf2f719e96ab3922dfe039cfd047956ce 0  
>> {e72d22b19f8ecf4150ab4f91d0973fd9955d3ddf} (*) {'user': 'test'} (glob)
>> -  1b2d564fad96311b45362f17c2aa855150efb35f  
>> 46abc7c4d8738e8563e577f7889e1b6db3da4199 0 (*) {'user': 'test'} (glob)
>> -  114f4176969ef342759a8a57e6bccefc4234829b  
>> 49d44ab2be1b67a79127568a67c9c99430633b48 0 (*) {'user': 'test'} (glob)
>> +  e72d22b19f8ecf4150ab4f91d0973fd9955d3ddf  
>> 49d44ab2be1b67a79127568a67c9c99430633b48 0 (*) {'operation': 'amend',  
>> 'user': 'test'} (glob)
>> +  3e30a45cf2f719e96ab3922dfe039cfd047956ce 0  
>> {e72d22b19f8ecf4150ab4f91d0973fd9955d3ddf} (*) {'operation': 'amend',  
>> 'user': 'test'} (glob)
>> +  1b2d564fad96311b45362f17c2aa855150efb35f  
>> 46abc7c4d8738e8563e577f7889e1b6db3da4199 0 (*) {'operation':  
>> 'histedit', 'user': 'test'} (glob)
>> +  114f4176969ef342759a8a57e6bccefc4234829b  
>> 49d44ab2be1b67a79127568a67c9c99430633b48 0 (*) {'operation':  
>> 'histedit', 'user': 'test'} (glob)
>>
>>  With some node gone missing during the edit.
>>
>> @@ -80,14 +80,14 @@ With some node gone missing during the e
>>    o  0:cb9a9f314b8b a
>>
>>    $ hg debugobsolete
>> -  e72d22b19f8ecf4150ab4f91d0973fd9955d3ddf  
>> 49d44ab2be1b67a79127568a67c9c99430633b48 0 (*) {'user': 'test'} (glob)
>> -  3e30a45cf2f719e96ab3922dfe039cfd047956ce 0  
>> {e72d22b19f8ecf4150ab4f91d0973fd9955d3ddf} (*) {'user': 'test'} (glob)
>> -  1b2d564fad96311b45362f17c2aa855150efb35f  
>> 46abc7c4d8738e8563e577f7889e1b6db3da4199 0 (*) {'user': 'test'} (glob)
>> -  114f4176969ef342759a8a57e6bccefc4234829b  
>> 49d44ab2be1b67a79127568a67c9c99430633b48 0 (*) {'user': 'test'} (glob)
>> -  76f72745eac0643d16530e56e2f86e36e40631f1  
>> 2ca853e48edbd6453a0674dc0fe28a0974c51b9c 0 (*) {'user': 'test'} (glob)
>> -  2ca853e48edbd6453a0674dc0fe28a0974c51b9c  
>> aba7da93703075eec9fb1dbaf143ff2bc1c49d46 0 (*) {'user': 'test'} (glob)
>> -  49d44ab2be1b67a79127568a67c9c99430633b48  
>> 273c1f3b86267ed3ec684bb13af1fa4d6ba56e02 0 (*) {'user': 'test'} (glob)
>> -  46abc7c4d8738e8563e577f7889e1b6db3da4199  
>> aba7da93703075eec9fb1dbaf143ff2bc1c49d46 0 (*) {'user': 'test'} (glob)
>> +  e72d22b19f8ecf4150ab4f91d0973fd9955d3ddf  
>> 49d44ab2be1b67a79127568a67c9c99430633b48 0 (*) {'operation': 'amend',  
>> 'user': 'test'} (glob)
>> +  3e30a45cf2f719e96ab3922dfe039cfd047956ce 0  
>> {e72d22b19f8ecf4150ab4f91d0973fd9955d3ddf} (*) {'operation': 'amend',  
>> 'user': 'test'} (glob)
>> +  1b2d564fad96311b45362f17c2aa855150efb35f  
>> 46abc7c4d8738e8563e577f7889e1b6db3da4199 0 (*) {'operation':  
>> 'histedit', 'user': 'test'} (glob)
>> +  114f4176969ef342759a8a57e6bccefc4234829b  
>> 49d44ab2be1b67a79127568a67c9c99430633b48 0 (*) {'operation':  
>> 'histedit', 'user': 'test'} (glob)
>> +  76f72745eac0643d16530e56e2f86e36e40631f1  
>> 2ca853e48edbd6453a0674dc0fe28a0974c51b9c 0 (*) {'operation': 'amend',  
>> 'user': 'test'} (glob)
>> +  2ca853e48edbd6453a0674dc0fe28a0974c51b9c  
>> aba7da93703075eec9fb1dbaf143ff2bc1c49d46 0 (*) {'operation': 'amend',  
>> 'user': 'test'} (glob)
>> +  49d44ab2be1b67a79127568a67c9c99430633b48  
>> 273c1f3b86267ed3ec684bb13af1fa4d6ba56e02 0 (*) {'operation':  
>> 'histedit', 'user': 'test'} (glob)
>> +  46abc7c4d8738e8563e577f7889e1b6db3da4199  
>> aba7da93703075eec9fb1dbaf143ff2bc1c49d46 0 (*) {'operation':  
>> 'histedit', 'user': 'test'} (glob)
>>    $ cd ..
>>
>>  Base setup for the rest of the testing
>> @@ -170,13 +170,13 @@ Base setup for the rest of the testing
>>    o  0:cb9a9f314b8b a
>>
>>    $ hg debugobsolete
>> -  96e494a2d553dd05902ba1cee1d94d4cb7b8faed 0  
>> {b346ab9a313db8537ecf96fca3ca3ca984ef3bd7} (*) {'user': 'test'} (glob)
>> -  b558abc46d09c30f57ac31e85a8a3d64d2e906e4 0  
>> {96e494a2d553dd05902ba1cee1d94d4cb7b8faed} (*) {'user': 'test'} (glob)
>> -  d2ae7f538514cd87c17547b0de4cea71fe1af9fb 0  
>> {cb9a9f314b8b07ba71012fcdbc544b5a4d82ff5b} (*) {'user': 'test'} (glob)
>> -  177f92b773850b59254aa5e923436f921b55483b  
>> b346ab9a313db8537ecf96fca3ca3ca984ef3bd7 0 (*) {'user': 'test'} (glob)
>> -  055a42cdd88768532f9cf79daa407fc8d138de9b  
>> 59d9f330561fd6c88b1a6b32f0e45034d88db784 0 (*) {'user': 'test'} (glob)
>> -  e860deea161a2f77de56603b340ebbb4536308ae  
>> 59d9f330561fd6c88b1a6b32f0e45034d88db784 0 (*) {'user': 'test'} (glob)
>> -  652413bf663ef2a641cab26574e46d5f5a64a55a  
>> cacdfd884a9321ec4e1de275ef3949fa953a1f83 0 (*) {'user': 'test'} (glob)
>> +  96e494a2d553dd05902ba1cee1d94d4cb7b8faed 0  
>> {b346ab9a313db8537ecf96fca3ca3ca984ef3bd7} (*) {'operation':  
>> 'histedit', 'user': 'test'} (glob)
>> +  b558abc46d09c30f57ac31e85a8a3d64d2e906e4 0  
>> {96e494a2d553dd05902ba1cee1d94d4cb7b8faed} (*) {'operation':  
>> 'histedit', 'user': 'test'} (glob)
>> +  d2ae7f538514cd87c17547b0de4cea71fe1af9fb 0  
>> {cb9a9f314b8b07ba71012fcdbc544b5a4d82ff5b} (*) {'operation':  
>> 'histedit', 'user': 'test'} (glob)
>> +  177f92b773850b59254aa5e923436f921b55483b  
>> b346ab9a313db8537ecf96fca3ca3ca984ef3bd7 0 (*) {'operation':  
>> 'histedit', 'user': 'test'} (glob)
>> +  055a42cdd88768532f9cf79daa407fc8d138de9b  
>> 59d9f330561fd6c88b1a6b32f0e45034d88db784 0 (*) {'operation':  
>> 'histedit', 'user': 'test'} (glob)
>> +  e860deea161a2f77de56603b340ebbb4536308ae  
>> 59d9f330561fd6c88b1a6b32f0e45034d88db784 0 (*) {'operation':  
>> 'histedit', 'user': 'test'} (glob)
>> +  652413bf663ef2a641cab26574e46d5f5a64a55a  
>> cacdfd884a9321ec4e1de275ef3949fa953a1f83 0 (*) {'operation':  
>> 'histedit', 'user': 'test'} (glob)
>>
>>
>>  Ensure hidden revision does not prevent histedit
>> diff --git a/tests/test-obsolete.t b/tests/test-obsolete.t
>> --- a/tests/test-obsolete.t
>> +++ b/tests/test-obsolete.t
>> @@ -1247,15 +1247,15 @@ only a subset of those are displayed (be
>>    adding d
>>    $ hg ci --amend -m dd
>>    $ hg debugobsolete --index --rev "3+7"
>> -  1 6fdef60fcbabbd3d50e9b9cbc2a240724b91a5e1  
>> d27fb9b066076fd921277a4b9e8b9cb48c95bc6a 0 \(.*\) {'user': 'test'} (re)
>> -  3 4715cf767440ed891755448016c2b8cf70760c30  
>> 7ae79c5d60f049c7b0dd02f5f25b9d60aaf7b36d 0 \(.*\) {'user': 'test'} (re)
>> +  1 6fdef60fcbabbd3d50e9b9cbc2a240724b91a5e1  
>> d27fb9b066076fd921277a4b9e8b9cb48c95bc6a 0 \(.*\) {'operation':  
>> 'amend', 'user': 'test'} (re)
>> +  3 4715cf767440ed891755448016c2b8cf70760c30  
>> 7ae79c5d60f049c7b0dd02f5f25b9d60aaf7b36d 0 \(.*\) {'operation':  
>> 'amend', 'user': 'test'} (re)
>>    $ hg debugobsolete --index --rev "3+7" -Tjson
>>    [
>>     {
>>      "date": *, (glob)
>>      "flag": 0,
>>      "index": 1,
>> -    "metadata": {"user": "test"},
>> +    "metadata": {"operation": "amend", "user": "test"},
>>      "precnode": "6fdef60fcbabbd3d50e9b9cbc2a240724b91a5e1",
>>      "succnodes": ["d27fb9b066076fd921277a4b9e8b9cb48c95bc6a"]
>>     },
>> @@ -1263,7 +1263,7 @@ only a subset of those are displayed (be
>>      "date": *, (glob)
>>      "flag": 0,
>>      "index": 3,
>> -    "metadata": {"user": "test"},
>> +    "metadata": {"operation": "amend", "user": "test"},
>>      "precnode": "4715cf767440ed891755448016c2b8cf70760c30",
>>      "succnodes": ["7ae79c5d60f049c7b0dd02f5f25b9d60aaf7b36d"]
>>     }
>> @@ -1271,14 +1271,14 @@ only a subset of those are displayed (be
>>
>>  Test the --delete option of debugobsolete command
>>    $ hg debugobsolete --index
>> -  0 cb9a9f314b8b07ba71012fcdbc544b5a4d82ff5b  
>> f9bd49731b0b175e42992a3c8fa6c678b2bc11f1 0 \(.*\) {'user': 'test'} (re)
>> -  1 6fdef60fcbabbd3d50e9b9cbc2a240724b91a5e1  
>> d27fb9b066076fd921277a4b9e8b9cb48c95bc6a 0 \(.*\) {'user': 'test'} (re)
>> -  2 1ab51af8f9b41ef8c7f6f3312d4706d870b1fb74  
>> 29346082e4a9e27042b62d2da0e2de211c027621 0 \(.*\) {'user': 'test'} (re)
>> -  3 4715cf767440ed891755448016c2b8cf70760c30  
>> 7ae79c5d60f049c7b0dd02f5f25b9d60aaf7b36d 0 \(.*\) {'user': 'test'} (re)
>> +  0 cb9a9f314b8b07ba71012fcdbc544b5a4d82ff5b  
>> f9bd49731b0b175e42992a3c8fa6c678b2bc11f1 0 \(.*\) {'operation':  
>> 'amend', 'user': 'test'} (re)
>> +  1 6fdef60fcbabbd3d50e9b9cbc2a240724b91a5e1  
>> d27fb9b066076fd921277a4b9e8b9cb48c95bc6a 0 \(.*\) {'operation':  
>> 'amend', 'user': 'test'} (re)
>> +  2 1ab51af8f9b41ef8c7f6f3312d4706d870b1fb74  
>> 29346082e4a9e27042b62d2da0e2de211c027621 0 \(.*\) {'operation':  
>> 'amend', 'user': 'test'} (re)
>> +  3 4715cf767440ed891755448016c2b8cf70760c30  
>> 7ae79c5d60f049c7b0dd02f5f25b9d60aaf7b36d 0 \(.*\) {'operation':  
>> 'amend', 'user': 'test'} (re)
>>    $ hg debugobsolete --delete 1 --delete 3
>>    deleted 2 obsolescence markers
>>    $ hg debugobsolete
>> -  cb9a9f314b8b07ba71012fcdbc544b5a4d82ff5b  
>> f9bd49731b0b175e42992a3c8fa6c678b2bc11f1 0 \(.*\) {'user': 'test'} (re)
>> -  1ab51af8f9b41ef8c7f6f3312d4706d870b1fb74  
>> 29346082e4a9e27042b62d2da0e2de211c027621 0 \(.*\) {'user': 'test'} (re)
>> +  cb9a9f314b8b07ba71012fcdbc544b5a4d82ff5b  
>> f9bd49731b0b175e42992a3c8fa6c678b2bc11f1 0 \(.*\) {'operation':  
>> 'amend', 'user': 'test'} (re)
>> +  1ab51af8f9b41ef8c7f6f3312d4706d870b1fb74  
>> 29346082e4a9e27042b62d2da0e2de211c027621 0 \(.*\) {'operation':  
>> 'amend', 'user': 'test'} (re)
>>    $ cd ..
>>
>> diff --git a/tests/test-rebase-obsolete.t b/tests/test-rebase-obsolete.t
>> --- a/tests/test-rebase-obsolete.t
>> +++ b/tests/test-rebase-obsolete.t
>> @@ -100,9 +100,9 @@ simple rebase
>>    o  0:cd010b8cd998 A
>>
>>    $ hg debugobsolete
>> -  42ccdea3bb16d28e1848c95fe2e44c000f3f21b1  
>> e4e5be0395b2cbd471ed22a26b1b6a1a0658a794 0 (*) {'user': 'test'} (glob)
>> -  5fddd98957c8a54a4d436dfe1da9d87f21a1b97b  
>> 2327fea05063f39961b14cb69435a9898dc9a245 0 (*) {'user': 'test'} (glob)
>> -  32af7686d403cf45b5d95f2d70cebea587ac806a  
>> 8eeb3c33ad33d452c89e5dcf611c347f978fb42b 0 (*) {'user': 'test'} (glob)
>> +  42ccdea3bb16d28e1848c95fe2e44c000f3f21b1  
>> e4e5be0395b2cbd471ed22a26b1b6a1a0658a794 0 (*) {'operation': 'rebase',  
>> 'user': 'test'} (glob)
>> +  5fddd98957c8a54a4d436dfe1da9d87f21a1b97b  
>> 2327fea05063f39961b14cb69435a9898dc9a245 0 (*) {'operation': 'rebase',  
>> 'user': 'test'} (glob)
>> +  32af7686d403cf45b5d95f2d70cebea587ac806a  
>> 8eeb3c33ad33d452c89e5dcf611c347f978fb42b 0 (*) {'operation': 'rebase',  
>> 'user': 'test'} (glob)
>>
>>
>>    $ cd ..
>> @@ -170,9 +170,9 @@ set.
>>    o  0:cd010b8cd998 A
>>
>>    $ hg debugobsolete
>> -  42ccdea3bb16d28e1848c95fe2e44c000f3f21b1 0  
>> {cd010b8cd998f3981a5a8115f94f8da4ab506089} (*) {'user': 'test'} (glob)
>> -  5fddd98957c8a54a4d436dfe1da9d87f21a1b97b  
>> 5ae4c968c6aca831df823664e706c9d4aa34473d 0 (*) {'user': 'test'} (glob)
>> -  32af7686d403cf45b5d95f2d70cebea587ac806a 0  
>> {5fddd98957c8a54a4d436dfe1da9d87f21a1b97b} (*) {'user': 'test'} (glob)
>> +  42ccdea3bb16d28e1848c95fe2e44c000f3f21b1 0  
>> {cd010b8cd998f3981a5a8115f94f8da4ab506089} (*) {'operation': 'rebase',  
>> 'user': 'test'} (glob)
>> +  5fddd98957c8a54a4d436dfe1da9d87f21a1b97b  
>> 5ae4c968c6aca831df823664e706c9d4aa34473d 0 (*) {'operation': 'rebase',  
>> 'user': 'test'} (glob)
>> +  32af7686d403cf45b5d95f2d70cebea587ac806a 0  
>> {5fddd98957c8a54a4d436dfe1da9d87f21a1b97b} (*) {'operation': 'rebase',  
>> 'user': 'test'} (glob)
>>
>>
>>  More complex case where part of the rebase set were already rebased
>> @@ -180,10 +180,10 @@ More complex case where part of the reba
>>    $ hg rebase --rev 'desc(D)' --dest 'desc(H)'
>>    rebasing 9:08483444fef9 "D"
>>    $ hg debugobsolete
>> -  42ccdea3bb16d28e1848c95fe2e44c000f3f21b1 0  
>> {cd010b8cd998f3981a5a8115f94f8da4ab506089} (*) {'user': 'test'} (glob)
>> -  5fddd98957c8a54a4d436dfe1da9d87f21a1b97b  
>> 5ae4c968c6aca831df823664e706c9d4aa34473d 0 (*) {'user': 'test'} (glob)
>> -  32af7686d403cf45b5d95f2d70cebea587ac806a 0  
>> {5fddd98957c8a54a4d436dfe1da9d87f21a1b97b} (*) {'user': 'test'} (glob)
>> -  08483444fef91d6224f6655ee586a65d263ad34c  
>> 4596109a6a4328c398bde3a4a3b6737cfade3003 0 (*) {'user': 'test'} (glob)
>> +  42ccdea3bb16d28e1848c95fe2e44c000f3f21b1 0  
>> {cd010b8cd998f3981a5a8115f94f8da4ab506089} (*) {'operation': 'rebase',  
>> 'user': 'test'} (glob)
>> +  5fddd98957c8a54a4d436dfe1da9d87f21a1b97b  
>> 5ae4c968c6aca831df823664e706c9d4aa34473d 0 (*) {'operation': 'rebase',  
>> 'user': 'test'} (glob)
>> +  32af7686d403cf45b5d95f2d70cebea587ac806a 0  
>> {5fddd98957c8a54a4d436dfe1da9d87f21a1b97b} (*) {'operation': 'rebase',  
>> 'user': 'test'} (glob)
>> +  08483444fef91d6224f6655ee586a65d263ad34c  
>> 4596109a6a4328c398bde3a4a3b6737cfade3003 0 (*) {'operation': 'rebase',  
>> 'user': 'test'} (glob)
>>    $ hg log -G
>>    @  11:4596109a6a43 D
>>    |
>> @@ -208,12 +208,12 @@ More complex case where part of the reba
>>    note: not rebasing 9:08483444fef9 "D", already in destination as  
>> 11:4596109a6a43 "D"
>>    rebasing 10:5ae4c968c6ac "C"
>>    $ hg debugobsolete
>> -  42ccdea3bb16d28e1848c95fe2e44c000f3f21b1 0  
>> {cd010b8cd998f3981a5a8115f94f8da4ab506089} (*) {'user': 'test'} (glob)
>> -  5fddd98957c8a54a4d436dfe1da9d87f21a1b97b  
>> 5ae4c968c6aca831df823664e706c9d4aa34473d 0 (*) {'user': 'test'} (glob)
>> -  32af7686d403cf45b5d95f2d70cebea587ac806a 0  
>> {5fddd98957c8a54a4d436dfe1da9d87f21a1b97b} (*) {'user': 'test'} (glob)
>> -  08483444fef91d6224f6655ee586a65d263ad34c  
>> 4596109a6a4328c398bde3a4a3b6737cfade3003 0 (*) {'user': 'test'} (glob)
>> -  8877864f1edb05d0e07dc4ba77b67a80a7b86672  
>> 462a34d07e599b87ea08676a449373fe4e2e1347 0 (*) {'user': 'test'} (glob)
>> -  5ae4c968c6aca831df823664e706c9d4aa34473d  
>> 98f6af4ee9539e14da4465128f894c274900b6e5 0 (*) {'user': 'test'} (glob)
>> +  42ccdea3bb16d28e1848c95fe2e44c000f3f21b1 0  
>> {cd010b8cd998f3981a5a8115f94f8da4ab506089} (*) {'operation': 'rebase',  
>> 'user': 'test'} (glob)
>> +  5fddd98957c8a54a4d436dfe1da9d87f21a1b97b  
>> 5ae4c968c6aca831df823664e706c9d4aa34473d 0 (*) {'operation': 'rebase',  
>> 'user': 'test'} (glob)
>> +  32af7686d403cf45b5d95f2d70cebea587ac806a 0  
>> {5fddd98957c8a54a4d436dfe1da9d87f21a1b97b} (*) {'operation': 'rebase',  
>> 'user': 'test'} (glob)
>> +  08483444fef91d6224f6655ee586a65d263ad34c  
>> 4596109a6a4328c398bde3a4a3b6737cfade3003 0 (*) {'operation': 'rebase',  
>> 'user': 'test'} (glob)
>> +  8877864f1edb05d0e07dc4ba77b67a80a7b86672  
>> 462a34d07e599b87ea08676a449373fe4e2e1347 0 (*) {'operation': 'rebase',  
>> 'user': 'test'} (glob)
>> +  5ae4c968c6aca831df823664e706c9d4aa34473d  
>> 98f6af4ee9539e14da4465128f894c274900b6e5 0 (*) {'operation': 'rebase',  
>> 'user': 'test'} (glob)
>>    $ hg log --rev 'divergent()'
>>    $ hg log -G
>>    o  13:98f6af4ee953 C
>> @@ -349,9 +349,9 @@ collapse rebase
>>    $ hg id --debug -r tip
>>    4dc2197e807bae9817f09905b50ab288be2dbbcf tip
>>    $ hg debugobsolete
>> -  42ccdea3bb16d28e1848c95fe2e44c000f3f21b1  
>> 4dc2197e807bae9817f09905b50ab288be2dbbcf 0 (*) {'user': 'test'} (glob)
>> -  5fddd98957c8a54a4d436dfe1da9d87f21a1b97b  
>> 4dc2197e807bae9817f09905b50ab288be2dbbcf 0 (*) {'user': 'test'} (glob)
>> -  32af7686d403cf45b5d95f2d70cebea587ac806a  
>> 4dc2197e807bae9817f09905b50ab288be2dbbcf 0 (*) {'user': 'test'} (glob)
>> +  42ccdea3bb16d28e1848c95fe2e44c000f3f21b1  
>> 4dc2197e807bae9817f09905b50ab288be2dbbcf 0 (*) {'operation': 'rebase',  
>> 'user': 'test'} (glob)
>> +  5fddd98957c8a54a4d436dfe1da9d87f21a1b97b  
>> 4dc2197e807bae9817f09905b50ab288be2dbbcf 0 (*) {'operation': 'rebase',  
>> 'user': 'test'} (glob)
>> +  32af7686d403cf45b5d95f2d70cebea587ac806a  
>> 4dc2197e807bae9817f09905b50ab288be2dbbcf 0 (*) {'operation': 'rebase',  
>> 'user': 'test'} (glob)
>>
>>    $ cd ..
>>
>> @@ -411,9 +411,9 @@ not be rebased.
>>    o  0:cd010b8cd998 A
>>
>>    $ hg debugobsolete
>> -  5fddd98957c8a54a4d436dfe1da9d87f21a1b97b  
>> e273c5e7d2d29df783dce9f9eaa3ac4adc69c15d 0 (*) {'user': 'test'} (glob)
>> -  32af7686d403cf45b5d95f2d70cebea587ac806a  
>> cf44d2f5a9f4297a62be94cbdd3dff7c7dc54258 0 (*) {'user': 'test'} (glob)
>> -  42ccdea3bb16d28e1848c95fe2e44c000f3f21b1  
>> 7c6027df6a99d93f461868e5433f63bde20b6dfb 0 (*) {'user': 'test'} (glob)
>> +  5fddd98957c8a54a4d436dfe1da9d87f21a1b97b  
>> e273c5e7d2d29df783dce9f9eaa3ac4adc69c15d 0 (*) {'operation': 'rebase',  
>> 'user': 'test'} (glob)
>> +  32af7686d403cf45b5d95f2d70cebea587ac806a  
>> cf44d2f5a9f4297a62be94cbdd3dff7c7dc54258 0 (*) {'operation': 'rebase',  
>> 'user': 'test'} (glob)
>> +  42ccdea3bb16d28e1848c95fe2e44c000f3f21b1  
>> 7c6027df6a99d93f461868e5433f63bde20b6dfb 0 (*) {'operation': 'rebase',  
>> 'user': 'test'} (glob)
>>
>>  Test that rewriting leaving instability behind is allowed
>>  ---------------------------------------------------------------------
>> _______________________________________________
>> Mercurial-devel mailing list
>> Mercurial-devel@mercurial-scm.org
>> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Gregory Szorc - May 13, 2017, 9:16 p.m.
On Wed, May 10, 2017 at 1:08 PM, Pierre-Yves David <
pierre-yves.david@ens-lyon.org> wrote:

> On 05/10/2017 05:45 PM, Durham Goode wrote:
>
>> On 5/10/17 3:40 AM, Pierre-Yves David wrote:
>>
>>> On 05/10/2017 01:34 AM, Durham Goode wrote:
>>>
>>>> # HG changeset patch
>>>> # User Durham Goode <durham@fb.com>
>>>> # Date 1494372571 25200
>>>> #      Tue May 09 16:29:31 2017 -0700
>>>> # Node ID 22051b0924bcbc5cde6d25d700dcce6afcaafd98
>>>> # Parent  8f1a2b848b52ea7bf3fe2404e3b62924c7aae93f
>>>> obsolete: add operation metadata to rebase/amend/histedit obsmarkers
>>>>
>>>
>>> TL;DR; Storing more information is useful, but the pre-existing plans
>>> aims at storing slightly different data in a different way. We should
>>> stick to it.
>>>
>>> The idea is to use the obsmarker bitfields to store the data and record
>>> a pre-defined set of "effects" in each markers. (eg: "content change,
>>> message change, parent change")
>>>
>>> For examples:
>>>
>>> 1) a `hg amend` that update the diff will create a markers recording the
>>> "content changes".
>>>
>>> 2) a `hg amend -e` that only update he commit message will record the
>>> "message change".
>>>
>>> 3) a `hg rebase` will record the "parent change".
>>>
>>> 4) a `hg amend` that update the message, the content and user will
>>> record these three things (content, message and meta changes).
>>>
>>> These data can then be used to have nice template as shown by Durham
>>> (eg: is X is a successors of Y and the markers as some "content change"
>>> bit, we can display "amended as Y"; if their are some parent change, we
>>> can display "rebased as Y", if both are present "amended and rebased as
>>> Y").
>>>
>>>
>> While having the above data seems nice, I don't know if it justifies not
>> going with the current simple approach today. Otherwise we have to wait
>> for this large obsstore refactor, which seems like a rather big
>> dependency to block an important UX feature we could deliver today.
>>
>
> There seems to be a major misunderstanding here.
>
> Obsmarkers -already- has a bitfield (for many years) so are -already- able
> to use them to record actions. There are no delay or blocker here
> everything is already in place for it. There are basically no reason to not
> just do it.
>

This is true. And nobody is denying that using bit flags is more optimal.

However, since we're not sure what we're doing with this feature yet,
defining strings in the open-ended metadata field and then optimizing
common values as bit flags later (possibly before 4.3 release) seems like a
reasonable approach.


>
> When the bit field is added in the future we can always change our
>> templates and messaging to use that data as well.  And if it's good
>> enough, we can drop the 'operation' metadata entirely, with little
>> compatibility issue.
>>
>> The above approach as multiple advantages:
>>>
>>>   Compact: Since we use the (existing) bitfield, the storage cost is
>>> free (or minimal if we need to adds an extra byte of bitfield).
>>> Accessing the data is also very cheap:
>>>
>>>   Compoundable: We'll often needs to combine multiple markers to reports
>>> changesets between two revisions (eg: we do not have the intermediate
>>> revisions locally). the bit field approach makes it trivial to compound
>>> the information. We can display display the same final information when
>>> the same result if obtained from different path (eg: from two markers
>>> [content change; message change] or from one markers [content change |
>>> message change]).
>>>   This part is important because simply using operation name makes it
>>> quite hard to combine the information.
>>>
>>
>> Yes, if there were multiple operations we'd have to union them just as
>> operation='modified' or something. Not ideal, but should be an outlier
>> case.
>>
>
> Having multiple markers between local changesets is the common case once
> people start exchanging draft changesets. We can not dismiss this as an
> outliers, behaving properly here is an actual request from the current
> users.
>

I don't think this matters today. Perfect is the enemy of good. We can also
hack around this other ways, such as having the presence of multiple
markers obsoleting a specific changeset via different methods change the UI.


>
>   UI Abstraction: since we record the "effect" information, we abstract
>>> the command names. This makes use more agile about the actual UI. User
>>> can use their own "hg myownrewritetool" command and still record
>>> information useful for the other users.
>>>
>>
>> I think recording the actual command the user ran is important. The goal
>> of showing this in the UI is to let the user know what past action
>> caused this commit to become a new commit, so if they run 'hg
>> flabbernate' it's much more useful to show them 'fblabbernated to be X'
>> than it is to show them 'amended to be X'. So potentially, even if we
>> have the bitfield data, it could still be important to record the actual
>> command that caused this obsmarker to be created (ex: "amended to be X
>> by hg flabbernate")
>>
>
> Having more information is always useful, but this looks like something to
> add as a second step.
>
> The action-bit approach has clear advantages (no size concerns, clear
> behavior with marker compositions), so this appears as a clear good first
> step here.
>
> In addition, if that first step turn out to not be enough and we want more
> precise data we might want to look at leveraging the "hg journal" local
> data for this.
>
>
> I would recommend the following step forward:
>
> 1) add a simple "rewritten as" template,
> 2) record action-bit, use it in the template (and other related output),
> 3) gather data and look at the best way to issue more details.
>

Since we're not sure what we're going to do yet, wasting limited bit flags
when we have an open-ended metadata field is premature optimization. I
support the approach in this patch. I also support converting things to bit
flags before 4.3 if we establish confidence that the feature is working how
we'd like.


>
> By chance step (1) and (2) are on the list of UI improvement founded by
> Unity (1) is about to be done (read: next week) and (2) is likely to be
> scheduled in the next batch (read, this month).
>
> If you want to work in this area, I recommend catching up with the recent
> improvement in evolve and getting in touch with Boris Feld (currently
> writing doing most of the lifting in this area).
>
> Cheers,
>
> --
> Pierre-Yves David
>
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
>
Augie Fackler - May 15, 2017, 2:39 p.m.
> On May 13, 2017, at 17:16, Gregory Szorc <gregory.szorc@gmail.com> wrote:
> 
>> I would recommend the following step forward:
>> 
>> 1) add a simple "rewritten as" template,
>> 2) record action-bit, use it in the template (and other related output),
>> 3) gather data and look at the best way to issue more details.
>> 
> Since we're not sure what we're going to do yet, wasting limited bit flags when we have an open-ended metadata field is premature optimization. I support the approach in this patch. I also support converting things to bit flags before 4.3 if we establish confidence that the feature is working how we'd like.

This sounds like a great approach to me - I share your hesitance around spending the limited bits in the flag field prematurely.
Pierre-Yves David - May 18, 2017, 8:46 a.m.
TL;DR; There seems to be significant confusion remaining around the goal 
and constraint of what we aims to do here. The email discussion is 
moving slowly, I think we should switch to a more synchronous medium.

On 05/13/2017 11:16 PM, Gregory Szorc wrote:
> On Wed, May 10, 2017 at 1:08 PM, Pierre-Yves David
> <pierre-yves.david@ens-lyon.org <mailto:pierre-yves.david@ens-lyon.org>>
> wrote:
>
>     On 05/10/2017 05:45 PM, Durham Goode wrote:
>
>         On 5/10/17 3:40 AM, Pierre-Yves David wrote:
>
>             On 05/10/2017 01:34 AM, Durham Goode wrote:
>
>                 # HG changeset patch
>                 # User Durham Goode <durham@fb.com <mailto:durham@fb.com>>
>                 # Date 1494372571 25200
>                 #      Tue May 09 16:29:31 2017 -0700
>                 # Node ID 22051b0924bcbc5cde6d25d700dcce6afcaafd98
>                 # Parent  8f1a2b848b52ea7bf3fe2404e3b62924c7aae93f
>                 obsolete: add operation metadata to
>                 rebase/amend/histedit obsmarkers
>
>
>             TL;DR; Storing more information is useful, but the
>             pre-existing plans
>             aims at storing slightly different data in a different way.
>             We should
>             stick to it.
>
>             The idea is to use the obsmarker bitfields to store the data
>             and record
>             a pre-defined set of "effects" in each markers. (eg:
>             "content change,
>             message change, parent change")
>
>             For examples:
>
>             1) a `hg amend` that update the diff will create a markers
>             recording the
>             "content changes".
>
>             2) a `hg amend -e` that only update he commit message will
>             record the
>             "message change".
>
>             3) a `hg rebase` will record the "parent change".
>
>             4) a `hg amend` that update the message, the content and
>             user will
>             record these three things (content, message and meta changes).
>
>             These data can then be used to have nice template as shown
>             by Durham
>             (eg: is X is a successors of Y and the markers as some
>             "content change"
>             bit, we can display "amended as Y"; if their are some parent
>             change, we
>             can display "rebased as Y", if both are present "amended and
>             rebased as
>             Y").
>
>
>         While having the above data seems nice, I don't know if it
>         justifies not
>         going with the current simple approach today. Otherwise we have
>         to wait
>         for this large obsstore refactor, which seems like a rather big
>         dependency to block an important UX feature we could deliver today.
>
>
>     There seems to be a major misunderstanding here.
>
>     Obsmarkers -already- has a bitfield (for many years) so are
>     -already- able to use them to record actions. There are no delay or
>     blocker here everything is already in place for it. There are
>     basically no reason to not just do it.
>
>
> This is true. And nobody is denying that using bit flags is more optimal.
>
> However, since we're not sure what we're doing with this feature yet,
> defining strings in the open-ended metadata field and then optimizing
> common values as bit flags later (possibly before 4.3 release) seems
> like a reasonable approach.

Using bit is not just about optimizing size for common values, it is 
about recording more usable value too. Command name cannot be combined 
(something we know we need) and has trouble carrying information across 
users using different interfaces (something already happening today).

The planned alternative (recording "effect" on operation) does not 
suffer from these two issues and are better suited for providing good 
information for team using evolution.

>         When the bit field is added in the future we can always change our
>         templates and messaging to use that data as well.  And if it's good
>         enough, we can drop the 'operation' metadata entirely, with little
>         compatibility issue.
>
>             The above approach as multiple advantages:
>
>               Compact: Since we use the (existing) bitfield, the storage
>             cost is
>             free (or minimal if we need to adds an extra byte of bitfield).
>             Accessing the data is also very cheap:
>
>               Compoundable: We'll often needs to combine multiple
>             markers to reports
>             changesets between two revisions (eg: we do not have the
>             intermediate
>             revisions locally). the bit field approach makes it trivial
>             to compound
>             the information. We can display display the same final
>             information when
>             the same result if obtained from different path (eg: from
>             two markers
>             [content change; message change] or from one markers
>             [content change |
>             message change]).
>               This part is important because simply using operation name
>             makes it
>             quite hard to combine the information.
>
>
>         Yes, if there were multiple operations we'd have to union them
>         just as
>         operation='modified' or something. Not ideal, but should be an
>         outlier
>         case.
>
>
>     Having multiple markers between local changesets is the common case
>     once people start exchanging draft changesets. We can not dismiss
>     this as an outliers, behaving properly here is an actual request
>     from the current users.
>
>
> I don't think this matters today. Perfect is the enemy of good. We can
> also hack around this other ways, such as having the presence of
> multiple markers obsoleting a specific changeset via different methods
> change the UI.

I'm a bit confused by this part of your reply, I'm not sure how to read 
it. Can you try to rephrase it to clarify again?


We know from field data and users request that it common to have a 
linear chain of markers between locally known changeset (chain involving 
locally unknown node). And team using evolution do requests better 
reporting in this case.

However, the part in your reply about "presence of multiple markers 
obsoleting a specific changeset" makes me think you are actually talking 
about something else.

>               UI Abstraction: since we record the "effect" information,
>             we abstract
>             the command names. This makes use more agile about the
>             actual UI. User
>             can use their own "hg myownrewritetool" command and still record
>             information useful for the other users.
>
>
>         I think recording the actual command the user ran is important.
>         The goal
>         of showing this in the UI is to let the user know what past action
>         caused this commit to become a new commit, so if they run 'hg
>         flabbernate' it's much more useful to show them 'fblabbernated
>         to be X'
>         than it is to show them 'amended to be X'. So potentially, even
>         if we
>         have the bitfield data, it could still be important to record
>         the actual
>         command that caused this obsmarker to be created (ex: "amended
>         to be X
>         by hg flabbernate")
>
>
>     Having more information is always useful, but this looks like
>     something to add as a second step.
>
>     The action-bit approach has clear advantages (no size concerns,
>     clear behavior with marker compositions), so this appears as a clear
>     good first step here.
>
>     In addition, if that first step turn out to not be enough and we
>     want more precise data we might want to look at leveraging the "hg
>     journal" local data for this.
>
>
>     I would recommend the following step forward:
>
>     1) add a simple "rewritten as" template,
>     2) record action-bit, use it in the template (and other related output),
>     3) gather data and look at the best way to issue more details.
>
>
> Since we're not sure what we're going to do yet, wasting limited bit
> flags when we have an open-ended metadata field is premature
> optimization. I support the approach in this patch. I also support
> converting things to bit flags before 4.3 if we establish confidence
> that the feature is working how we'd like.

We have to keep in mind that the total size of markers is an important 
factor here. There are already useful information excluded from markers 
for size reason (eg: parent information for non-prune). We have to keep 
being careful here and weight what extra data is the most important. 
Adding command name would increase markers size by tens of percent. 
Especially since markers will live forever in the project once created.

If exhausting bit flag is a concern, we can easily extend this bit 
field. For now, we could store this extension in the metadata mapping, 
and we would store them as an extra byte in the next obsstore format 
(that we'll need soon at least for hash agility anyway).
We could easily dedicate a whole byte to store the proposed action bit 
(and store it the marker extra for now, that would help adjusting bit 
value over time).

Cheers,
Augie Fackler - May 18, 2017, 1:18 p.m.
> On May 18, 2017, at 04:46, Pierre-Yves David <pierre-yves.david@ens-lyon.org> wrote:
> 
>>>    I would recommend the following step forward:
>>> 
>>>    1) add a simple "rewritten as" template,
>>>    2) record action-bit, use it in the template (and other related output),
>>>    3) gather data and look at the best way to issue more details.
>>> 
>> 
>> Since we're not sure what we're going to do yet, wasting limited bit
>> flags when we have an open-ended metadata field is premature
>> optimization. I support the approach in this patch. I also support
>> converting things to bit flags before 4.3 if we establish confidence
>> that the feature is working how we'd like.
> 
> We have to keep in mind that the total size of markers is an important factor here. There are already useful information excluded from markers for size reason (eg: parent information for non-prune). We have to keep being careful here and weight what extra data is the most important. Adding command name would increase markers size by tens of percent. Especially since markers will live forever in the project once created.

I understand that, but I think storing the command name is worthwhile as an experiment. I'd like us to be better about letting the perfect be the enemy of the good. I think we should go ahead and land this patch as-is, and if it turns out to be massively unwise we can back it out before 4.3.

Does anyone (other than Pierre-Yves, who has made his position very clear) object (strongly or weakly) to moving forward with this patch as stated?

Thanks!
Augie

> If exhausting bit flag is a concern, we can easily extend this bit field. For now, we could store this extension in the metadata mapping, and we would store them as an extra byte in the next obsstore format (that we'll need soon at least for hash agility anyway).
> We could easily dedicate a whole byte to store the proposed action bit (and store it the marker extra for now, that would help adjusting bit value over time).
via Mercurial-devel - May 19, 2017, 4:39 p.m.
On Thu, May 18, 2017 at 6:18 AM, Augie Fackler <raf@durin42.com> wrote:
>
>> On May 18, 2017, at 04:46, Pierre-Yves David <pierre-yves.david@ens-lyon.org> wrote:
>>
>>>>    I would recommend the following step forward:
>>>>
>>>>    1) add a simple "rewritten as" template,
>>>>    2) record action-bit, use it in the template (and other related output),
>>>>    3) gather data and look at the best way to issue more details.
>>>>
>>>
>>> Since we're not sure what we're going to do yet, wasting limited bit
>>> flags when we have an open-ended metadata field is premature
>>> optimization. I support the approach in this patch. I also support
>>> converting things to bit flags before 4.3 if we establish confidence
>>> that the feature is working how we'd like.
>>
>> We have to keep in mind that the total size of markers is an important factor here. There are already useful information excluded from markers for size reason (eg: parent information for non-prune). We have to keep being careful here and weight what extra data is the most important. Adding command name would increase markers size by tens of percent. Especially since markers will live forever in the project once created.
>
> I understand that, but I think storing the command name is worthwhile as an experiment. I'd like us to be better about letting the perfect be the enemy of the good. I think we should go ahead and land this patch as-is, and if it turns out to be massively unwise we can back it out before 4.3.
>
> Does anyone (other than Pierre-Yves, who has made his position very clear) object (strongly or weakly) to moving forward with this patch as stated?

I agree with Pierre-Yves that the bit-based solution seems better
long-term. I'm not particularly worried about wasting bits. I was also
happy for Durham's patch as a short-term solution. But since
Pierre-Yves et al are already working on the bit-based solution, I'd
prefer to give them at least a month to make progress on that.


>
> Thanks!
> Augie
>
>> If exhausting bit flag is a concern, we can easily extend this bit field. For now, we could store this extension in the metadata mapping, and we would store them as an extra byte in the next obsstore format (that we'll need soon at least for hash agility anyway).
>> We could easily dedicate a whole byte to store the proposed action bit (and store it the marker extra for now, that would help adjusting bit value over time).
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Gregory Szorc - May 19, 2017, 4:51 p.m.
> On May 19, 2017, at 09:39, Martin von Zweigbergk <martinvonz@google.com> wrote:
> 
>> On Thu, May 18, 2017 at 6:18 AM, Augie Fackler <raf@durin42.com> wrote:
>> 
>>> On May 18, 2017, at 04:46, Pierre-Yves David <pierre-yves.david@ens-lyon.org> wrote:
>>> 
>>>>>   I would recommend the following step forward:
>>>>> 
>>>>>   1) add a simple "rewritten as" template,
>>>>>   2) record action-bit, use it in the template (and other related output),
>>>>>   3) gather data and look at the best way to issue more details.
>>>>> 
>>>> 
>>>> Since we're not sure what we're going to do yet, wasting limited bit
>>>> flags when we have an open-ended metadata field is premature
>>>> optimization. I support the approach in this patch. I also support
>>>> converting things to bit flags before 4.3 if we establish confidence
>>>> that the feature is working how we'd like.
>>> 
>>> We have to keep in mind that the total size of markers is an important factor here. There are already useful information excluded from markers for size reason (eg: parent information for non-prune). We have to keep being careful here and weight what extra data is the most important. Adding command name would increase markers size by tens of percent. Especially since markers will live forever in the project once created.
>> 
>> I understand that, but I think storing the command name is worthwhile as an experiment. I'd like us to be better about letting the perfect be the enemy of the good. I think we should go ahead and land this patch as-is, and if it turns out to be massively unwise we can back it out before 4.3.
>> 
>> Does anyone (other than Pierre-Yves, who has made his position very clear) object (strongly or weakly) to moving forward with this patch as stated?
> 
> I agree with Pierre-Yves that the bit-based solution seems better
> long-term. I'm not particularly worried about wasting bits. I was also
> happy for Durham's patch as a short-term solution. But since
> Pierre-Yves et al are already working on the bit-based solution, I'd
> prefer to give them at least a month to make progress on that.

Perfect is the enemy of done.

If we're really uncomfortable with the strings, we can put that behind an experimental config or requirement so we don't pollute real repos. Then we can figure out something before 4.3. At least this way something lands and some progress is made (even if the exact implementation doesn't pan out).

> 
> 
>> 
>> Thanks!
>> Augie
>> 
>>> If exhausting bit flag is a concern, we can easily extend this bit field. For now, we could store this extension in the metadata mapping, and we would store them as an extra byte in the next obsstore format (that we'll need soon at least for hash agility anyway).
>>> We could easily dedicate a whole byte to store the proposed action bit (and store it the marker extra for now, that would help adjusting bit value over time).
>> _______________________________________________
>> Mercurial-devel mailing list
>> Mercurial-devel@mercurial-scm.org
>> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Jun Wu - May 19, 2017, 4:54 p.m.
Excerpts from Martin von Zweigbergk's message of 2017-05-19 09:39:41 -0700:
> I agree with Pierre-Yves that the bit-based solution seems better
> long-term. I'm not particularly worried about wasting bits. I was also
> happy for Durham's patch as a short-term solution. But since
> Pierre-Yves et al are already working on the bit-based solution, I'd
> prefer to give them at least a month to make progress on that.

I think the bits do not contain enough information. Consider "absorb", it's
"content-change" so would you show "amend as" or "absorb as"? There are
other content-rewriting commands that are not "amend".

The obsstore will have a format change to support hash-preserving (I'll try
to get some plans public in the near future) and I plan to de-duplicate all
strings stored there so space usage is less a concern.

Since the metadata contains more information and space usage will be
addressed in the next format (which will also dedup "user" metadata). I
don't think the flag idea should block the metadata change.
via Mercurial-devel - May 19, 2017, 5:11 p.m.
On Fri, May 19, 2017 at 9:54 AM, Jun Wu <quark@fb.com> wrote:
> Excerpts from Martin von Zweigbergk's message of 2017-05-19 09:39:41 -0700:
>> I agree with Pierre-Yves that the bit-based solution seems better
>> long-term. I'm not particularly worried about wasting bits. I was also
>> happy for Durham's patch as a short-term solution. But since
>> Pierre-Yves et al are already working on the bit-based solution, I'd
>> prefer to give them at least a month to make progress on that.
>
> I think the bits do not contain enough information. Consider "absorb", it's
> "content-change" so would you show "amend as" or "absorb as"? There are
> other content-rewriting commands that are not "amend".
>
> The obsstore will have a format change to support hash-preserving (I'll try
> to get some plans public in the near future) and I plan to de-duplicate all
> strings stored there so space usage is less a concern.
>
> Since the metadata contains more information and space usage will be
> addressed in the next format (which will also dedup "user" metadata). I
> don't think the flag idea should block the metadata change.

If we are not concerned about space, having both the bits and the
strings sounds good, I agree.
Jun Wu - May 19, 2017, 5:17 p.m.
Excerpts from Gregory Szorc's message of 2017-05-19 09:51:46 -0700:
> > I agree with Pierre-Yves that the bit-based solution seems better
> > long-term. I'm not particularly worried about wasting bits. I was also
> > happy for Durham's patch as a short-term solution. But since
> > Pierre-Yves et al are already working on the bit-based solution, I'd
> > prefer to give them at least a month to make progress on that.
> 
> Perfect is the enemy of done.
> 
> If we're really uncomfortable with the strings, we can put that behind an
> experimental config or requirement so we don't pollute real repos. Then we
> can figure out something before 4.3. At least this way something lands and
> some progress is made (even if the exact implementation doesn't pan out).

Talking about perfection, I also don't think the 3 bits: parent-change,
content-change and metadata-change is a good abstraction.

Because they are redundant data. They can be calculated afterwards, in a
cache. Having them in obsstore means we could have inconsistency - changelog
disagrees with obsstore about the flags (data written by some bad
extensions) - how would you deal with that?

"operation" metadata is not redundant - you cannot get them from elsewhere
so they should be recorded.
Augie Fackler - May 19, 2017, 5:22 p.m.
> On May 19, 2017, at 10:17, Jun Wu <quark@fb.com> wrote:
> 
> Excerpts from Gregory Szorc's message of 2017-05-19 09:51:46 -0700:
>>> I agree with Pierre-Yves that the bit-based solution seems better
>>> long-term. I'm not particularly worried about wasting bits. I was also
>>> happy for Durham's patch as a short-term solution. But since
>>> Pierre-Yves et al are already working on the bit-based solution, I'd
>>> prefer to give them at least a month to make progress on that.
>> 
>> Perfect is the enemy of done.
>> 
>> If we're really uncomfortable with the strings, we can put that behind an
>> experimental config or requirement so we don't pollute real repos. Then we
>> can figure out something before 4.3. At least this way something lands and
>> some progress is made (even if the exact implementation doesn't pan out).
> 
> Talking about perfection, I also don't think the 3 bits: parent-change,
> content-change and metadata-change is a good abstraction.
> 
> Because they are redundant data. They can be calculated afterwards, in a
> cache. Having them in obsstore means we could have inconsistency - changelog
> disagrees with obsstore about the flags (data written by some bad
> extensions) - how would you deal with that?
> 
> "operation" metadata is not redundant - you cannot get them from elsewhere
> so they should be recorded.

I'm satisfied at this point in the thread that it's worth experimenting with storing these strings, and if we're worried about the size increase we can disable the storing of the metadata before 4.3 goes final.

Patch is queued. Thanks everyone.
via Mercurial-devel - May 19, 2017, 5:23 p.m.
On Fri, May 19, 2017 at 10:17 AM, Jun Wu <quark@fb.com> wrote:
> Excerpts from Gregory Szorc's message of 2017-05-19 09:51:46 -0700:
>> > I agree with Pierre-Yves that the bit-based solution seems better
>> > long-term. I'm not particularly worried about wasting bits. I was also
>> > happy for Durham's patch as a short-term solution. But since
>> > Pierre-Yves et al are already working on the bit-based solution, I'd
>> > prefer to give them at least a month to make progress on that.
>>
>> Perfect is the enemy of done.
>>
>> If we're really uncomfortable with the strings, we can put that behind an
>> experimental config or requirement so we don't pollute real repos. Then we
>> can figure out something before 4.3. At least this way something lands and
>> some progress is made (even if the exact implementation doesn't pan out).
>
> Talking about perfection, I also don't think the 3 bits: parent-change,
> content-change and metadata-change is a good abstraction.
>
> Because they are redundant data. They can be calculated afterwards, in a
> cache. Having them in obsstore means we could have inconsistency - changelog
> disagrees with obsstore about the flags (data written by some bad
> extensions) - how would you deal with that?
>
> "operation" metadata is not redundant - you cannot get them from elsewhere
> so they should be recorded.

That's a great point! I really hate linkrevs for exactly that reason
(I understand that they were useful, but they also cause so many
problems). Caches have their own problems, of course :-)
Boris Feld - May 19, 2017, 8:02 p.m.
On Fri, 2017-05-19 at 09:39 -0700, Martin von Zweigbergk via Mercurial-
devel wrote:
> On Thu, May 18, 2017 at 6:18 AM, Augie Fackler <raf@durin42.com>
> wrote:
> > 
> > > On May 18, 2017, at 04:46, Pierre-Yves David <pierre-yves.david@e
> > > ns-lyon.org> wrote:
> > > 
> > > > >    I would recommend the following step forward:
> > > > > 
> > > > >    1) add a simple "rewritten as" template,
> > > > >    2) record action-bit, use it in the template (and other
> > > > > related output),
> > > > >    3) gather data and look at the best way to issue more
> > > > > details.
> > > > > 
> > > > 
> > > > Since we're not sure what we're going to do yet, wasting
> > > > limited bit
> > > > flags when we have an open-ended metadata field is premature
> > > > optimization. I support the approach in this patch. I also
> > > > support
> > > > converting things to bit flags before 4.3 if we establish
> > > > confidence
> > > > that the feature is working how we'd like.
> > > 
> > > We have to keep in mind that the total size of markers is an
> > > important factor here. There are already useful information
> > > excluded from markers for size reason (eg: parent information for
> > > non-prune). We have to keep being careful here and weight what
> > > extra data is the most important. Adding command name would
> > > increase markers size by tens of percent. Especially since
> > > markers will live forever in the project once created.
> > 
> > I understand that, but I think storing the command name is
> > worthwhile as an experiment. I'd like us to be better about letting
> > the perfect be the enemy of the good. I think we should go ahead
> > and land this patch as-is, and if it turns out to be massively
> > unwise we can back it out before 4.3.
> > 
> > Does anyone (other than Pierre-Yves, who has made his position very
> > clear) object (strongly or weakly) to moving forward with this
> > patch as stated?
> 
> I agree with Pierre-Yves that the bit-based solution seems better
> long-term. I'm not particularly worried about wasting bits. I was
> also
> happy for Durham's patch as a short-term solution. But since
> Pierre-Yves et al are already working on the bit-based solution, I'd
> prefer to give them at least a month to make progress on that.
> 
Hi everyone,

I works with Pierre-Yves on Evolve, I wrote the obslog command that was
released yesterday with Evolve 6.2.0. Today I finished a first
experiment on this topic. It is available on the evolve default branch.

I tried an intermediary approach on the subject, I agree that we are
not ready to decide right now how to use the bitfield in obs markers
and that knowing what was the effect of the change is maybe more
universal (what about hg amend that can or not change content but also
the user, date or description).

My approach was to use a bit-field but put it in the metadata under a
"versioned" field (currently: "ef1") so we can change the format, or
totally remove it, in the future without "losing" the bits.

Another change that I made is, instead of asking the command to give
the data, compute what changed directly before creating the obsmarkers.
It's quite naive for the moment but it have the nice property to make
all the commands (from core or extensions) compatibles, instantly
without any extra work. In order to avoid adding overhead for
repositories not willing to test it, I've put it under an experimental
flag: "experimental.evolution.effect-flags = 1".

Finally, I've updated the debugobshistory command to display the effect
flag modifications and it was very straightforward, you can see
examples of output here: https://www.mercurial-scm.org/repo/evolve/file
/tip/tests/test-evolve-effectflags.t. This is a first version, I plan
to keep improving the output.

Now my plan is to get some team of testers to activate it and give
feedback for a couple of week. Once we get happy enough with the result
we'll start upstreaming it. 
I'll activate it on my own repositories, feel free to do it too.
> 
> > 
> > Thanks!
> > Augie
> > 
> > > If exhausting bit flag is a concern, we can easily extend this
> > > bit field. For now, we could store this extension in the metadata
> > > mapping, and we would store them as an extra byte in the next
> > > obsstore format (that we'll need soon at least for hash agility
> > > anyway).
> > > We could easily dedicate a whole byte to store the proposed
> > > action bit (and store it the marker extra for now, that would
> > > help adjusting bit value over time).
> > 
> > _______________________________________________
> > Mercurial-devel mailing list
> > Mercurial-devel@mercurial-scm.org
> > https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
> 
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Pierre-Yves David - May 19, 2017, 8:08 p.m.
On 05/19/2017 07:17 PM, Jun Wu wrote:
> Excerpts from Gregory Szorc's message of 2017-05-19 09:51:46 -0700:
>>> I agree with Pierre-Yves that the bit-based solution seems better
>>> long-term. I'm not particularly worried about wasting bits. I was also
>>> happy for Durham's patch as a short-term solution. But since
>>> Pierre-Yves et al are already working on the bit-based solution, I'd
>>> prefer to give them at least a month to make progress on that.
>>
>> Perfect is the enemy of done.
>>
>> If we're really uncomfortable with the strings, we can put that behind an
>> experimental config or requirement so we don't pollute real repos. Then we
>> can figure out something before 4.3. At least this way something lands and
>> some progress is made (even if the exact implementation doesn't pan out).
>
> Talking about perfection, I also don't think the 3 bits: parent-change,
> content-change and metadata-change is a good abstraction.
>
> Because they are redundant data. They can be calculated afterwards, in a
> cache.

No they can not. The obsolescence graph can contains reference to node 
we do not have locally then we cannot compute the "effect" of operation 
between these nodes.

Storing the effect are creation time is important. For example, I can 
see that both Alice and Bob rewrote a changeset between two locally 
known version. If I need to dig deeper, I can use the effect-flags to 
discover that Alice changed the code but Bob is the one who did the 
description proof reading and the rebase,

Commands like the new 'obslog' needs that information in the marker to 
provide with usable output.

Please make sure to take the distributed case in account when thinking 
about evolution design.

> Having them in obsstore means we could have inconsistency - changelog
> disagrees with obsstore about the flags (data written by some bad
> extensions) - how would you deal with that?

The effect-flags are used for informative output only, so if it bad, 
there will be slight confusion, but nothing will break.

Client are not supposed to create bad one, but we cannot guarantee that. 
If we want to be paranoid, we could have a server setting to double 
check received marker chains are "okay" (that will probably be a bit 
expensive).

> "operation" metadata is not redundant - you cannot get them from elsewhere
> so they should be recorded.

Locally, we already have the command information in the journal. And 
once we cross the distributed barrier, the content of "operation perform 
very poorly.
Pierre-Yves David - May 19, 2017, 8:12 p.m.
On 05/19/2017 06:54 PM, Jun Wu wrote:
> Excerpts from Martin von Zweigbergk's message of 2017-05-19 09:39:41 -0700:
>> I agree with Pierre-Yves that the bit-based solution seems better
>> long-term. I'm not particularly worried about wasting bits. I was also
>> happy for Durham's patch as a short-term solution. But since
>> Pierre-Yves et al are already working on the bit-based solution, I'd
>> prefer to give them at least a month to make progress on that.
>
> I think the bits do not contain enough information. Consider "absorb", it's
> "content-change" so would you show "amend as" or "absorb as"? There are
> other content-rewriting commands that are not "amend".

The general philosophy here is to express the fact the content changes.
We do not really care how (user already have journal and blackbox to 
understand the how).

Effect-flags store different data (more detailed on some aspect, less on 
other), in a way that work well on distributed system (evolution goal) 
with negligible size overhead. Sure there are trade off on what we store 
but it is unclear that they matters.

I would rather focus on experimenting with the one approach that work 
well in the distributed case (and already available). We might end up 
not needing anything extra on top of it.

> The obsstore will have a format change to support hash-preserving (I'll try
> to get some plans public in the near future)

I'm looking forward to have the plan details published. Our discussion 
were promising.

> and I plan to de-duplicate all
> strings stored there so space usage is less a concern.

Okay that is something different. I've not heard of details yet so I 
would be happy to learn more about it. Until then it seems a bit 
optimistic to ignore size impact.
Jun Wu - May 19, 2017, 8:49 p.m.
Excerpts from Pierre-Yves David's message of 2017-05-19 22:08:31 +0200:
> > Talking about perfection, I also don't think the 3 bits: parent-change,
> > content-change and metadata-change is a good abstraction.
> >
> > Because they are redundant data. They can be calculated afterwards, in a
> > cache.
> 
> No they can not. The obsolescence graph can contains reference to node 
> we do not have locally then we cannot compute the "effect" of operation 
> between these nodes.
> 
> Storing the effect are creation time is important. For example, I can 
> see that both Alice and Bob rewrote a changeset between two locally 
> known version. If I need to dig deeper, I can use the effect-flags to 
> discover that Alice changed the code but Bob is the one who did the 
> description proof reading and the rebase,

If it's for divergence resolution - anyone who has the original changeset
could figure out what to do without the obsstore flags.

I think it's cleaner to just require the old changeset to perform an
automatic divergence resolution. The flags sound like an incomplete solution
to me because it cannot solve the content-divergence case. It also has the
inconsistency problem which does not look too pretty.

Practically, the people resolving the divergence are usually the people
causing the divergence so they do have the old changeset to figure out what
to do.

> Commands like the new 'obslog' needs that information in the marker to 
> provide with usable output.
> 
> Please make sure to take the distributed case in account when thinking 
> about evolution design.
> > Having them in obsstore means we could have inconsistency - changelog
> > disagrees with obsstore about the flags (data written by some bad
> > extensions) - how would you deal with that?
> 
> The effect-flags are used for informative output only, so if it bad, 
> there will be slight confusion, but nothing will break.
> 
> Client are not supposed to create bad one, but we cannot guarantee that. 
> If we want to be paranoid, we could have a server setting to double 
> check received marker chains are "okay" (that will probably be a bit 
> expensive).

It works but I feel it's making things much more complex than necessary to
just solve a problem (divergence resolution) incompletely. I'd prefer neat,
complete solutions, like just use the changelog.

> > "operation" metadata is not redundant - you cannot get them from elsewhere
> > so they should be recorded.
> 
> Locally, we already have the command information in the journal. And 
> once we cross the distributed barrier, the content of "operation perform 
> very poorly.

journal is an extension and is experimental and I do have plans to make big
changes to it. obsstore is in core already. I don't think we should depend
on journal for now.
Pierre-Yves David - May 19, 2017, 10:16 p.m.
On 05/19/2017 07:22 PM, Augie Fackler wrote:
>
>> On May 19, 2017, at 10:17, Jun Wu <quark@fb.com> wrote:
>>
>> Excerpts from Gregory Szorc's message of 2017-05-19 09:51:46 -0700:
>>>> I agree with Pierre-Yves that the bit-based solution seems better
>>>> long-term. I'm not particularly worried about wasting bits. I was also
>>>> happy for Durham's patch as a short-term solution. But since
>>>> Pierre-Yves et al are already working on the bit-based solution, I'd
>>>> prefer to give them at least a month to make progress on that.
>>>
>>> Perfect is the enemy of done.
>>>
>>> If we're really uncomfortable with the strings, we can put that behind an
>>> experimental config or requirement so we don't pollute real repos. Then we
>>> can figure out something before 4.3. At least this way something lands and
>>> some progress is made (even if the exact implementation doesn't pan out).
>>
>> Talking about perfection, I also don't think the 3 bits: parent-change,
>> content-change and metadata-change is a good abstraction.
>>
>> Because they are redundant data. They can be calculated afterwards, in a
>> cache. Having them in obsstore means we could have inconsistency - changelog
>> disagrees with obsstore about the flags (data written by some bad
>> extensions) - how would you deal with that?
>>
>> "operation" metadata is not redundant - you cannot get them from elsewhere
>> so they should be recorded.
>
> I'm satisfied at this point in the thread that it's worth experimenting with storing these strings, and if we're worried about the size increase we can disable the storing of the metadata before 4.3 goes final.
>
> Patch is queued. Thanks everyone.

I would greatly prefer that we focus our experimenting with the 
(available) effect-flag approach.

At least, I think we should get the "operation" experiment behind an 
experimental flag. The impact on obsmarkers size is significant 
(+10-20%) and it is poorly usable in distributed context. I would rather 
limit the change that it end up all over the place and impact use on the 
long run.

Cheers,
Augie Fackler - May 19, 2017, 10:19 p.m.
> On May 19, 2017, at 15:16, Pierre-Yves David <pierre-yves.david@ens-lyon.org> wrote:
> 
> I would greatly prefer that we focus our experimenting with the (available) effect-flag approach.

Nobody is saying that you can't try that, and if it works out that'll be great. I'm unwilling to let the perfect be the enemy of the good, especially when the perfect is still an early-phase experiment and the good is here today and solves real usability problems.

> At least, I think we should get the "operation" experiment behind an experimental flag.

You could send that patch?

> The impact on obsmarkers size is significant (+10-20%) and it is poorly usable in distributed context. I would rather limit the change that it end up all over the place and impact use on the long run.
Pierre-Yves David - May 19, 2017, 10:20 p.m.
On 05/19/2017 10:49 PM, Jun Wu wrote:
> Excerpts from Pierre-Yves David's message of 2017-05-19 22:08:31 +0200:
>>> Talking about perfection, I also don't think the 3 bits: parent-change,
>>> content-change and metadata-change is a good abstraction.
>>>
>>> Because they are redundant data. They can be calculated afterwards, in a
>>> cache.
>>
>> No they can not. The obsolescence graph can contains reference to node
>> we do not have locally then we cannot compute the "effect" of operation
>> between these nodes.
>>
>> Storing the effect are creation time is important. For example, I can
>> see that both Alice and Bob rewrote a changeset between two locally
>> known version. If I need to dig deeper, I can use the effect-flags to
>> discover that Alice changed the code but Bob is the one who did the
>> description proof reading and the rebase,
>
> If it's for divergence resolution - anyone who has the original changeset
> could figure out what to do without the obsstore flags.

No, this is to offer informative (and zoomable) capability to look at 
the obsolescence history. It is to be used by human user, this class of 
feature is an actual requestion from actual team testers.

Most of the rest of your reply seems to be around divergence resolution 
so I'll drop it for now.

>>> "operation" metadata is not redundant - you cannot get them from elsewhere
>>> so they should be recorded.
>>
>> Locally, we already have the command information in the journal. And
>> once we cross the distributed barrier, the content of "operation perform
>> very poorly.
>
> journal is an extension and is experimental and I do have plans to make big
> changes to it. obsstore is in core already. I don't think we should depend
> on journal for now.

Evolution is an experimental feature with most of it UI in an extension, 
so this is not too different. Journal is an extension dedicated to 
keeping track of locally executed commands, this seems to fit your 
"operation" use case well. This is why I'm pointing at it.

Cheers,
Jun Wu - May 19, 2017, 10:28 p.m.
Excerpts from Pierre-Yves David's message of 2017-05-20 00:20:08 +0200:
> > If it's for divergence resolution - anyone who has the original changeset
> > could figure out what to do without the obsstore flags.
> 
> No, this is to offer informative (and zoomable) capability to look at 
> the obsolescence history. It is to be used by human user, this class of 
> feature is an actual requestion from actual team testers.

If it's not for divergence resolution, but just for human eyes. Then the
operation metadata is strictly better from a user-experience point of view.

> Most of the rest of your reply seems to be around divergence resolution 
> so I'll drop it for now.
> 
> >>> "operation" metadata is not redundant - you cannot get them from elsewhere
> >>> so they should be recorded.
> >>
> >> Locally, we already have the command information in the journal. And
> >> once we cross the distributed barrier, the content of "operation perform
> >> very poorly.
> >
> > journal is an extension and is experimental and I do have plans to make big
> > changes to it. obsstore is in core already. I don't think we should depend
> > on journal for now.
> 
> Evolution is an experimental feature with most of it UI in an extension, 
> so this is not too different. Journal is an extension dedicated to 
> keeping track of locally executed commands, this seems to fit your 
> "operation" use case well. This is why I'm pointing at it.

As you said that distributed cases need to be considered when designing
things. In order to make the journal information useful, it needs to be
exchanged somehow. That does not sound like a good idea. So please ignore
journal for now.
via Mercurial-devel - May 21, 2017, 4:30 a.m.
On Fri, May 19, 2017 at 3:20 PM, Pierre-Yves David
<pierre-yves.david@ens-lyon.org> wrote:
> On 05/19/2017 10:49 PM, Jun Wu wrote:
>>
>> Excerpts from Pierre-Yves David's message of 2017-05-19 22:08:31 +0200:
>>>>
>>>> Talking about perfection, I also don't think the 3 bits: parent-change,
>>>> content-change and metadata-change is a good abstraction.
>>>>
>>>> Because they are redundant data. They can be calculated afterwards, in a
>>>> cache.
>>>
>>>
>>> No they can not. The obsolescence graph can contains reference to node
>>> we do not have locally then we cannot compute the "effect" of operation
>>> between these nodes.
>>>
>>> Storing the effect are creation time is important. For example, I can
>>> see that both Alice and Bob rewrote a changeset between two locally
>>> known version. If I need to dig deeper, I can use the effect-flags to
>>> discover that Alice changed the code but Bob is the one who did the
>>> description proof reading and the rebase,
>>
>>
>> If it's for divergence resolution - anyone who has the original changeset
>> could figure out what to do without the obsstore flags.
>
>
> No, this is to offer informative (and zoomable) capability to look at the
> obsolescence history. It is to be used by human user, this class of feature
> is an actual requestion from actual team testers.

It seems like a very reasonable assumption that people will not care
to zoom into the changes in the part of the obs history for which they
don't have any commits. If we accept that assumption, then Jun is
right that this could be moved out into a cache, right? It would be
data that can be safely cached with just the obsmarker (some hash of
it probably) as key. It wouldn't even be affected by stripping commits
(nor obsmarkers).

I really like the new obslog/olog command from the evolve extension,
but it's clearly missing important information (which is no surprise
for a new command). I would really like a -p option for it. The tricky
thing, of course, is that it would pretty much have to have a way of
skipping hunks that changed because of rebasing. I always found git's
"combined diff" format hard to read, but I don't have a better
suggestion here.

The other obvious missing information from obslog is what this thread
is about. The things I can think of that would be useful to me are 1)
3 bits: commit moved, commit message changed, contents changes, and 2)
the operation that created the obsmarker. (2) is of course what
Durham's patch is about. I do think it will be helpful to know that a
commit was created as a result of histedit rather than just knowing
that it moved (for example). Regarding (2), and as noted above, I'm
still with Jun about calculating the bits rather than storing them in
the obsmarkers. However, that seems like more work and it sounded from
Boris's email that the bits would stored inside the metadata field and
we can always ignore that field in 3 years when we have implemented
the bit cache.

In summary, it seems to me like both the operation and the bits would
be useful, so I support getting both in. I'm also not very concerned
about the size overhead. We can start compressing that later if
necessary.

Sorry to add more to an already long thread.

>
> Most of the rest of your reply seems to be around divergence resolution so
> I'll drop it for now.
>
>>>> "operation" metadata is not redundant - you cannot get them from
>>>> elsewhere
>>>> so they should be recorded.
>>>
>>>
>>> Locally, we already have the command information in the journal. And
>>> once we cross the distributed barrier, the content of "operation perform
>>> very poorly.
>>
>>
>> journal is an extension and is experimental and I do have plans to make
>> big
>> changes to it. obsstore is in core already. I don't think we should depend
>> on journal for now.
>
>
> Evolution is an experimental feature with most of it UI in an extension, so
> this is not too different. Journal is an extension dedicated to keeping
> track of locally executed commands, this seems to fit your "operation" use
> case well. This is why I'm pointing at it.
>
> Cheers,
>
> --
> Pierre-Yves David

Patch

diff --git a/hgext/histedit.py b/hgext/histedit.py
--- a/hgext/histedit.py
+++ b/hgext/histedit.py
@@ -1631,7 +1631,7 @@  def safecleanupnode(ui, repo, name, node
                              key=repo.changelog.rev)
         markers = [getmarker(t) for t in sortednodes]
         if markers:
-            obsolete.createmarkers(repo, markers)
+            obsolete.createmarkers(repo, markers, operation='histedit')
     else:
         return cleanupnode(ui, repo, name, nodes)
 
diff --git a/hgext/rebase.py b/hgext/rebase.py
--- a/hgext/rebase.py
+++ b/hgext/rebase.py
@@ -1341,7 +1341,7 @@  def clearrebased(ui, repo, state, skippe
                     succs = (repo[newrev],)
                 markers.append((repo[rev], succs))
         if markers:
-            obsolete.createmarkers(repo, markers)
+            obsolete.createmarkers(repo, markers, operation='rebase')
     else:
         rebased = [rev for rev in state if state[rev] > nullmerge]
         if rebased:
diff --git a/mercurial/cmdutil.py b/mercurial/cmdutil.py
--- a/mercurial/cmdutil.py
+++ b/mercurial/cmdutil.py
@@ -2740,7 +2740,7 @@  def amend(ui, repo, commitfunc, old, ext
                     if node:
                         obs.append((ctx, ()))
 
-                    obsolete.createmarkers(repo, obs)
+                    obsolete.createmarkers(repo, obs, operation='amend')
         if not createmarkers and newid != old.node():
             # Strip the intermediate commit (if there was one) and the amended
             # commit
diff --git a/mercurial/obsolete.py b/mercurial/obsolete.py
--- a/mercurial/obsolete.py
+++ b/mercurial/obsolete.py
@@ -1204,7 +1204,8 @@  def _computedivergentset(repo):
     return divergent
 
 
-def createmarkers(repo, relations, flag=0, date=None, metadata=None):
+def createmarkers(repo, relations, flag=0, date=None, metadata=None,
+                  operation=None):
     """Add obsolete markers between changesets in a repo
 
     <relations> must be an iterable of (<old>, (<new>, ...)[,{metadata}])
@@ -1225,6 +1226,8 @@  def createmarkers(repo, relations, flag=
         metadata = {}
     if 'user' not in metadata:
         metadata['user'] = repo.ui.username()
+    if operation:
+        metadata['operation'] = operation
     tr = repo.transaction('add-obsolescence-marker')
     try:
         markerargs = []
diff --git a/tests/test-histedit-obsolete.t b/tests/test-histedit-obsolete.t
--- a/tests/test-histedit-obsolete.t
+++ b/tests/test-histedit-obsolete.t
@@ -50,10 +50,10 @@  Test that histedit learns about obsolesc
   o  0:cb9a9f314b8b a
   
   $ hg debugobsolete
-  e72d22b19f8ecf4150ab4f91d0973fd9955d3ddf 49d44ab2be1b67a79127568a67c9c99430633b48 0 (*) {'user': 'test'} (glob)
-  3e30a45cf2f719e96ab3922dfe039cfd047956ce 0 {e72d22b19f8ecf4150ab4f91d0973fd9955d3ddf} (*) {'user': 'test'} (glob)
-  1b2d564fad96311b45362f17c2aa855150efb35f 46abc7c4d8738e8563e577f7889e1b6db3da4199 0 (*) {'user': 'test'} (glob)
-  114f4176969ef342759a8a57e6bccefc4234829b 49d44ab2be1b67a79127568a67c9c99430633b48 0 (*) {'user': 'test'} (glob)
+  e72d22b19f8ecf4150ab4f91d0973fd9955d3ddf 49d44ab2be1b67a79127568a67c9c99430633b48 0 (*) {'operation': 'amend', 'user': 'test'} (glob)
+  3e30a45cf2f719e96ab3922dfe039cfd047956ce 0 {e72d22b19f8ecf4150ab4f91d0973fd9955d3ddf} (*) {'operation': 'amend', 'user': 'test'} (glob)
+  1b2d564fad96311b45362f17c2aa855150efb35f 46abc7c4d8738e8563e577f7889e1b6db3da4199 0 (*) {'operation': 'histedit', 'user': 'test'} (glob)
+  114f4176969ef342759a8a57e6bccefc4234829b 49d44ab2be1b67a79127568a67c9c99430633b48 0 (*) {'operation': 'histedit', 'user': 'test'} (glob)
 
 With some node gone missing during the edit.
 
@@ -80,14 +80,14 @@  With some node gone missing during the e
   o  0:cb9a9f314b8b a
   
   $ hg debugobsolete
-  e72d22b19f8ecf4150ab4f91d0973fd9955d3ddf 49d44ab2be1b67a79127568a67c9c99430633b48 0 (*) {'user': 'test'} (glob)
-  3e30a45cf2f719e96ab3922dfe039cfd047956ce 0 {e72d22b19f8ecf4150ab4f91d0973fd9955d3ddf} (*) {'user': 'test'} (glob)
-  1b2d564fad96311b45362f17c2aa855150efb35f 46abc7c4d8738e8563e577f7889e1b6db3da4199 0 (*) {'user': 'test'} (glob)
-  114f4176969ef342759a8a57e6bccefc4234829b 49d44ab2be1b67a79127568a67c9c99430633b48 0 (*) {'user': 'test'} (glob)
-  76f72745eac0643d16530e56e2f86e36e40631f1 2ca853e48edbd6453a0674dc0fe28a0974c51b9c 0 (*) {'user': 'test'} (glob)
-  2ca853e48edbd6453a0674dc0fe28a0974c51b9c aba7da93703075eec9fb1dbaf143ff2bc1c49d46 0 (*) {'user': 'test'} (glob)
-  49d44ab2be1b67a79127568a67c9c99430633b48 273c1f3b86267ed3ec684bb13af1fa4d6ba56e02 0 (*) {'user': 'test'} (glob)
-  46abc7c4d8738e8563e577f7889e1b6db3da4199 aba7da93703075eec9fb1dbaf143ff2bc1c49d46 0 (*) {'user': 'test'} (glob)
+  e72d22b19f8ecf4150ab4f91d0973fd9955d3ddf 49d44ab2be1b67a79127568a67c9c99430633b48 0 (*) {'operation': 'amend', 'user': 'test'} (glob)
+  3e30a45cf2f719e96ab3922dfe039cfd047956ce 0 {e72d22b19f8ecf4150ab4f91d0973fd9955d3ddf} (*) {'operation': 'amend', 'user': 'test'} (glob)
+  1b2d564fad96311b45362f17c2aa855150efb35f 46abc7c4d8738e8563e577f7889e1b6db3da4199 0 (*) {'operation': 'histedit', 'user': 'test'} (glob)
+  114f4176969ef342759a8a57e6bccefc4234829b 49d44ab2be1b67a79127568a67c9c99430633b48 0 (*) {'operation': 'histedit', 'user': 'test'} (glob)
+  76f72745eac0643d16530e56e2f86e36e40631f1 2ca853e48edbd6453a0674dc0fe28a0974c51b9c 0 (*) {'operation': 'amend', 'user': 'test'} (glob)
+  2ca853e48edbd6453a0674dc0fe28a0974c51b9c aba7da93703075eec9fb1dbaf143ff2bc1c49d46 0 (*) {'operation': 'amend', 'user': 'test'} (glob)
+  49d44ab2be1b67a79127568a67c9c99430633b48 273c1f3b86267ed3ec684bb13af1fa4d6ba56e02 0 (*) {'operation': 'histedit', 'user': 'test'} (glob)
+  46abc7c4d8738e8563e577f7889e1b6db3da4199 aba7da93703075eec9fb1dbaf143ff2bc1c49d46 0 (*) {'operation': 'histedit', 'user': 'test'} (glob)
   $ cd ..
 
 Base setup for the rest of the testing
@@ -170,13 +170,13 @@  Base setup for the rest of the testing
   o  0:cb9a9f314b8b a
   
   $ hg debugobsolete
-  96e494a2d553dd05902ba1cee1d94d4cb7b8faed 0 {b346ab9a313db8537ecf96fca3ca3ca984ef3bd7} (*) {'user': 'test'} (glob)
-  b558abc46d09c30f57ac31e85a8a3d64d2e906e4 0 {96e494a2d553dd05902ba1cee1d94d4cb7b8faed} (*) {'user': 'test'} (glob)
-  d2ae7f538514cd87c17547b0de4cea71fe1af9fb 0 {cb9a9f314b8b07ba71012fcdbc544b5a4d82ff5b} (*) {'user': 'test'} (glob)
-  177f92b773850b59254aa5e923436f921b55483b b346ab9a313db8537ecf96fca3ca3ca984ef3bd7 0 (*) {'user': 'test'} (glob)
-  055a42cdd88768532f9cf79daa407fc8d138de9b 59d9f330561fd6c88b1a6b32f0e45034d88db784 0 (*) {'user': 'test'} (glob)
-  e860deea161a2f77de56603b340ebbb4536308ae 59d9f330561fd6c88b1a6b32f0e45034d88db784 0 (*) {'user': 'test'} (glob)
-  652413bf663ef2a641cab26574e46d5f5a64a55a cacdfd884a9321ec4e1de275ef3949fa953a1f83 0 (*) {'user': 'test'} (glob)
+  96e494a2d553dd05902ba1cee1d94d4cb7b8faed 0 {b346ab9a313db8537ecf96fca3ca3ca984ef3bd7} (*) {'operation': 'histedit', 'user': 'test'} (glob)
+  b558abc46d09c30f57ac31e85a8a3d64d2e906e4 0 {96e494a2d553dd05902ba1cee1d94d4cb7b8faed} (*) {'operation': 'histedit', 'user': 'test'} (glob)
+  d2ae7f538514cd87c17547b0de4cea71fe1af9fb 0 {cb9a9f314b8b07ba71012fcdbc544b5a4d82ff5b} (*) {'operation': 'histedit', 'user': 'test'} (glob)
+  177f92b773850b59254aa5e923436f921b55483b b346ab9a313db8537ecf96fca3ca3ca984ef3bd7 0 (*) {'operation': 'histedit', 'user': 'test'} (glob)
+  055a42cdd88768532f9cf79daa407fc8d138de9b 59d9f330561fd6c88b1a6b32f0e45034d88db784 0 (*) {'operation': 'histedit', 'user': 'test'} (glob)
+  e860deea161a2f77de56603b340ebbb4536308ae 59d9f330561fd6c88b1a6b32f0e45034d88db784 0 (*) {'operation': 'histedit', 'user': 'test'} (glob)
+  652413bf663ef2a641cab26574e46d5f5a64a55a cacdfd884a9321ec4e1de275ef3949fa953a1f83 0 (*) {'operation': 'histedit', 'user': 'test'} (glob)
 
 
 Ensure hidden revision does not prevent histedit
diff --git a/tests/test-obsolete.t b/tests/test-obsolete.t
--- a/tests/test-obsolete.t
+++ b/tests/test-obsolete.t
@@ -1247,15 +1247,15 @@  only a subset of those are displayed (be
   adding d
   $ hg ci --amend -m dd
   $ hg debugobsolete --index --rev "3+7"
-  1 6fdef60fcbabbd3d50e9b9cbc2a240724b91a5e1 d27fb9b066076fd921277a4b9e8b9cb48c95bc6a 0 \(.*\) {'user': 'test'} (re)
-  3 4715cf767440ed891755448016c2b8cf70760c30 7ae79c5d60f049c7b0dd02f5f25b9d60aaf7b36d 0 \(.*\) {'user': 'test'} (re)
+  1 6fdef60fcbabbd3d50e9b9cbc2a240724b91a5e1 d27fb9b066076fd921277a4b9e8b9cb48c95bc6a 0 \(.*\) {'operation': 'amend', 'user': 'test'} (re)
+  3 4715cf767440ed891755448016c2b8cf70760c30 7ae79c5d60f049c7b0dd02f5f25b9d60aaf7b36d 0 \(.*\) {'operation': 'amend', 'user': 'test'} (re)
   $ hg debugobsolete --index --rev "3+7" -Tjson
   [
    {
     "date": *, (glob)
     "flag": 0,
     "index": 1,
-    "metadata": {"user": "test"},
+    "metadata": {"operation": "amend", "user": "test"},
     "precnode": "6fdef60fcbabbd3d50e9b9cbc2a240724b91a5e1",
     "succnodes": ["d27fb9b066076fd921277a4b9e8b9cb48c95bc6a"]
    },
@@ -1263,7 +1263,7 @@  only a subset of those are displayed (be
     "date": *, (glob)
     "flag": 0,
     "index": 3,
-    "metadata": {"user": "test"},
+    "metadata": {"operation": "amend", "user": "test"},
     "precnode": "4715cf767440ed891755448016c2b8cf70760c30",
     "succnodes": ["7ae79c5d60f049c7b0dd02f5f25b9d60aaf7b36d"]
    }
@@ -1271,14 +1271,14 @@  only a subset of those are displayed (be
 
 Test the --delete option of debugobsolete command
   $ hg debugobsolete --index
-  0 cb9a9f314b8b07ba71012fcdbc544b5a4d82ff5b f9bd49731b0b175e42992a3c8fa6c678b2bc11f1 0 \(.*\) {'user': 'test'} (re)
-  1 6fdef60fcbabbd3d50e9b9cbc2a240724b91a5e1 d27fb9b066076fd921277a4b9e8b9cb48c95bc6a 0 \(.*\) {'user': 'test'} (re)
-  2 1ab51af8f9b41ef8c7f6f3312d4706d870b1fb74 29346082e4a9e27042b62d2da0e2de211c027621 0 \(.*\) {'user': 'test'} (re)
-  3 4715cf767440ed891755448016c2b8cf70760c30 7ae79c5d60f049c7b0dd02f5f25b9d60aaf7b36d 0 \(.*\) {'user': 'test'} (re)
+  0 cb9a9f314b8b07ba71012fcdbc544b5a4d82ff5b f9bd49731b0b175e42992a3c8fa6c678b2bc11f1 0 \(.*\) {'operation': 'amend', 'user': 'test'} (re)
+  1 6fdef60fcbabbd3d50e9b9cbc2a240724b91a5e1 d27fb9b066076fd921277a4b9e8b9cb48c95bc6a 0 \(.*\) {'operation': 'amend', 'user': 'test'} (re)
+  2 1ab51af8f9b41ef8c7f6f3312d4706d870b1fb74 29346082e4a9e27042b62d2da0e2de211c027621 0 \(.*\) {'operation': 'amend', 'user': 'test'} (re)
+  3 4715cf767440ed891755448016c2b8cf70760c30 7ae79c5d60f049c7b0dd02f5f25b9d60aaf7b36d 0 \(.*\) {'operation': 'amend', 'user': 'test'} (re)
   $ hg debugobsolete --delete 1 --delete 3
   deleted 2 obsolescence markers
   $ hg debugobsolete
-  cb9a9f314b8b07ba71012fcdbc544b5a4d82ff5b f9bd49731b0b175e42992a3c8fa6c678b2bc11f1 0 \(.*\) {'user': 'test'} (re)
-  1ab51af8f9b41ef8c7f6f3312d4706d870b1fb74 29346082e4a9e27042b62d2da0e2de211c027621 0 \(.*\) {'user': 'test'} (re)
+  cb9a9f314b8b07ba71012fcdbc544b5a4d82ff5b f9bd49731b0b175e42992a3c8fa6c678b2bc11f1 0 \(.*\) {'operation': 'amend', 'user': 'test'} (re)
+  1ab51af8f9b41ef8c7f6f3312d4706d870b1fb74 29346082e4a9e27042b62d2da0e2de211c027621 0 \(.*\) {'operation': 'amend', 'user': 'test'} (re)
   $ cd ..
 
diff --git a/tests/test-rebase-obsolete.t b/tests/test-rebase-obsolete.t
--- a/tests/test-rebase-obsolete.t
+++ b/tests/test-rebase-obsolete.t
@@ -100,9 +100,9 @@  simple rebase
   o  0:cd010b8cd998 A
   
   $ hg debugobsolete
-  42ccdea3bb16d28e1848c95fe2e44c000f3f21b1 e4e5be0395b2cbd471ed22a26b1b6a1a0658a794 0 (*) {'user': 'test'} (glob)
-  5fddd98957c8a54a4d436dfe1da9d87f21a1b97b 2327fea05063f39961b14cb69435a9898dc9a245 0 (*) {'user': 'test'} (glob)
-  32af7686d403cf45b5d95f2d70cebea587ac806a 8eeb3c33ad33d452c89e5dcf611c347f978fb42b 0 (*) {'user': 'test'} (glob)
+  42ccdea3bb16d28e1848c95fe2e44c000f3f21b1 e4e5be0395b2cbd471ed22a26b1b6a1a0658a794 0 (*) {'operation': 'rebase', 'user': 'test'} (glob)
+  5fddd98957c8a54a4d436dfe1da9d87f21a1b97b 2327fea05063f39961b14cb69435a9898dc9a245 0 (*) {'operation': 'rebase', 'user': 'test'} (glob)
+  32af7686d403cf45b5d95f2d70cebea587ac806a 8eeb3c33ad33d452c89e5dcf611c347f978fb42b 0 (*) {'operation': 'rebase', 'user': 'test'} (glob)
 
 
   $ cd ..
@@ -170,9 +170,9 @@  set.
   o  0:cd010b8cd998 A
   
   $ hg debugobsolete
-  42ccdea3bb16d28e1848c95fe2e44c000f3f21b1 0 {cd010b8cd998f3981a5a8115f94f8da4ab506089} (*) {'user': 'test'} (glob)
-  5fddd98957c8a54a4d436dfe1da9d87f21a1b97b 5ae4c968c6aca831df823664e706c9d4aa34473d 0 (*) {'user': 'test'} (glob)
-  32af7686d403cf45b5d95f2d70cebea587ac806a 0 {5fddd98957c8a54a4d436dfe1da9d87f21a1b97b} (*) {'user': 'test'} (glob)
+  42ccdea3bb16d28e1848c95fe2e44c000f3f21b1 0 {cd010b8cd998f3981a5a8115f94f8da4ab506089} (*) {'operation': 'rebase', 'user': 'test'} (glob)
+  5fddd98957c8a54a4d436dfe1da9d87f21a1b97b 5ae4c968c6aca831df823664e706c9d4aa34473d 0 (*) {'operation': 'rebase', 'user': 'test'} (glob)
+  32af7686d403cf45b5d95f2d70cebea587ac806a 0 {5fddd98957c8a54a4d436dfe1da9d87f21a1b97b} (*) {'operation': 'rebase', 'user': 'test'} (glob)
 
 
 More complex case where part of the rebase set were already rebased
@@ -180,10 +180,10 @@  More complex case where part of the reba
   $ hg rebase --rev 'desc(D)' --dest 'desc(H)'
   rebasing 9:08483444fef9 "D"
   $ hg debugobsolete
-  42ccdea3bb16d28e1848c95fe2e44c000f3f21b1 0 {cd010b8cd998f3981a5a8115f94f8da4ab506089} (*) {'user': 'test'} (glob)
-  5fddd98957c8a54a4d436dfe1da9d87f21a1b97b 5ae4c968c6aca831df823664e706c9d4aa34473d 0 (*) {'user': 'test'} (glob)
-  32af7686d403cf45b5d95f2d70cebea587ac806a 0 {5fddd98957c8a54a4d436dfe1da9d87f21a1b97b} (*) {'user': 'test'} (glob)
-  08483444fef91d6224f6655ee586a65d263ad34c 4596109a6a4328c398bde3a4a3b6737cfade3003 0 (*) {'user': 'test'} (glob)
+  42ccdea3bb16d28e1848c95fe2e44c000f3f21b1 0 {cd010b8cd998f3981a5a8115f94f8da4ab506089} (*) {'operation': 'rebase', 'user': 'test'} (glob)
+  5fddd98957c8a54a4d436dfe1da9d87f21a1b97b 5ae4c968c6aca831df823664e706c9d4aa34473d 0 (*) {'operation': 'rebase', 'user': 'test'} (glob)
+  32af7686d403cf45b5d95f2d70cebea587ac806a 0 {5fddd98957c8a54a4d436dfe1da9d87f21a1b97b} (*) {'operation': 'rebase', 'user': 'test'} (glob)
+  08483444fef91d6224f6655ee586a65d263ad34c 4596109a6a4328c398bde3a4a3b6737cfade3003 0 (*) {'operation': 'rebase', 'user': 'test'} (glob)
   $ hg log -G
   @  11:4596109a6a43 D
   |
@@ -208,12 +208,12 @@  More complex case where part of the reba
   note: not rebasing 9:08483444fef9 "D", already in destination as 11:4596109a6a43 "D"
   rebasing 10:5ae4c968c6ac "C"
   $ hg debugobsolete
-  42ccdea3bb16d28e1848c95fe2e44c000f3f21b1 0 {cd010b8cd998f3981a5a8115f94f8da4ab506089} (*) {'user': 'test'} (glob)
-  5fddd98957c8a54a4d436dfe1da9d87f21a1b97b 5ae4c968c6aca831df823664e706c9d4aa34473d 0 (*) {'user': 'test'} (glob)
-  32af7686d403cf45b5d95f2d70cebea587ac806a 0 {5fddd98957c8a54a4d436dfe1da9d87f21a1b97b} (*) {'user': 'test'} (glob)
-  08483444fef91d6224f6655ee586a65d263ad34c 4596109a6a4328c398bde3a4a3b6737cfade3003 0 (*) {'user': 'test'} (glob)
-  8877864f1edb05d0e07dc4ba77b67a80a7b86672 462a34d07e599b87ea08676a449373fe4e2e1347 0 (*) {'user': 'test'} (glob)
-  5ae4c968c6aca831df823664e706c9d4aa34473d 98f6af4ee9539e14da4465128f894c274900b6e5 0 (*) {'user': 'test'} (glob)
+  42ccdea3bb16d28e1848c95fe2e44c000f3f21b1 0 {cd010b8cd998f3981a5a8115f94f8da4ab506089} (*) {'operation': 'rebase', 'user': 'test'} (glob)
+  5fddd98957c8a54a4d436dfe1da9d87f21a1b97b 5ae4c968c6aca831df823664e706c9d4aa34473d 0 (*) {'operation': 'rebase', 'user': 'test'} (glob)
+  32af7686d403cf45b5d95f2d70cebea587ac806a 0 {5fddd98957c8a54a4d436dfe1da9d87f21a1b97b} (*) {'operation': 'rebase', 'user': 'test'} (glob)
+  08483444fef91d6224f6655ee586a65d263ad34c 4596109a6a4328c398bde3a4a3b6737cfade3003 0 (*) {'operation': 'rebase', 'user': 'test'} (glob)
+  8877864f1edb05d0e07dc4ba77b67a80a7b86672 462a34d07e599b87ea08676a449373fe4e2e1347 0 (*) {'operation': 'rebase', 'user': 'test'} (glob)
+  5ae4c968c6aca831df823664e706c9d4aa34473d 98f6af4ee9539e14da4465128f894c274900b6e5 0 (*) {'operation': 'rebase', 'user': 'test'} (glob)
   $ hg log --rev 'divergent()'
   $ hg log -G
   o  13:98f6af4ee953 C
@@ -349,9 +349,9 @@  collapse rebase
   $ hg id --debug -r tip
   4dc2197e807bae9817f09905b50ab288be2dbbcf tip
   $ hg debugobsolete
-  42ccdea3bb16d28e1848c95fe2e44c000f3f21b1 4dc2197e807bae9817f09905b50ab288be2dbbcf 0 (*) {'user': 'test'} (glob)
-  5fddd98957c8a54a4d436dfe1da9d87f21a1b97b 4dc2197e807bae9817f09905b50ab288be2dbbcf 0 (*) {'user': 'test'} (glob)
-  32af7686d403cf45b5d95f2d70cebea587ac806a 4dc2197e807bae9817f09905b50ab288be2dbbcf 0 (*) {'user': 'test'} (glob)
+  42ccdea3bb16d28e1848c95fe2e44c000f3f21b1 4dc2197e807bae9817f09905b50ab288be2dbbcf 0 (*) {'operation': 'rebase', 'user': 'test'} (glob)
+  5fddd98957c8a54a4d436dfe1da9d87f21a1b97b 4dc2197e807bae9817f09905b50ab288be2dbbcf 0 (*) {'operation': 'rebase', 'user': 'test'} (glob)
+  32af7686d403cf45b5d95f2d70cebea587ac806a 4dc2197e807bae9817f09905b50ab288be2dbbcf 0 (*) {'operation': 'rebase', 'user': 'test'} (glob)
 
   $ cd ..
 
@@ -411,9 +411,9 @@  not be rebased.
   o  0:cd010b8cd998 A
   
   $ hg debugobsolete
-  5fddd98957c8a54a4d436dfe1da9d87f21a1b97b e273c5e7d2d29df783dce9f9eaa3ac4adc69c15d 0 (*) {'user': 'test'} (glob)
-  32af7686d403cf45b5d95f2d70cebea587ac806a cf44d2f5a9f4297a62be94cbdd3dff7c7dc54258 0 (*) {'user': 'test'} (glob)
-  42ccdea3bb16d28e1848c95fe2e44c000f3f21b1 7c6027df6a99d93f461868e5433f63bde20b6dfb 0 (*) {'user': 'test'} (glob)
+  5fddd98957c8a54a4d436dfe1da9d87f21a1b97b e273c5e7d2d29df783dce9f9eaa3ac4adc69c15d 0 (*) {'operation': 'rebase', 'user': 'test'} (glob)
+  32af7686d403cf45b5d95f2d70cebea587ac806a cf44d2f5a9f4297a62be94cbdd3dff7c7dc54258 0 (*) {'operation': 'rebase', 'user': 'test'} (glob)
+  42ccdea3bb16d28e1848c95fe2e44c000f3f21b1 7c6027df6a99d93f461868e5433f63bde20b6dfb 0 (*) {'operation': 'rebase', 'user': 'test'} (glob)
 
 Test that rewriting leaving instability behind is allowed
 ---------------------------------------------------------------------