From patchwork Tue Dec 7 17:03:12 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: D11879: simplemerge: stop merging file flags From: phabricator X-Patchwork-Id: 50204 Message-Id: To: Phabricator Cc: mercurial-devel@mercurial-scm.org Date: Tue, 7 Dec 2021 17:03:12 +0000 martinvonz created this revision. Herald added a reviewer: hg-reviewers. Herald added a subscriber: mercurial-patches. REVISION SUMMARY As 384df4db6520 (merge: merge file flags together with file content, 2013-01-09) explains, we shouldn't do a 3-way merge of the symlink. However, since 84614212ae39 (flags: actually merge flags in simplemerge, 2020-05-16), we do that in `simplemerge.simplemerge()`. What's more, the merging of the executable flag there isn't actually necessary; it was made a no-op by the very next commit, i.e. 4234c9af515d (flags: read flag from dirstate/disk for workingcopyctx (issue5743), 2020-05-16). I found the overall flag-merging code (not the bit in `simplemerge.py`) very hard to follow, but I think I now finally understand how it works. `mergestate.resolve()` calculates the merged file flags and sets them on the local side of the merge (confusingly by calling `_restore_backup()`). Then it calls `filemerge.filemerge()`, which in turn calls `simplemerge.simplemerge()` (if premerge is enabled). That means that the flags on the local side `fcs.flags()` are already correct when the flag-merging code in `simplemerge.simplemerge()` runs. Interestingly, that code still works when the local side already has the merged value, it just doesn't change the value. Here's a truth table to explain why: BLOMCAR 0000000 0011111 0101011 0111111 1000000 1010000 1100000 1111101 B: Base L: Local O: Other M: Merged flags from `mergestate.resolve()`, i.e. what's called "local" when we get to `simplemerge.simplemerge()` C: `commonflags` in `simplemerge.simplemerge()`, i.e. `M & O` A: `addedflags` in `simplemerge.simplemerge()`, i.e. `(M ^ O) - B` R: Re-merged flags `simplemerge.simplemerge()`, i.e. `C | A` As you can see, the re-merged flags are always unchanged compared to the initial merged flags (R equals M). Therefore, this patch effectively backs out 84614212ae39 . (I might later refactor this code to have the flags explicitly passed in.) `simplemerge.simplemerge()` is also called from `contrib/simplemerge.py`, but that code never passes any flags. REPOSITORY rHG Mercurial BRANCH default REVISION DETAIL https://phab.mercurial-scm.org/D11879 AFFECTED FILES mercurial/simplemerge.py CHANGE DETAILS To: martinvonz, #hg-reviewers Cc: mercurial-patches, mercurial-devel diff --git a/mercurial/simplemerge.py b/mercurial/simplemerge.py --- a/mercurial/simplemerge.py +++ b/mercurial/simplemerge.py @@ -19,12 +19,10 @@ from __future__ import absolute_import from .i18n import _ -from .node import nullrev from . import ( error, mdiff, pycompat, - util, ) from .utils import stringutil @@ -424,12 +422,6 @@ return result -def is_not_null(ctx): - if not util.safehasattr(ctx, "node"): - return False - return ctx.rev() != nullrev - - def _mergediff(m3, name_a, name_b, name_base): lines = [] conflicts = False @@ -546,21 +538,13 @@ ) conflicts = m3.conflicts and not mode == b'union' - # merge flags if necessary - flags = localctx.flags() - localflags = set(pycompat.iterbytestr(flags)) - otherflags = set(pycompat.iterbytestr(otherctx.flags())) - if is_not_null(basectx) and localflags != otherflags: - baseflags = set(pycompat.iterbytestr(basectx.flags())) - commonflags = localflags & otherflags - addedflags = (localflags ^ otherflags) - baseflags - flags = b''.join(sorted(commonflags | addedflags)) - mergedtext = b''.join(lines) if opts.get('print'): ui.fout.write(mergedtext) else: - localctx.write(mergedtext, flags) + # localctx.flags() already has the merged flags (done in + # mergestate.resolve()) + localctx.write(mergedtext, localctx.flags()) if conflicts: return 1