[PATCH 10 of 10] effectflag: detect when diff changed

[Christophe de Vienne]

Hello,

As a end-user I encountered some situations were having such information
about changeset not available in my local repo would have help.

The main situation I recall is a colleague doing multiple amend or
rebase of several changesets. On my side I had made other rebase &
amends (lack of communication I guess) and pushed. When he pulled we
ended up with several divergent changeset that were not trivial to sort.
To solve the issue I pulled everything locally and had to compare the
divergent changeset diffs one by one.

Having some more information about obsolete (and not available) cset in
my repository would have help a lot to decide which version of the
changeset to take. Like who did what (amend ? rebase ?).

Le 09/07/2017 à 22:17, quark at fb.com (Jun Wu) a écrit :
> It's not very clear to me that what do we plan to use the effectflags.
> 
> I guess the most useful case is divergent resolution, like Alice changes
> message, Bob changes content, so we know what to do. But a chain of changes
> could cancel effects with each other, like:
> 
>   A0 - (amend by Alice) -> A1 - (amend by Alice) -> A2
>    \
>     -- (a chain of rewrites by Bob) -> A9
> 
> "A0 -> A1" changes diff content, "A1 -> A2" changes diff content. But
> "A0 -> A2" does not necessarily change diff content.
> 
> This is a divergent case where A0 has two visible successors: A2 and A9. A2
> does not have diff content change but with effectflags, A2 might be treated
> as having diff content change.
> 
> I think we should store the hash of actual diff text as metadata to detect
> diff changes across a chain of rewrites. That hash could be stored in a
> cache.
> 
> For other information like branch, desc, meta, parents. Since reading them
> is very fast using changelog, I'm not sure about the benefit of storing them
> in obsmarkers. It might be helpful to provide extra information for nodes
> unknown to local changelog (I'd also argue those markers are meaningless and
> should be hidden from the user, but that's another topic), but I don't think
> this feature is useful for an end-user.
> 
> 
> Excerpts from Boris Feld's message of 2017-07-07 14:38:39 +0200:
>> # HG changeset patch
>> # User Boris Feld <boris.feld at octobus.net>
>> # Date 1499346007 -7200
>> #      Thu Jul 06 15:00:07 2017 +0200
>> # Node ID 449fc1c748c6e058e892a4c940e20137e52e7808
>> # Parent  6a40d87dfedcce4064eb4bcdb131ed4d427fd4de
>> # EXP-Topic effectflag
>> effectflag: detect when diff changed
>>
>> Store in effect flag when the diff changed between the predecessor and
>> its successors.
>>
>> Comparing the diff is not easy because we do not want to incorrectly detect a
>> diff modification when the changeset has only been rebased.
>>
>> diff -r 6a40d87dfedc -r 449fc1c748c6 mercurial/obsutil.py
>> --- a/mercurial/obsutil.py    Thu Jul 06 14:58:44 2017 +0200
>> +++ b/mercurial/obsutil.py    Thu Jul 06 15:00:07 2017 +0200
>> @@ -542,6 +542,7 @@
>>  DESCCHANGED = 1 << 0 # action changed the description
>>  METACHANGED = 1 << 1 # action change the meta
>>  PARENTCHANGED = 1 << 2 # action change the parent
>> +DIFFCHANGED = 1 << 3 # action change diff introduced by the changeset
>>  USERCHANGED = 1 << 4 # the user changed
>>  DATECHANGED = 1 << 5 # the date changed
>>  BRANCHCHANGED = 1 << 6 # the branch changed
>> @@ -565,6 +566,46 @@
>>  
>>      return True
>>  
>> +def _prepare_hunk(hunk):
>> +    """Drop all information but the username and patch"""
>> +    cleanunk = []
>> +    for line in hunk.splitlines():
>> +        if line.startswith(b'# User') or not line.startswith(b'#'):
>> +            if line.startswith(b'@@'):
>> +                line = b'@@\n'
>> +            cleanunk.append(line)
>> +    return cleanunk
>> +
>> +def _getdifflines(iterdiff):
>> +    """return a cleaned up lines"""
>> +    try:
>> +        lines = next(iterdiff)
>> +    except StopIteration:
>> +        return None
>> +    return _prepare_hunk(lines)
>> +
>> +def _cmpdiff(leftctx, rightctx):
>> +    """return True if both ctx introduce the "same diff"
>> +
>> +    This is a first and basic implementation, with many shortcoming.
>> +    """
>> +
>> +    # Leftctx or right ctx might be filtered, so we need to use the contexts
>> +    # with an unfiltered repository to safely compute the diff
>> +    leftunfi = leftctx._repo.unfiltered()[leftctx.rev()]
>> +    leftdiff = leftunfi.diff(git=1)
>> +    rightunfi = rightctx._repo.unfiltered()[rightctx.rev()]
>> +    rightdiff = rightunfi.diff(git=1)
>> +
>> +    left, right = (0, 0)
>> +    while None not in (left, right):
>> +        left = _getdifflines(leftdiff)
>> +        right = _getdifflines(rightdiff)
>> +
>> +        if left != right:
>> +            return False
>> +    return True
>> +
>>  def geteffectflag(relation):
>>      """ From an obs-marker relation, compute what changed between the
>>      predecessor and the successor.
>> @@ -604,4 +645,12 @@
>>          if ctxmeta != srcmeta:
>>              effects |= METACHANGED
>>  
>> +        # Check if at least one of the parent has changed
>> +        if changectx.parents() != source.parents():
>> +            effects |= PARENTCHANGED
>> +
>> +        # Check if the diff has changed
>> +        if not _cmpdiff(source, changectx):
>> +            effects |= DIFFCHANGED
>> +
>>      return effects
>> diff -r 6a40d87dfedc -r 449fc1c748c6 tests/test-obsmarkers-effectflag.t
>> --- a/tests/test-obsmarkers-effectflag.t    Thu Jul 06 14:58:44 2017 +0200
>> +++ b/tests/test-obsmarkers-effectflag.t    Thu Jul 06 15:00:07 2017 +0200
>> @@ -95,7 +95,7 @@
>>  
>>    $ hg debugobsolete --rev .
>>    d6f4d8b8d3c8cde990f13915bced7f92ce1cc54f 0 {ebfe0333e0d96f68a917afd97c0a0af87f1c3b5f} (Thu Jan 01 00:00:00 1970 +0000) {'ef1': '0', 'user': 'test'}
>> -  ebfe0333e0d96f68a917afd97c0a0af87f1c3b5f 75781fdbdbf58a987516b00c980bccda1e9ae588 0 (Thu Jan 01 00:00:00 1970 +0000) {'ef1': '0', 'user': 'test'}
>> +  ebfe0333e0d96f68a917afd97c0a0af87f1c3b5f 75781fdbdbf58a987516b00c980bccda1e9ae588 0 (Thu Jan 01 00:00:00 1970 +0000) {'ef1': '8', 'user': 'test'}
>>  
>>  amend with multiple effect (desc and meta)
> 

-- 
Christophe de Vienne