This is an archive of the discontinued Mercurial Phabricator instance.

Differential D540

effectflag: detect when meta changed
ClosedPublic

Authored by lothiraldan on Aug 28 2017, 7:03 AM.

Details

Reviewers
None
Group Reviewers
hg-reviewers
Commits
rHG95759620d492: effectflag: detect when meta changed
Summary

Store in effect flag when the meta changed between the predecessor and its
successors. We blacklisted some known meta that would always changed when
another flag change. For example rebase would always add a meta rebase-source
while the effect flag parents will already detect this situation.

It can happens with various hg commands.

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

lothiraldan created this revision.Aug 28 2017, 7:03 AM
lothiraldan updated this revision to Diff 2111.Sep 27 2017, 4:08 AM
martinvonz added a subscriber: martinvonz.Sep 30 2017, 12:25 PM
martinvonz added inline comments.
mercurial/obsutil.py
327–330

"ismetablacklisted" and "...does *not* match ... blacklist" seems confusing. Shouldn't this function be called notmetablacklisted or metanotblacklisted or similar?

332–336

I think this is both shorter and clearer (but not tested):

return not any(pattern.match(metakey) for pattern in METABLACKLIST)
lothiraldan added inline comments.Oct 1 2017, 6:28 AM
mercurial/obsutil.py
327–330

Right, I think I reversed the return values without changing the name, I will find something.

lothiraldan updated this revision to Diff 2257.Oct 1 2017, 7:52 AM
lothiraldan marked 3 inline comments as done.Oct 1 2017, 7:57 AM
Closed by commit rHG95759620d492: effectflag: detect when meta changed (authored by lothiraldan). · Explain WhyOct 1 2017, 5:20 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

Diff 2311

mercurial/obsutil.py

# obsutil.py - utility functions for obsolescence # obsutil.py - utility functions for obsolescence
# #
# Copyright 2017 Boris Feld <boris.feld@octobus.net> # Copyright 2017 Boris Feld <boris.feld@octobus.net>
# #
# This software may be used and distributed according to the terms of the # This software may be used and distributed according to the terms of the
# GNU General Public License version 2 or any later version. # GNU General Public License version 2 or any later version.
from __future__ import absolute_import from __future__ import absolute_import
import re
from . import ( from . import (
phases, phases,
util util
) )
class marker(object): class marker(object):
"""Wrap obsolete marker raw data""" """Wrap obsolete marker raw data"""
known = (n for n in succs if n in nm) known = (n for n in succs if n in nm)
foreground = set(repo.set('%ln::', known)) foreground = set(repo.set('%ln::', known))
return set(c.node() for c in foreground) return set(c.node() for c in foreground)
# logic around storing and using effect flags # logic around storing and using effect flags
EFFECTFLAGFIELD = "ef1" EFFECTFLAGFIELD = "ef1"
DESCCHANGED = 1 << 0 # action changed the description DESCCHANGED = 1 << 0 # action changed the description
METACHANGED = 1 << 1 # action change the meta
PARENTCHANGED = 1 << 2 # action change the parent PARENTCHANGED = 1 << 2 # action change the parent
USERCHANGED = 1 << 4 # the user changed USERCHANGED = 1 << 4 # the user changed
DATECHANGED = 1 << 5 # the date changed DATECHANGED = 1 << 5 # the date changed
BRANCHCHANGED = 1 << 6 # the branch changed BRANCHCHANGED = 1 << 6 # the branch changed
METABLACKLIST = [
re.compile('^branch$'),
re.compile('^.*-source$'),
re.compile('^.*_source$'),
re.compile('^source$'),
]
def metanotblacklisted(metaitem):
""" Check that the key of a meta item (extrakey, extravalue) does not
match at least one of the blacklist pattern
"""
martinvonzUnsubmitted
Done

"ismetablacklisted" and "...does *not* match ... blacklist" seems confusing. Shouldn't this function be called notmetablacklisted or metanotblacklisted or similar?

martinvonz: "ismetablacklisted" and "...does *not* match ... blacklist" seems confusing. Shouldn't this…
lothiraldanAuthorUnsubmitted
Done

Right, I think I reversed the return values without changing the name, I will find something.

lothiraldan: Right, I think I reversed the return values without changing the name, I will find something.
metakey = metaitem[0]
return not any(pattern.match(metakey) for pattern in METABLACKLIST)
def geteffectflag(relation): def geteffectflag(relation):
""" From an obs-marker relation, compute what changed between the """ From an obs-marker relation, compute what changed between the
martinvonzUnsubmitted
Done

I think this is both shorter and clearer (but not tested):

return not any(pattern.match(metakey) for pattern in METABLACKLIST)
martinvonz: I think this is both shorter and clearer (but not tested): return not any(pattern.match…
predecessor and the successor. predecessor and the successor.
""" """
effects = 0 effects = 0
source = relation[0] source = relation[0]
for changectx in relation[1]: for changectx in relation[1]:
# Check if description has changed # Check if description has changed
# Check if branch has changed # Check if branch has changed
if changectx.branch() != source.branch(): if changectx.branch() != source.branch():
effects |= BRANCHCHANGED effects |= BRANCHCHANGED
# Check if at least one of the parent has changed # Check if at least one of the parent has changed
if changectx.parents() != source.parents(): if changectx.parents() != source.parents():
effects |= PARENTCHANGED effects |= PARENTCHANGED
# Check if other meta has changed
changeextra = changectx.extra().items()
ctxmeta = filter(metanotblacklisted, changeextra)
sourceextra = source.extra().items()
srcmeta = filter(metanotblacklisted, sourceextra)
if ctxmeta != srcmeta:
effects |= METACHANGED
return effects return effects
def getobsoleted(repo, tr): def getobsoleted(repo, tr):
"""return the set of pre-existing revisions obsoleted by a transaction""" """return the set of pre-existing revisions obsoleted by a transaction"""
torev = repo.unfiltered().changelog.nodemap.get torev = repo.unfiltered().changelog.nodemap.get
phase = repo._phasecache.phase phase = repo._phasecache.phase
succsmarkers = repo.obsstore.successors.get succsmarkers = repo.obsstore.successors.get
public = phases.public public = phases.public

tests/test-obsmarkers-effectflag.t

marked working directory as branch closedbranch marked working directory as branch closedbranch
$ mkcommit G0 $ mkcommit G0
$ mkcommit I0 $ mkcommit I0
$ hg commit --amend --close-branch $ hg commit --amend --close-branch
check result check result
$ hg debugobsolete -r . $ hg debugobsolete -r .
2f599e54c1c6974299065cdf54e1ad640bfb7b5d 12c6238b5e371eea00fd2013b12edce3f070928b 0 (Thu Jan 01 00:00:00 1970 +0000) {'ef1': '0', 'operation': 'amend', 'user': 'test'} 2f599e54c1c6974299065cdf54e1ad640bfb7b5d 12c6238b5e371eea00fd2013b12edce3f070928b 0 (Thu Jan 01 00:00:00 1970 +0000) {'ef1': '2', 'operation': 'amend', 'user': 'test'}