This is an archive of the discontinued Mercurial Phabricator instance.

Differential D11279

fix: add test to demonstrate how many times tools are executed
ClosedPublic

Authored by hooper on Aug 11 2021, 10:16 PM.

Details

Reviewers
Alphare
pulkit
Group Reviewers
hg-reviewers
Commits
rHG066cdec8f74f: fix: add test to demonstrate how many times tools are executed
Summary

The current implementation wastes a lot of effort invoking fixer tools more
than once on the same file name+content. It is good to track changes in this
behavior.

Diff Detail

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

Event Timeline

hooper created this revision.Aug 11 2021, 10:16 PM
Alphare requested changes to this revision.Aug 25 2021, 5:14 AM
Alphare added a subscriber: Alphare.
Alphare added inline comments.
tests/test-fix.t
1766

This probably should be done in a separate changeset.

This revision now requires changes to proceed.Aug 25 2021, 5:14 AM
hooper marked an inline comment as done.Sep 2 2021, 6:07 PM
hooper edited the summary of this revision. (Show Details)
hooper updated this revision to Diff 30178.
Alphare added a comment.Sep 8 2021, 9:17 AM

This stack needs rebased, sorry!

Alphare requested changes to this revision.Sep 9 2021, 5:10 AM
This revision now requires changes to proceed.Sep 9 2021, 5:10 AM
hooper updated this revision to Diff 30329.Sep 20 2021, 4:29 PM
pulkit accepted this revision.Oct 12 2021, 9:33 AM
hooper added a commit: rHG066cdec8f74f: fix: add test to demonstrate how many times tools are executed.Oct 12 2021, 1:19 PM
This revision was not accepted when it landed; it landed in state Needs Review.
Closed by commit rHG066cdec8f74f: fix: add test to demonstrate how many times tools are executed (authored by hooper). · Explain Why
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mtests/test-fix.t (98 lines)

Diff 29888

tests/test-fix.t

hello hello
r0.ignored: r0.ignored:
hello hello
r0.whole: r0.whole:
hello hello
We should execute the fixer tools as few times as possible, because they might
be slow or expensive to execute. The inputs to each execution are effectively
the file path, file content, and line ranges. So, we should be able to re-use
results whenever those inputs are repeated. That saves a lot of work when
fixing chains of commits that all have the same file revision for a path being
fixed.
$ hg init numberofinvocations
$ cd numberofinvocations
$ printf "bar1" > bar.log
$ printf "baz1" > baz.log
$ printf "foo1" > foo.log
$ printf "qux1" > qux.log
$ hg commit -Aqm "commit1"
$ printf "bar2" > bar.log
$ printf "baz2" > baz.log
$ printf "foo2" > foo.log
$ hg commit -Aqm "commit2"
$ printf "bar3" > bar.log
$ printf "baz3" > baz.log
$ hg commit -Aqm "commit3"
$ printf "bar4" > bar.log
$ LOGFILE=$TESTTMP/log
$ LOGGER=$TESTTMP/log.py
$ cat >> $LOGGER <<EOF
> # Appends the input file's name to the log file.
> import sys
> with open('$LOGFILE', 'a') as f:
> f.write(sys.argv[1] + '\n')
> sys.stdout.write(sys.stdin.read())
> EOF
$ hg fix --working-dir -r "all()" \
> --config "fix.log:command=\"$PYTHON\" \"$LOGGER\" {rootpath}" \
> --config "fix.log:pattern=glob:**.log"
$ cat $LOGFILE | sort | uniq -c
4 bar.log
4 baz.log
4 foo.log
4 qux.log
$ cd ..
For tools that support line ranges, it's wrong to blindly re-use fixed file
AlphareUnsubmitted
Done

This probably should be done in a separate changeset.

Alphare: This probably should be done in a separate changeset.
content for the same file revision if it appears twice with different baserevs,
because the line ranges could be different. Since computing line ranges is
ambiguous, this isn't a matter of correctness, but it affects the usability of
this extension. It could maybe be simpler if baserevs were computed on a
per-file basis to make this situation impossible to construct.
In the following example, we construct two subgraphs with the same file
revisions, and fix different sub-subgraphs to get different baserevs and
different changed line ranges. The key precondition is that revisions 1 and 4
have the same file revision, and the key result is that their successors don't
have the same file content, because we want to fix different areas of that same
file revision's content.
$ hg init differentlineranges
$ cd differentlineranges
$ printf "a\nb\n" > file.changed
$ hg commit -Aqm "0 ab"
$ printf "a\nx\n" > file.changed
$ hg commit -Aqm "1 ax"
$ hg remove file.changed
$ hg commit -Aqm "2 removed"
$ hg revert file.changed -r 0
$ hg commit -Aqm "3 ab (reverted)"
$ hg revert file.changed -r 1
$ hg commit -Aqm "4 ax (reverted)"
$ hg manifest --debug --template "{hash}\n" -r 0; \
> hg manifest --debug --template "{hash}\n" -r 3
418f692145676128d2fb518b027ddbac624be76e
418f692145676128d2fb518b027ddbac624be76e
$ hg manifest --debug --template "{hash}\n" -r 1; \
> hg manifest --debug --template "{hash}\n" -r 4
09b8b3ce5a507caaa282f7262679e6d04091426c
09b8b3ce5a507caaa282f7262679e6d04091426c
$ hg fix --working-dir -r 1+3+4
3 new orphan changesets
$ hg cat file.changed -r "successors(1)" --hidden
a
X
$ hg cat file.changed -r "successors(4)" --hidden
A
X
$ cd ..