This is an archive of the discontinued Mercurial Phabricator instance.

Differential D6163

copies: extract function for deciding whether to use changeset-centric algos
ClosedPublic

Authored by martinvonz on Mar 20 2019, 3:03 PM.

Details

Reviewers
None
Group Reviewers
hg-reviewers
Commits
rHG27475ae67676: copies: extract function for deciding whether to use changeset-centric algos
Summary

We'll eventually have a "experimental.copies.read-from=changeset-only"
option too and I don't want to spread the logic for determining if we
should use changeset-centric of filelog-centric algorithms.

Diff Detail

Repository
rHG Mercurial
Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

martinvonz created this revision.Mar 20 2019, 3:03 PM
pulkit added a subscriber: pulkit.Mar 20 2019, 3:11 PM
pulkit added inline comments.
mercurial/copies.py
163

maybe we need to have D2010, but we can drop algo (and maybe use also) from the name I guess.

martinvonz added inline comments.Mar 20 2019, 3:17 PM
mercurial/copies.py
163

I initially didn't have use and found it less clear. I also think "use changeset-centric" sounds weird. Maybe just changesetcentric as you said then, but that also seems unclear. It feels like we're working around D2010. I can accept that we don't want to queue that, but I don't really like any of the shorter names either. Maybe the current name is fine given the docstring?

martinvonz added inline comments.Apr 2 2019, 12:49 PM
mercurial/copies.py
163

Or maybe there's another name that would work? Maybe usecontextbasedalgo? Or shouldgetcopiesfromcontext? Other suggestions?

pulkit added inline comments.Apr 3 2019, 7:49 AM
mercurial/copies.py
163

I don't have any concerns on name in specific. I was trying to work around D2010 in the above suggestion.

martinvonz added inline comments.Apr 3 2019, 9:04 AM
mercurial/copies.py
163

Does that mean you're okay with the current form of this patch?

pulkit added inline comments.Apr 4 2019, 1:34 PM
mercurial/copies.py
163

Yes.

Closed by commit rHG27475ae67676: copies: extract function for deciding whether to use changeset-centric algos (authored by martinvonz). · Explain WhyApr 13 2019, 7:45 AM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mmercurial/copies.py (7 lines)
Mmercurial/scmutil.py (2 lines)

Diff 14575

mercurial/copies.py

"""Computes which files are in b but not a. """Computes which files are in b but not a.
This is its own function so extensions can easily wrap this call to see what This is its own function so extensions can easily wrap this call to see what
files _forwardcopies is about to process. files _forwardcopies is about to process.
""" """
ma = a.manifest() ma = a.manifest()
mb = b.manifest() mb = b.manifest()
return mb.filesnotin(ma, match=match) return mb.filesnotin(ma, match=match)
def usechangesetcentricalgo(repo):
pulkitUnsubmitted
Not Done

maybe we need to have D2010, but we can drop algo (and maybe use also) from the name I guess.

pulkit: maybe we need to have D2010, but we can drop `algo` (and maybe `use` also) from the name I…
martinvonzAuthorUnsubmitted
Not Done

I initially didn't have use and found it less clear. I also think "use changeset-centric" sounds weird. Maybe just changesetcentric as you said then, but that also seems unclear. It feels like we're working around D2010. I can accept that we don't want to queue that, but I don't really like any of the shorter names either. Maybe the current name is fine given the docstring?

martinvonz: I initially didn't have `use` and found it less clear. I also think "use changeset-centric"…
martinvonzAuthorUnsubmitted
Not Done

Or maybe there's another name that would work? Maybe usecontextbasedalgo? Or shouldgetcopiesfromcontext? Other suggestions?

martinvonz: Or maybe there's another name that would work? Maybe `usecontextbasedalgo`? Or…
pulkitUnsubmitted
Not Done

I don't have any concerns on name in specific. I was trying to work around D2010 in the above suggestion.

pulkit: I don't have any concerns on name in specific. I was trying to work around D2010 in the above…
martinvonzAuthorUnsubmitted
Not Done

Does that mean you're okay with the current form of this patch?

martinvonz: Does that mean you're okay with the current form of this patch?
pulkitUnsubmitted
Not Done

Yes.

pulkit: Yes.
"""Checks if we should use changeset-centric copy algorithms"""
return (repo.ui.config('experimental', 'copies.read-from') ==
'compatibility')
def _committedforwardcopies(a, b, match): def _committedforwardcopies(a, b, match):
"""Like _forwardcopies(), but b.rev() cannot be None (working copy)""" """Like _forwardcopies(), but b.rev() cannot be None (working copy)"""
# files might have to be traced back to the fctx parent of the last # files might have to be traced back to the fctx parent of the last
# one-side-only changeset, but not further back than that # one-side-only changeset, but not further back than that
repo = a._repo repo = a._repo
if repo.ui.config('experimental', 'copies.read-from') == 'compatibility': if usechangesetcentricalgo(repo):
return _changesetforwardcopies(a, b, match) return _changesetforwardcopies(a, b, match)
debug = repo.ui.debugflag and repo.ui.configbool('devel', 'debug.copies') debug = repo.ui.debugflag and repo.ui.configbool('devel', 'debug.copies')
dbg = repo.ui.debug dbg = repo.ui.debug
if debug: if debug:
dbg('debug.copies: looking into rename from %s to %s\n' dbg('debug.copies: looking into rename from %s to %s\n'
% (a, b)) % (a, b))
limit = _findlimit(repo, a, b) limit = _findlimit(repo, a, b)

mercurial/scmutil.py

wctx = repo[None] wctx = repo[None]
with repo.wlock(): with repo.wlock():
wctx.forget(deleted) wctx.forget(deleted)
wctx.add(unknown) wctx.add(unknown)
for new, old in renames.iteritems(): for new, old in renames.iteritems():
wctx.copy(old, new) wctx.copy(old, new)
def getrenamedfn(repo, endrev=None): def getrenamedfn(repo, endrev=None):
if repo.ui.config('experimental', 'copies.read-from') == 'compatibility': if copiesmod.usechangesetcentricalgo(repo):
def getrenamed(fn, rev): def getrenamed(fn, rev):
ctx = repo[rev] ctx = repo[rev]
p1copies = ctx.p1copies() p1copies = ctx.p1copies()
if fn in p1copies: if fn in p1copies:
return p1copies[fn] return p1copies[fn]
p2copies = ctx.p2copies() p2copies = ctx.p2copies()
if fn in p2copies: if fn in p2copies:
return p2copies[fn] return p2copies[fn]