This is an archive of the discontinued Mercurial Phabricator instance.

Differential D6367

context: add ctx.files{modified,added,removed}() methods
ClosedPublic

Authored by martinvonz on May 11 2019, 3:18 AM.

Details

Reviewers
pulkit
Group Reviewers
hg-reviewers
Commits
rHG65fa61ca20af: context: add ctx.files{modified,added,removed}() methods
Summary

Changeset-centric copy tracing is currently very slow because it often
reads manifests. One place it needs the manifest is in _chain(), where
it removes a copy X->Y if Y has subsequently gotten removed. I want to
speed that up by keeping track directly in the changeset of which
files are removed in the changeset. These methods will be similar to
ctx.p[12]copies() in that way: they will either read from the
changeset or calculate the information from the manifests otherwise.

Note that these are different from ctx.{modified,added,removed}() on
merge commits. Those functions always compare to p1, but the new ones
compare to both parents. filesadded() means "file does not exist in
either parent but exists now", filesremoved() means "file existed in
either parent but does not exist now", and filesmodified() means "file
existed in either parent and still exists". The set of files in
ctx.files() is the union of the files from the three new functions
(and the three new ones are all disjoint sets).

Also note that uncommitted merges are weird as usual. The invariant
mentioned above still holds, but the functions compare to p1 (and are
thus identical to the existing methods).

Diff Detail

Repository
rHG Mercurial
Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

martinvonz created this revision.May 11 2019, 3:18 AM
yuja added a subscriber: yuja.May 11 2019, 11:07 PM

+ def modified(self):
+ modified = set(self.files())
+ modified.difference_update(self.added())
+ modified.difference_update(self.removed())
+ return sorted(modified)
+ def added(self):
+ added = []
+ for f in self.files():
+ if not any(f in p for p in self.parents()):
+ added.append(f)
+ return added
+ def removed(self):
+ removed = []
+ for f in self.files():
+ if f not in self:
+ removed.append(f)
+ return removed

Do these definitions agree with the committablectx ones?
I think the committablectx calculates them based on the status from p1.

martinvonz added a comment.May 12 2019, 1:07 PM

*From: *yuja (Yuya Nishihara) <phabricator@mercurial-scm.org>
*Date: *Sat, May 11, 2019, 20:12
*To: * <mercurial-devel@mercurial-scm.org>

yuja added a comment.

> +    def modified(self):
>  +        modified = set(self.files())
>  +        modified.difference_update(self.added())
>  +        modified.difference_update(self.removed())
>  +        return sorted(modified)
>  +    def added(self):
>  +        added = []
>  +        for f in self.files():
>  +            if not any(f in p for p in self.parents()):
>  +                added.append(f)
>  +        return added
>  +    def removed(self):
>  +        removed = []
>  +        for f in self.files():
>  +            if f not in self:
>  +                removed.append(f)
>  +        return removed
Do these definitions agree with the committablectx ones?
I think the committablectx calculates them based on the status from p1.

Oh, good point. I suppose I should rename these to something like
files{modified,added,removed} since they're slightly different.

REPOSITORY

rHG Mercurial

REVISION DETAIL

https://phab.mercurial-scm.org/D6367

To: martinvonz, hg-reviewers
Cc: yuja, mercurial-devel

Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

martinvonz retitled this revision from context: add modified(), added(), removed() to changectx too to context: add ctx.files{modified,added,removed}() methods.May 14 2019, 6:58 PM
martinvonz edited the summary of this revision. (Show Details)
martinvonz updated this revision to Diff 15079.
martinvonz updated this revision to Diff 15151.May 16 2019, 1:41 PM
pulkit accepted this revision.May 25 2019, 1:09 PM
Closed by commit rHG65fa61ca20af: context: add ctx.files{modified,added,removed}() methods (authored by martinvonz). · Explain WhyMay 25 2019, 6:54 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mmercurial/context.py (18 lines)

Diff 15061

mercurial/context.py

return self._changeset.manifest return self._changeset.manifest
def user(self): def user(self):
return self._changeset.user return self._changeset.user
def date(self): def date(self):
return self._changeset.date return self._changeset.date
def files(self): def files(self):
return self._changeset.files return self._changeset.files
def modified(self):
modified = set(self.files())
modified.difference_update(self.added())
modified.difference_update(self.removed())
return sorted(modified)
def added(self):
added = []
for f in self.files():
if not any(f in p for p in self.parents()):
added.append(f)
return added
def removed(self):
removed = []
for f in self.files():
if f not in self:
removed.append(f)
return removed
@propertycache @propertycache
def _copies(self): def _copies(self):
source = self._repo.ui.config('experimental', 'copies.read-from') source = self._repo.ui.config('experimental', 'copies.read-from')
p1copies = self._changeset.p1copies p1copies = self._changeset.p1copies
p2copies = self._changeset.p2copies p2copies = self._changeset.p2copies
# If config says to get copy metadata only from changeset, then return # If config says to get copy metadata only from changeset, then return
# that, defaulting to {} if there was no copy metadata. # that, defaulting to {} if there was no copy metadata.
# In compatibility mode, we return copy data from the changeset if # In compatibility mode, we return copy data from the changeset if