[PATCH STABLE] merge: don't try to merge subrepos twice (issue4988)

Siddharth Agarwal sid at less-broken.com
Fri Jan 29 16:26:22 CST 2016


On 1/29/16 14:22, Siddharth Agarwal wrote:
> # HG changeset patch
> # User Siddharth Agarwal <sid0 at fb.com>
> # Date 1454105969 28800
> #      Fri Jan 29 14:19:29 2016 -0800
> # Branch stable
> # Node ID 4e74815fcb8b3197141c59fa5b26de1e59d1d0f1
> # Parent  7cb7264cfd52d2672644db4bc16a0bd50aa093ca
> # Available At http://42.netv6.net/sid0-wip/hg/
> #              hg pull http://42.netv6.net/sid0-wip/hg/ -r 4e74815fcb8b
> merge: don't try to merge subrepos twice (issue4988)
>
> In my patch series ending with rev 25e4b2f000c5 I switched most change/delete
> conflicts to be handled at the resolve layer. .hgsubstate was the one file that
> we weren't able to handle, so we kept the old code path around for it.
>
> The old code path added .hgsubstate to one of the other lists as the user
> specifies, including possibly the 'g' list.
>
> Now since we did this check after converting the actions from being keyed by
> file to being keyed by action type, there was nothing that actually removed
> .hgsubstate from the 'cd' or 'dc' lists. This meant that the file would
> eventually make its way into the 'mergeactions' list, now freshly augmented
> with 'cd' and 'dc' actions.
>
> We call subrepo.submerge for both 'g' actions and merge actions.
>
> This means that if the resolution to an .hgsubstate change/delete conflict was
> to add it to the 'g' list, subrepo.submerge would be called twice. It turns out
> that his doesn't cause any adverse effects on Linux due to caching, but
> apparently breaks on other operating systems including Windows.
>
> The fix here moves this to before we convert the actions over. This ensures
> that it .hgsubstate doesn't make its way into multiple lists.
>
> The real fix here is going to be:
> (1) move .hgsubstate conflict resolution into the resolve layer, and
> (2) use a real data structure for the actions rather than shuffling data around
>      between lists and dictionaries: we need a hash (or prefix-based) index by
>      file and a list index by action type.
>
> There's a very tiny behavior change here: collision detection on case-sensitive

er, case-INsensitive.

> systems will happen after this is resolved, not before. I think this is the
> right change -- .hgsubstate could theoretically collide with other files -- but
> in any case it makes no practical difference.
>
> Thanks to Yuya Nishihara for investigating this.
>
> diff --git a/mercurial/merge.py b/mercurial/merge.py
> --- a/mercurial/merge.py
> +++ b/mercurial/merge.py
> @@ -1464,6 +1464,34 @@ def update(repo, node, branchmerge, forc
>           actionbyfile, diverge, renamedelete = calculateupdates(
>               repo, wc, p2, pas, branchmerge, force, mergeancestor,
>               followcopies, matcher=matcher)
> +
> +        # Prompt and create actions. Most of this is in the resolve phase
> +        # already, but we can't handle .hgsubstate in filemerge or
> +        # subrepo.submerge yet so we have to keep prompting for it.
> +        if '.hgsubstate' in actionbyfile:
> +            f = '.hgsubstate'
> +            m, args, msg = actionbyfile[f]
> +            if m == 'cd':
> +                if repo.ui.promptchoice(
> +                    _("local changed %s which remote deleted\n"
> +                      "use (c)hanged version or (d)elete?"
> +                      "$$ &Changed $$ &Delete") % f, 0):
> +                    actionbyfile[f] = ('r', None, "prompt delete")
> +                elif f in p1:
> +                    actionbyfile[f] = ('am', None, "prompt keep")
> +                else:
> +                    actionbyfile[f] = ('a', None, "prompt keep")
> +            elif m == 'dc':
> +                f1, f2, fa, move, anc = args
> +                flags = p2[f2].flags()
> +                if repo.ui.promptchoice(
> +                    _("remote changed %s which local deleted\n"
> +                      "use (c)hanged version or leave (d)eleted?"
> +                      "$$ &Changed $$ &Deleted") % f, 0) == 0:
> +                    actionbyfile[f] = ('g', (flags, False), "prompt recreating")
> +                else:
> +                    del actionbyfile[f]
> +
>           # Convert to dictionary-of-lists format
>           actions = dict((m, []) for m in 'a am f g cd dc r dm dg m e k'.split())
>           for f, (m, args, msg) in actionbyfile.iteritems():
> @@ -1479,33 +1507,6 @@ def update(repo, node, branchmerge, forc
>               else:
>                   _checkcollision(repo, wc.manifest(), actions)
>   
> -        # Prompt and create actions. Most of this is in the resolve phase
> -        # already, but we can't handle .hgsubstate in filemerge or
> -        # subrepo.submerge yet so we have to keep prompting for it.
> -        for f, args, msg in sorted(actions['cd']):
> -            if f != '.hgsubstate':
> -                continue
> -            if repo.ui.promptchoice(
> -                _("local changed %s which remote deleted\n"
> -                  "use (c)hanged version or (d)elete?"
> -                  "$$ &Changed $$ &Delete") % f, 0):
> -                actions['r'].append((f, None, "prompt delete"))
> -            elif f in p1:
> -                actions['am'].append((f, None, "prompt keep"))
> -            else:
> -                actions['a'].append((f, None, "prompt keep"))
> -
> -        for f, args, msg in sorted(actions['dc']):
> -            if f != '.hgsubstate':
> -                continue
> -            f1, f2, fa, move, anc = args
> -            flags = p2[f2].flags()
> -            if repo.ui.promptchoice(
> -                _("remote changed %s which local deleted\n"
> -                  "use (c)hanged version or leave (d)eleted?"
> -                  "$$ &Changed $$ &Deleted") % f, 0) == 0:
> -                actions['g'].append((f, (flags, False), "prompt recreating"))
> -
>           # divergent renames
>           for f, fl in sorted(diverge.iteritems()):
>               repo.ui.warn(_("note: possible conflict - %s was renamed "
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel at selenic.com
> https://selenic.com/mailman/listinfo/mercurial-devel



More information about the Mercurial-devel mailing list