Patchwork convert: Add 'close' as a possible splicemap option

login
register
mail settings
Submitter John Peacock
Date Jan. 2, 2013, 3:51 p.m.
Message ID <326d97248e8b753f01b4.1357141869@jpeacock.int.messagesystems.com>
Download mbox | patch
Permalink /patch/367/
State Changes Requested
Headers show

Comments

John Peacock - Jan. 2, 2013, 3:51 p.m.
# HG changeset patch
# User John Peacock <john.peacock at messagesystems.com>
# Date 1357141812 18000
# Branch stable
# Node ID 326d97248e8b753f01b41349e2e6e3163e26fae4
# Parent  6d2ba9875d083e7acad3c1674d6b922a4b7bdb76
convert: Add 'close' as a possible splicemap option.

Sometimes a shared repo that makes extensive use of feature branches
is not managed appropriately and a growing number of open but inactive
branches litter the branches listing.  Since there is no way to close
multiple branches in a single commit (even though the act of
closing is just altering metadata), cleaning up a repository like
this can be an exercise in frustration and makes the history unclear.

This patch extends the splicemap to accept a second type of line which
signals a changeset to mark as being closed as part of the conversion.
During the scanning of the source, the changeset matching the key has
the changeset marked as closed in the extra metadata.  Then later when
that changeset is actually processed, the original source parents are
used for history and the 'close' value is just ignored.

It seems unlikely that a single changeset would require both synthetic
history as well as being closed, so that case is not handled.  Either
new parent(s) can be synthesized OR a branch can be closed, but not both.
Mads Kiilerich - Jan. 3, 2013, 1:02 a.m.
John Peacock wrote, On 01/02/2013 04:51 PM:
> # HG changeset patch
> # User John Peacock <john.peacock at messagesystems.com>
> # Date 1357141812 18000
> # Branch stable
> # Node ID 326d97248e8b753f01b41349e2e6e3163e26fae4
> # Parent  6d2ba9875d083e7acad3c1674d6b922a4b7bdb76
> convert: Add 'close' as a possible splicemap option.

A couple of minor details: The subject line should be all lower case and 
without trailing ".".

> Sometimes a shared repo that makes extensive use of feature branches
> is not managed appropriately and a growing number of open but inactive
> branches litter the branches listing.  Since there is no way to close
> multiple branches in a single commit (even though the act of
> closing is just altering metadata), cleaning up a repository like
> this can be an exercise in frustration and makes the history unclear.
>
> This patch extends the splicemap to accept a second type of line which
> signals a changeset to mark as being closed as part of the conversion.
> During the scanning of the source, the changeset matching the key has
> the changeset marked as closed in the extra metadata.  Then later when
> that changeset is actually processed, the original source parents are
> used for history and the 'close' value is just ignored.
>
> It seems unlikely that a single changeset would require both synthetic
> history as well as being closed, so that case is not handled.  Either
> new parent(s) can be synthesized OR a branch can be closed, but not both.

This seems like a heavy overload of splicemap.

It is nice to avoid adding extra parameters and config files, but this 
might be too much of a hack.

I wonder: couldn't these branches easily be closed with some simple hg 
commands after the conversion has completed?
   hg up -r "converted(86d3d4ecdf64)" && hg commit --close-branch

And if you want some functionality for closing branches, shouldn't there 
also be a way to close them topologically by (dummy) merging them? 
Creating a 'splicemap' format for that would be a can of worms ... but 
it is quite obvious how to do that if using a script with hg commands.

> diff --git a/tests/test-convert-splicemap.t b/tests/test-convert-splicemap.t
> --- a/tests/test-convert-splicemap.t
> +++ b/tests/test-convert-splicemap.t
> @@ -220,3 +220,46 @@
>     scanning source...
>     abort: unknown splice map parent: deadbeef102a90ea7b4a3361e4082ed620918c26
>     [255]
> +
> +Test branch closing

This could probably be tested with a minor modification of an existing test.

