[PATCH 2 of 5 topic-experiment] topics: make sure we commit on new parents while changing topics (issue5441)

Pierre-Yves David pierre-yves.david at ens-lyon.org
Mon Jun 19 02:31:16 EDT 2017


On 06/19/2017 12:12 AM, Pulkit Goyal wrote:
> # HG changeset patch
> # User Pulkit Goyal <7895pulkit at gmail.com>
> # Date 1497736426 -19800
> #      Sun Jun 18 03:23:46 2017 +0530
> # Node ID 3c6e013ffb8cb4e76b1e496bf8c34ebf790a506d
> # Parent  522b3457c765d271d8029a92dc3d8590b6679c6b
> topics: make sure we commit on new parents while changing topics (issue5441)
>
> While changing topics of a set of linear commits, we used to commit our new
> changesets with new topic on parents of its predecessor i.e. changeset before
> the topic change. If the topic of parent was also changed, that parent will
> become obsolete and hence resulting the cnew commit in unstable state. For a set
> of linear commits this repeats and we end up in a tree state as mentioned in the
> bug.
>
> This patch fixes the bug by checking whether the parent was obsoleted and if
> yes, commit on the new parent.

This patch solve the simple case (everything is linear, no merge) but it 
is a bit too simple for more advanced cases.

The general approach in the patch is good, but storing the mapping for 
the last rewrite only is too limited. You should build a dict that map 
(old → new) for all affected changeset and use it in the computation. eg:

    p1 = c.p1().node()
    p1 = rewritemap.get(p1, p1)

To catch the current issue, you can add a test with branching within the 
topic (eg: three changesets: A; B (child of A); C (child of A)).

I also recommend testing the cases were merges are involved.

--

In addition, another bug remain around topic change. If '.' is part of 
the rewritten changeset, the working copy should be updated. Right now 
it remains on the old changeset, keeping it visible (this is shown in 
the test).

We should update the working copy location when '.' is moved. This is an 
easy case since the content of the working copy is not affected. You 
should be able to find examples of this in other command (eg: hg fold).

> diff --git a/hgext3rd/topic/__init__.py b/hgext3rd/topic/__init__.py
> --- a/hgext3rd/topic/__init__.py
> +++ b/hgext3rd/topic/__init__.py
> @@ -282,6 +282,10 @@
>          l = repo.lock()
>          txn = repo.transaction('rewrite-topics')
>          try:
> +            newp = None
> +            oldp = None
> +            p1 = None
> +            p2 = None
>              for c in repo.set('%r', change):
>                  def filectxfn(repo, ctx, path):
>                      try:
> @@ -309,11 +313,25 @@
>                  ui.debug('changing topic of %s from %s to %s\n' % (
>                      c, oldtopic, newtopic))
>                  ui.debug('fixedextra: %r\n' % fixedextra)
> +                # While changing topic of set of linear commits, make sure that
> +                # we base our commits on new parent rather than old parent which
> +                # was obsoleted while changing the topic
> +                if newp and c.p1().node() == oldp:
> +                    p1 = newp
> +                    p2 = c.p2().node()
> +                elif newp and c.p2().node() == oldp:
> +                    p1 = c.p1().node()
> +                    p2 = newp
> +                else:
> +                    p1 = c.p1().node()
> +                    p2 = c.p2().node()
>                  mc = context.memctx(
> -                    repo, (c.p1().node(), c.p2().node()), c.description(),
> +                    repo, (p1, p2), c.description(),
>                      c.files(), filectxfn,
>                      user=c.user(), date=c.date(), extra=fixedextra)
>                  newnode = repo.commitctx(mc)
> +                oldp = c.node()
> +                newp = newnode
>                  ui.debug('new node id is %s\n' % node.hex(newnode))
>                  needevolve = needevolve or (len(c.children()) > 0)
>                  obsolete.createmarkers(repo, [(c, (repo[newnode],))])
> diff --git a/tests/test-topic.t b/tests/test-topic.t
> --- a/tests/test-topic.t
> +++ b/tests/test-topic.t
> @@ -679,30 +679,28 @@
>    changed topic on 2 changes
>    please run hg evolve --rev "topic(changewat)" now
>    $ hg log -Gr 'draft()'
> -  o  changeset:   21:3c7d84fcabcd
> +  o  changeset:   21:58e15a6365ca
>    |  tag:         tip
>    |  topic:       changewat
> -  |  parent:      13:d91cd8fd490e
>    |  user:        test
>    |  date:        Thu Jan 01 00:00:00 1970 +0000
> -  |  trouble:     unstable
>    |  summary:     fran?
>    |
> -  | o  changeset:   20:a96ac830b62e
> -  | |  topic:       changewat
> -  | |  parent:      3:a53952faf762
> +  o  changeset:   20:a96ac830b62e
> +  |  topic:       changewat
> +  |  parent:      3:a53952faf762
> +  |  user:        test
> +  |  date:        Thu Jan 01 00:00:00 1970 +0000
> +  |  summary:     start on fran
> +  |
> +  | @  changeset:   19:b72b86a1f96b
> +  | |  topic:       watwat
> +  | |  parent:      13:d91cd8fd490e
>    | |  user:        test
>    | |  date:        Thu Jan 01 00:00:00 1970 +0000
> -  | |  summary:     start on fran
> +  | |  summary:     fran?
>    | |
> -  +---@  changeset:   19:b72b86a1f96b
> -  | |    topic:       watwat
> -  | |    parent:      13:d91cd8fd490e
> -  | |    user:        test
> -  | |    date:        Thu Jan 01 00:00:00 1970 +0000
> -  | |    summary:     fran?
> -  | |
> -  x |  changeset:   13:d91cd8fd490e
> +  | x  changeset:   13:d91cd8fd490e
>    |/   topic:       wat
>    |    parent:      3:a53952faf762
>    |    user:        test
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel at mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
>

-- 
Pierre-Yves David


More information about the Mercurial-devel mailing list