Patchwork D11879: simplemerge: stop merging file flags

login
register
mail settings
Submitter phabricator
Date Dec. 7, 2021, 5:03 p.m.
Message ID <differential-rev-PHID-DREV-a3bncvi6hn5go7rl5ao3-req@mercurial-scm.org>
Download mbox | patch
Permalink /patch/50204/
State New
Headers show

Comments

phabricator - Dec. 7, 2021, 5:03 p.m.
martinvonz created this revision.
Herald added a reviewer: hg-reviewers.
Herald added a subscriber: mercurial-patches.

REVISION SUMMARY
  As 384df4db6520 <https://phab.mercurial-scm.org/rHG384df4db652099c6ce5ccddd3a8551d68bcf920e> (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 <https://phab.mercurial-scm.org/rHG84614212ae3984666a9754f7c335f7813691fcf1> (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 <https://phab.mercurial-scm.org/rHG4234c9af515d61b486043dbf9ea6c65804b4c9d1> (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 <https://phab.mercurial-scm.org/rHG84614212ae3984666a9754f7c335f7813691fcf1>. (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

Patch

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