> +
> +  $ hg init closebranch
> +  $ cd closebranch
> +  $ echo a > a
> +  $ hg ci -Am createa
> +  adding a
> +  $ hg branch newa
> +  marked working directory as branch newa
> +  (branches are permanent and global, did you want a bookmark?)
> +  $ hg echo aa > a
> +  hg: unknown command 'echo'
> +  [255]

???

> +  $ hg ci -m newa
> +  $ hg up default
> +  1 files updated, 0 files merged, 0 files removed, 0 files unresolved
> +  $ hg merge newa
> +  1 files updated, 0 files merged, 0 files removed, 0 files unresolved
> +  (branch merge, don't forget to commit)
> +  $ hg ci -m mergenewa
> +  $ hg branches
> +  default                        2:5502a03b9720
> +  newa                           1:86d3d4ecdf64 (inactive)
> +
> +
> +  $ hg log -r1 --template "{node} close\n" > ../splicemap.cb

FWIW, for readability I would prefer to have the splicemap content 
explicit in the test.

/Mads
John Peacock - Jan. 3, 2013, 2:59 p.m.
On 01/02/2013 08:02 PM, Mads Kiilerich wrote:
> John Peacock wrote, On 01/02/2013 04:51 PM:
>> # HG changeset patch
>> # User John Peacock <john.peacock at messagesystems.com>
>> # Date 1357141812 18000
>> # Branch stable
>> # Node ID 326d97248e8b753f01b41349e2e6e3163e26fae4
>> # Parent  6d2ba9875d083e7acad3c1674d6b922a4b7bdb76
>> convert: Add 'close' as a possible splicemap option.
>
> A couple of minor details: The subject line should be all lower case and
> without trailing ".".

Thanks for being patient with me; I have reread the applicable wiki page 
(again)... ;-)

> This seems like a heavy overload of splicemap.
>
> It is nice to avoid adding extra parameters and config files, but this
> might be too much of a hack.

The issue is that any solution like this has the same basic 
functionality as splicemap: key [values].  I could have added an 
additional option file (--closemap) but it would wind up being an almost 
literal copy/paste of the splicemap code.  Perhaps making a generic 
mapping implementation that supported different modes:

   key action [args]

would ultimately be cleaner.  For now <action> would only be "splice" 
and now "close", but it could be extended to "merge" (as you mention below).

> I wonder: couldn't these branches easily be closed with some simple hg
> commands after the conversion has completed?
>    hg up -r "converted(86d3d4ecdf64)" && hg commit --close-branch

Sure, but we have 56 of these branches, so the post-conversion history 
would just have a lot of effectively useless commits as cleanup.  Much 
nicer to just close them during the conversion.  If there was a way to 
create a transaction affecting the metadata for multiple branches in a 
single commit, that would be different (and I suspect much harder, if 
not impossible).

> And if you want some functionality for closing branches, shouldn't there
> also be a way to close them topologically by (dummy) merging them?
> Creating a 'splicemap' format for that would be a can of worms ... but
> it is quite obvious how to do that if using a script with hg commands.

In this case, all of the branches I was interested in closing were 
inactive (already merged).  Branches that should be closed with (dummy) 
merges are legitimate history cleanup, and I wouldn't hesitate to do 
those post conversion.  That is useful history; setting what is 
effectively a single flag in a header is not useful history.

>> +  $ hg echo aa > a
>> +  hg: unknown command 'echo'
>> +  [255]
>
> ???

Oops!  Copypasta (or rather that `hg` is preprogrammed into my fingers 
as the prefix for any command these days). ;-)

>> +  $ hg log -r1 --template "{node} close\n" > ../splicemap.cb
>
> FWIW, for readability I would prefer to have the splicemap content
> explicit in the test.

It wasn't clear to me that the node value would be consistent when rerun 
by different people.  In fact, while trying to write a different test, 
the initial repo had epoch 0 date (Jan 1, 1970), but the repo created by 
convert wound up with the current date.  There may be some subtlety I am 
missing...

John

Patch

diff --git a/hgext/convert/__init__.py b/hgext/convert/__init__.py
--- a/hgext/convert/__init__.py
+++ b/hgext/convert/__init__.py
@@ -122,6 +122,20 @@ 
     specify the revision on "trunk" as the first parent and the one on
     the "release-1.0" branch as the second.
 
