D6613: commit: improve the files field of changelog for merges (RFC)

valentin.gatienbaron (Valentin Gatien-Baron) phabricator at mercurial-scm.org
Wed Jul 10 20:52:01 EDT 2019


valentin.gatienbaron added a comment.


  In D6613#96908 <https://phab.mercurial-scm.org/D6613#96908>, @mharbison72 wrote:
  
  > In D6613#96822 <https://phab.mercurial-scm.org/D6613#96822>, @valentin.gatienbaron wrote:
  >
  >> Ok. Maybe it would be simpler or more robust to do the direct thing: optionally treat the files list in the input commit as input and reuse them blindly in the resulting commit, when doing a hg->hg conversion without filemap.
  >
  > Maybe.  I've been wondering if it's possible to just pass along the manifest and changelog instead of recalculating it.  It seems there have been other issues over the years[1].  @yuja fixed something manifest related around the time of that thread IIRC.  Even if a filemap is in use, the map may not modify early commits.  And when those are changed unexpectedly, it makes me wonder what got lost/mangled.  I looked through the repos I converted last year without any file mapping, and there were manifest node changes, but also differences in `files` and `files+` in the changelog.
  
  Ideally, the whole file list would move out the changelog. Then changing the list of modified files wouldn't change hashes anymore, but of course at the cost of changing everything once.
  Given that the filelist in the commit is (ignoring changes to mercurial) a function of the manifest, it may be reasonable to make convert reuse the filelist if the manifestnode gets reused. Or to put it differently, if the conversion is the identity so far for a commit, keep behaving as the identity.
  
  >> (btw, a tweak to test-merge-combinations shows the cases where convert is not the identity: 12-- and -1--, so cases where a file is absent in p1, that p2 added or modified, and the merge redeletes the file. Commit shows such files are modified (rightly) but convert doesn't)
  >
  > Is that a non-tracked test?  Can you throw a patch up somewhere showing this?  One of the things I noticed last year was that I'd get better results if `cleanp2`[2] was forced to be empty.  I was unable to come up with a simple test case showing a different hash with shipping code and a forcibly emptied `cleanp2`, circa 4.6.  What you're describing seems to be the opposite of 216fa1ba9993 <https://phab.mercurial-scm.org/rHG216fa1ba999333e86200d2608e3b78670168d516>, but something is wrong with that calculation, so maybe it's worth a try.
  
  Saying "the cases where convert misbehaves" is probably optimistic. It shows some cases where convert misbehaves. test-merge-combination.t is in the previous differential. cleanp2 seems orthogonal, as fiddling with it doesn't change what I see. This is the tweak I was talking about (based on this differential):
  
    diff --git a/tests/test-merge-combination.t b/tests/test-merge-combination.t
    --- a/tests/test-merge-combination.t
    +++ b/tests/test-merge-combination.t
    @@ -58,9 +58,16 @@ revision. "C" indicates that hg merge ha
       >           else expected=a
       >           fi
       >           got=`hg log -r 3 --template '{files}\n' | tr --delete 'e '`
    +  >           hg --config extensions.convert= convert . repo-convert -s hg -d hg --sourcesort -q
    +  >           got2=`hg --cwd repo-convert log -r 3 --template '{files}\n' | tr --delete 'e '`
       >           if [ "$got" = "$expected" ]
    -  >           then echo "$line$conflicts: agree on \"$got\""
    -  >           else echo "$line$conflicts: hg said \"$got\", expected \"$expected\""
    +  >           then echo -n "$line$conflicts: agree on \"$got\""
    +  >           else echo -n "$line$conflicts: hg said \"$got\", expected \"$expected\""
    +  >           fi
    +  >           if [ "$got" = "$got2" ]; then
    +  >             echo
    +  >           else
    +  >             echo ", commit:\"$got\" vs convert:\"$got2\""
       >           fi
       >           cd ../
       >           rm -rf repo
    @@ -115,7 +122,7 @@ All the merges of various file contents.
       12-1 C: agree on "a"
       12-2 C: hg said "", expected "a"
       12-3 C: agree on "a"
    -  12-- C: agree on "a"
    +  12-- C: agree on "a", commit:"a" vs convert:""
       1-11  : hg said "", expected "a"
       1-12  : agree on "a"
       1-1-  : agree on ""
    @@ -135,7 +142,7 @@ All the merges of various file contents.
       -12- C: agree on "a"
       -1-1  : agree on ""
       -1-2  : agree on "a"
    -  -1--  : agree on "a"
    +  -1--  : agree on "a", commit:"a" vs convert:""
       --11  : agree on ""
       --12  : agree on "a"
       --1-  : agree on "a"

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D6613/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D6613

To: valentin.gatienbaron, #hg-reviewers, mharbison72
Cc: JordiGH, yuja, mharbison72, martinvonz, mjpieters, mercurial-devel


More information about the Mercurial-devel mailing list