Page MenuHomePhabricator

scmutil: allowing different files to be prefetched per revision
ClosedPublic

Authored by rdamazio on Fri, Jul 10, 2:45 AM.

Details

Summary

The old API takes a list of revision separate from the file matcher, and thus
provides no way to fetch different sets of files from each revision. In
preparation for adding one such usage, I'm changing the API to take a list of
(revision, file matcher) tuples instead.

Diff Detail

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

Event Timeline

rdamazio created this revision.Fri, Jul 10, 2:45 AM
pulkit accepted this revision.Fri, Jul 10, 5:56 AM
This revision is now accepted and ready to land.Fri, Jul 10, 5:56 AM

Should we have an entry in the release notes about this or have we given up on documenting API changes? I'm fine with either, as long as we have some policy.

mjacob added a subscriber: mjacob.Fri, Jul 10, 8:10 PM
mjacob added inline comments.
mercurial/context.py
2543–2549

test-check-format.t complains about this.

Should we have an entry in the release notes about this or have we given up on documenting API changes? I'm fine with either, as long as we have some policy.

I'm happy to send another change for the release notes, let me know what's decided on policy.

mercurial/context.py
2543–2549

Hmmm apparently I'm missing something in my setup, I ran all tests, but it output this:

Skipped test-check-format.t: missing feature: the black formatter for python

So sorry about that. How do you want me to fix? Send another commit on top?

rdamazio added inline comments.Fri, Jul 10, 8:38 PM
mercurial/context.py
2543–2549

Apparently the issue was that I had black 19.3b0, and the tests require 19.10b0.

mjacob added inline comments.Fri, Jul 10, 8:48 PM
mercurial/context.py
2543–2549