+    You can also use a splicemap to close inactive branches during the
+    conversion (if the source repo was not well managed).  In this case,
+    the entry contains a key, followed by a space, and literal 'close'::
+
+      key close
+
+    It is not possible to combine both synthetic history and marking a
+    branch as closed.  A branch doesn't need to strictly be inactive
+    (an inactive branch is one whose head revision has already been
+    merged); any branches that have not been merged but are no longer 
+    needed or valid can be close in this fashion as well.  Note that a
+    closed branch is still reachable by updating to that branch and will
+    be reopened automatically when a new commit is made.
+
     The branchmap is a file that allows you to rename a branch when it is
     being brought in from whatever external repository. When used in
     conjunction with a splicemap, it allows for a powerful combination
diff --git a/hgext/convert/convcmd.py b/hgext/convert/convcmd.py
--- a/hgext/convert/convcmd.py
+++ b/hgext/convert/convcmd.py
@@ -159,6 +159,11 @@ 
                 # We do not have to wait for nodes already in dest.
                 if self.dest.hascommit(self.map.get(p, p)):
                     continue
+                # Mark the commit as closed and ignore "parent"
+                if p == 'close':
+                    commit = self.cachecommit(c)
+                    commit.extra['close'] = 1
+                    continue
                 # Parent is not in dest and not being converted, not good
                 if p not in parents:
                     raise util.Abort(_('unknown splice map parent: %s') % p)
@@ -343,9 +348,15 @@ 
         self.dest.setbranch(commit.branch, pbranches)
         try:
             parents = self.splicemap[rev]
-            self.ui.status(_('spliced in %s as parents of %s\n') %
-                           (parents, rev))
-            parents = [self.map.get(p, p) for p in parents]
+            # Commit is already marked closed, so use original parents
+            if 'close' in parents:
+              self.ui.status(_('splice map closed %s\n') %
+                             (commit.branch))
+              parents = [self.map.get(p, p) for p in commit.parents]
+            else:
+              self.ui.status(_('spliced in %s as parents of %s\n') %
+                             (parents, rev))
+              parents = [self.map.get(p, p) for p in parents]
         except KeyError:
             parents = [b[0] for b in pbranches]
         source = progresssource(self.ui, self.source, len(files))
diff --git a/tests/test-convert-splicemap.t b/tests/test-convert-splicemap.t
--- a/tests/test-convert-splicemap.t
+++ b/tests/test-convert-splicemap.t
@@ -220,3 +220,46 @@ 
   scanning source...
   abort: unknown splice map parent: deadbeef102a90ea7b4a3361e4082ed620918c26
   [255]
+
+Test branch closing
+
+  $ hg init closebranch
+  $ cd closebranch
+  $ echo a > a
+  $ hg ci -Am createa
+  adding a
+  $ hg branch newa
+  marked working directory as branch newa
+  (branches are permanent and global, did you want a bookmark?)
+  $ hg echo aa > a
+  hg: unknown command 'echo'
+  [255]
+  $ hg ci -m newa
+  $ hg up default
+  1 files updated, 0 files merged, 0 files removed, 0 files unresolved
+  $ hg merge newa
+  1 files updated, 0 files merged, 0 files removed, 0 files unresolved
+  (branch merge, don't forget to commit)
+  $ hg ci -m mergenewa
+  $ hg branches
+  default                        2:5502a03b9720
+  newa                           1:86d3d4ecdf64 (inactive)
+
+
+  $ hg log -r1 --template "{node} close\n" > ../splicemap.cb
+  $ cd ..
+  $ hg convert --splicemap=splicemap.cb closebranch closebranch.closed
+  initializing destination closebranch.closed repository
+  scanning source...
+  sorting...
+  converting...
+  2 createa
+  1 newa
+  splice map closed newa
+  0 mergenewa
+  $ cd closebranch.closed
+  $ hg branches
+  default                        2:7c1b32ad8d42
+  $ hg branches -c
+  default                        2:7c1b32ad8d42
+  newa                           1:57ad6ec6adcc (closed)