This is an archive of the discontinued Mercurial Phabricator instance.

Differential D9127

changing-files: add clean computation of changed files for linear changesets
ClosedPublic

Authored by marmoute on Sep 30 2020, 9:11 AM.

Details

Reviewers
Alphare
pulkit
Group Reviewers
hg-reviewers
Commits
rHG0303fc1f43f8: changing-files: add clean computation of changed files for linear changesets
rHG8b7dbf153157: changing-files: add clean computation of changed files for linear changesets
rHGfc1b2b98762c: changing-files: add clean computation of changed files for linear changesets
Summary

The files field is not reliable, so we need to compute things from scratch. We
deal with the second simplest case, linear changesets. We diff the current
manifest with the parent manifest. This reveal the file added, changed and
removed.

Diff Detail

Repository
rHG Mercurial
Branch
default
Lint
No Linters Available
Unit
No Unit Test Coverage

Event Timeline

marmoute created this revision.Sep 30 2020, 9:11 AM
marmoute updated this revision to Diff 22986.Oct 2 2020, 3:14 AM
marmoute updated this revision to Diff 22994.Oct 2 2020, 5:09 AM
Alphare accepted this revision.Oct 2 2020, 5:45 AM
Alphare added a subscriber: Alphare.

I think _process_linear is missing one or two inline comments because it's hard to read if you don't know the diff interface by heart, but otherwise it looks good.

mercurial/metadata.py
238

Fascinating

276

I understand that from a performance perspective this kind of indexing is good, but I think a comment on what d[1][0] is would improve readability by a lot.

marmoute updated this revision to Diff 23014.Oct 2 2020, 12:40 PM
pulkit accepted this revision.Oct 7 2020, 8:10 AM
This revision is now accepted and ready to land.Oct 7 2020, 8:10 AM

Revision Contents
Changeset List

PathPackages
Mmercurial/metadata.py (46 lines)

Diff 23014

mercurial/metadata.py

def compute_all_files_changes(ctx): def compute_all_files_changes(ctx):
"""compute the files changed by a revision""" """compute the files changed by a revision"""
p1 = ctx.p1() p1 = ctx.p1()
p2 = ctx.p2() p2 = ctx.p2()
if p1.rev() == node.nullrev and p2.rev() == node.nullrev: if p1.rev() == node.nullrev and p2.rev() == node.nullrev:
return _process_root(ctx) return _process_root(ctx)
elif p1.rev() != node.nullrev and p2.rev() == node.nullrev:
return _process_linear(p1, ctx)
elif p1.rev() == node.nullrev and p2.rev() != node.nullrev:
# In the wild, one can encounter changeset where p1 is null but p2 is not
return _process_linear(p1, ctx, parent=2)
elif p1.rev() == p2.rev():
AlphareUnsubmitted
Not Done

Fascinating

Alphare: Fascinating
# In the wild, one can encounter such "non-merge"
return _process_linear(p1, ctx)
filescopies = computechangesetcopies(ctx) filescopies = computechangesetcopies(ctx)
filesadded = computechangesetfilesadded(ctx) filesadded = computechangesetfilesadded(ctx)
filesremoved = computechangesetfilesremoved(ctx) filesremoved = computechangesetfilesremoved(ctx)
filesmerged = computechangesetfilesmerged(ctx) filesmerged = computechangesetfilesmerged(ctx)
files = ChangingFiles() files = ChangingFiles()
files.update_touched(ctx.files()) files.update_touched(ctx.files())
files.update_added(filesadded) files.update_added(filesadded)
files.update_removed(filesremoved) files.update_removed(filesremoved)
# Simple, there was nothing before it, so everything is added. # Simple, there was nothing before it, so everything is added.
md = ChangingFiles() md = ChangingFiles()
manifest = ctx.manifest() manifest = ctx.manifest()
for filename in manifest: for filename in manifest:
md.mark_added(filename) md.mark_added(filename)
return md return md
def _process_linear(parent_ctx, children_ctx, parent=1):
"""compute the appropriate changed files for a changeset with a single parent
"""
md = ChangingFiles()
parent_manifest = parent_ctx.manifest()
children_manifest = children_ctx.manifest()
copies_candidate = []
for filename, d in parent_manifest.diff(children_manifest).items():
if d[1][0] is None:
AlphareUnsubmitted
Not Done

I understand that from a performance perspective this kind of indexing is good, but I think a comment on what d[1][0] is would improve readability by a lot.

Alphare: I understand that from a performance perspective this kind of indexing is good, but I think a…
# no filenode for the "new" value, file is absent
md.mark_removed(filename)
else:
copies_candidate.append(filename)
if d[0][0] is None:
# not filenode for the "old" value file was absent
md.mark_added(filename)
else:
# filenode for both "old" and "new"
md.mark_touched(filename)
if parent == 1:
copied = md.mark_copied_from_p1
elif parent == 2:
copied = md.mark_copied_from_p2
else:
assert False, "bad parent value %d" % parent
for filename in copies_candidate:
copy_info = children_ctx[filename].renamed()
if copy_info:
source, srcnode = copy_info
copied(source, filename)
return md
def computechangesetfilesadded(ctx): def computechangesetfilesadded(ctx):
"""return the list of files added in a changeset """return the list of files added in a changeset
""" """
added = [] added = []
for f in ctx.files(): for f in ctx.files():
if not any(f in p for p in ctx.parents()): if not any(f in p for p in ctx.parents()):
added.append(f) added.append(f)
return added return added