I’m not sure what the policy for this is. What happened recently in a similar situation is that another patch was sent and folded into the changeset creating the test failure (see https://phab.mercurial-scm.org/D8680#129685). I don’t know if that’s the preferred way or it was done because the patch for the fix was already sent.

rdamazio updated this revision to Diff 21844.Fri, Jul 10, 9:20 PM
rdamazio marked an inline comment as done.Fri, Jul 10, 9:20 PM
rdamazio added inline comments.
mercurial/context.py
2543–2549

Well, phabricator let me just upload the amended commits after applying the formatting. Hope this works.

rdamazio marked 2 inline comments as done.Fri, Jul 10, 9:21 PM

(also notice the formatting changed D8721, D8722 and D8723)

mjacob added inline comments.Fri, Jul 10, 9:35 PM
hgext/remotefilelog/__init__.py
1124

Sorry, I didn’t realize before that this patch created another test failure:

--- /home/manu/vcs/hg/tests/test-check-pyflakes.t
+++ /home/manu/vcs/hg/tests/test-check-pyflakes.t.err
@@ -23,4 +23,5 @@
   mercurial/hgweb/server.py:*:* undefined name 'reload' (glob) (?)
   mercurial/util.py:*:* undefined name 'file' (glob) (?)
   mercurial/encoding.py:*:* undefined name 'localstr' (glob) (?)
+  hgext/remotefilelog/__init__.py:1124:18 import 'match' from line 136 shadowed by loop variable

I don’t know why it complains about this but not the cases where the import is shadowed by parameters.

rdamazio added inline comments.Fri, Jul 10, 9:55 PM
hgext/remotefilelog/__init__.py
1124

Ugh:

Skipped test-check-pyflakes.t: missing feature: Pyflakes python linter

To be fair, the full list is this:

Skipped test-casecollision-merge.t: missing feature: case insensitive file system
Skipped test-casefolding.t: missing feature: case insensitive file system
Skipped test-check-clang-format.t: missing feature: clang-format C code formatter
Skipped test-check-format.t: missing feature: the black formatter for python
Skipped test-check-jshint.t: missing feature: JSHint static code analysis tool
Skipped test-check-pyflakes.t: missing feature: Pyflakes python linter
Skipped test-check-pylint.t: missing feature: Pylint python linter
Skipped test-check-rust-format.t: missing feature: rustfmt tool
Skipped test-chg.t: missing feature: running with chg
Skipped test-convert-baz.t: missing feature: GNU Arch baz client
Skipped test-convert-bzr-114.t: missing feature: Canonical's Bazaar client >= 1.14
Skipped test-convert-bzr-directories.t: missing feature: Canonical's Bazaar client
Skipped test-convert-bzr-ghosts.t: missing feature: Canonical's Bazaar client
Skipped test-convert-bzr-merges.t: missing feature: Canonical's Bazaar client
Skipped test-convert-bzr-treeroot.t: missing feature: Canonical's Bazaar client
Skipped test-convert-bzr.t: missing feature: Canonical's Bazaar client
Skipped test-convert-darcs.t: missing feature: darcs client
Skipped test-convert-hg-svn.t: missing feature: subversion python bindings
Skipped test-convert-mtn.t: missing feature: monotone client (>= 1.0)
Skipped test-convert-p4-filetypes.t: missing feature: Perforce server and client
Skipped test-convert-p4.t: missing feature: Perforce server and client
Skipped test-convert-svn-branches.t: missing feature: subversion python bindings
Skipped test-convert-svn-encoding.t: missing feature: subversion python bindings
Skipped test-convert-svn-move.t: missing feature: subversion python bindings
Skipped test-convert-svn-source.t: missing feature: subversion python bindings
Skipped test-convert-svn-startrev.t: missing feature: subversion python bindings
Skipped test-convert-svn-tags.t: missing feature: subversion python bindings
Skipped test-convert-tla.t: missing feature: GNU Arch tla client
Skipped test-debian-packages.t: missing feature: debian build dependencies (run dpkg-checkbuilddeps in contrib/)
Skipped test-docker-packaging.t: missing feature: docker support
Skipped test-fix-clang-format.t: missing feature: clang-format C code formatter
Skipped test-git-interop.t: missing feature: pygit2 Python library
Skipped test-lfs-test-server.t#git-server: missing feature: git-lfs test server
Skipped test-mac-packages.t: missing feature: OS X packaging tools
Skipped test-no-symlinks.t: system supports symbolic links
Skipped test-phabricator.t: missing feature: vcr http mocking library
Skipped test-releasenotes-formatting.t: missing feature: Fuzzy string matching library
Skipped test-releasenotes-merging.t: missing feature: Fuzzy string matching library
Skipped test-releasenotes-parsing.t: missing feature: Fuzzy string matching library
Skipped test-remotefilelog-bundle2-legacy.t: skipped
Skipped test-rhg.t: missing feature: Using the Rust extensions
Skipped test-sparse-fsmonitor.t: skipped
Skipped test-sparse-revlog.t: missing artifact, run "/<redacted>/hg/tests/artifacts/scripts/generate-churning-bundle.py"
Skipped test-sqlitestore.t: missing feature: sqlite3 module is available
Skipped test-verify-repo-operations.py: missing feature: allow slow tests (use --allow-slow-tests)
Skipped test-wireproto-exchangev2-shallow.t: missing feature: sqlite3 module is available

Where's the documentation on setting this up correctly with all the required tools? I don't see anything in https://www.mercurial-scm.org/wiki/ContributingChanges .

I'll install and fix the pyflakes issue.

rdamazio marked an inline comment as done.Fri, Jul 10, 10:08 PM
rdamazio updated this revision to Diff 21847.
rdamazio added inline comments.Fri, Jul 10, 10:08 PM
hgext/remotefilelog/__init__.py
1124

There were other offenses of this one in the same file, so renamed the import.

mjacob added inline comments.Fri, Jul 10, 10:14 PM
hgext/remotefilelog/__init__.py
1124

I don’t think that everyone has all the required tools installed. Even the buildbot didn’t catch the black issue (but it catched the pyflakes issue). Recently I found that hgext.convert.subversion was not working on Python 3 at all and nobody realized. More and more builders will be migrated to use Python 3. I think that would be a good chance to install most of the requirements.

Unless you work on C code, JS code or Rust code, I think that installing all Python-specific tools for tests/test-check-* is sufficient for most cases. The other failures will hopefully be catched by some buildbot even after it was queued. :)

rdamazio added inline comments.Fri, Jul 10, 10:37 PM
hgext/remotefilelog/__init__.py
1124

Makes sense.

Thanks for the review, let me know if you need me to do anything else before you can queue this.

mjacob added inline comments.Fri, Jul 10, 11:49 PM
hgext/remotefilelog/__init__.py
1124

I can’t queue, but @pulkit or others can do it. The changes to the three revisions look good, although it would probably be a bit better for looking at the history later if the import rename was in a separate changeset.

pulkit added inline comments.Sat, Jul 11, 4:44 AM
hgext/remotefilelog/__init__.py
1124

Replaced the existing patches with the new uploaded ones. Seems like I was also missing black and pyflakes on the machine I was testing.