This is an archive of the discontinued Mercurial Phabricator instance.

prefetch: do not attempt to prefetch trees for draft commits
ClosedPublic

Authored by singhsrb on Nov 17 2017, 3:57 PM.
Tags
None
Subscribers

Details

Summary

After D1417, hg prefetch takes care of downloading both the files
and trees during the prefetch. However, when the command is run without any
options, it attempts to prefetch the trees for the draft commits which results
in an error. We should not even attempt to prefetch trees for the draft
commits.

Test Plan

Added a test to detect this case and ran all the tests.

Diff Detail

Repository
rFBHGX Facebook Mercurial Extensions
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

singhsrb created this revision.Nov 17 2017, 3:57 PM
Herald added a reviewer: Restricted Project. · View Herald TranscriptNov 17 2017, 3:57 PM
durham accepted this revision.Nov 17 2017, 4:30 PM
durham added a subscriber: durham.
durham added inline comments.
tests/test-treemanifest-prefetch.t
494

No trees were requested by it still printed this output? That seems weird. Maybe a bug (not related to this change though).

treemanifest/__init__.py
1051–1052

I think you can do something like

revset = revsetlang.formatspec('%r - draft()', opts.get('rev'))
revs = scmutil.revrange(repo, revset)

to avoid doing two revsets. Not a big deal though.

This revision is now accepted and ready to land.Nov 17 2017, 4:30 PM
singhsrb added inline comments.Nov 17 2017, 5:17 PM
tests/test-treemanifest-prefetch.t
494

Yeah I thought that was weird too. I will fix this in a separate change!

treemanifest/__init__.py
1051–1052

I really wanted to do that but opts.rev('rev') is a list of revsets (or ints) which makes this difficult. Also, revrange performs an OR operation on the list of revsets, so we will have to use something else for our operation if we want to negate in one of the revsets.

quark added a subscriber: quark.Nov 17 2017, 11:43 PM
quark added inline comments.
treemanifest/__init__.py
1052–1057

revrange returns a smartset, which could be parsed as a list of revisions. formatspec takes %ld to format a list of revisions. There is also repo.ctx, which is a thin layer around repo.revs to get context objects:

revs = scmutil.revrange(repo, opts.get('rev'))
spec = revsetlang.formatspec('%ld & public()', revs)
mfnodes = set(ctx.manifestnode() for ctx in repo.set(spec))

& public() is better since it also excludes secret commits.

This comment was removed by singhsrb.
singhsrb added inline comments.Nov 18 2017, 12:45 AM
treemanifest/__init__.py
1052–1057

That’s perfect! I will make this change separately! Thanks @quark.

This revision was automatically updated to reflect the committed changes.