This is an archive of the discontinued Mercurial Phabricator instance.

narrow: add '--import-rules' flag to tracked command
ClosedPublic

Authored by pulkit on Aug 6 2018, 7:07 AM.

Details

Summary

This patch adds a --import-rules flag to tracked command provided by narrow
extension. Using the --import-rules flag, you can pass a filename from which
narrowspecs should be read and added to main narrowspec.

A lot of times, in automation or manually also, when you are working with big
repo, specifying each path name on commandline using '--addinclude' and
'--addexclude' is tedious and something which can scale. So we needed something
where we can pass a file to extend the narrowspecs.

Nice thing about this is that the automations which reads some file to change
the sparse profile, can now read the same file for changing narrowspecs too.

Tests are added for the new feature.

Diff Detail

Repository
rHG Mercurial
Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

pulkit created this revision.Aug 6 2018, 7:07 AM
durin42 accepted this revision as: durin42.

Seems fine to me, though thinking we should probably spend some time to make sure the sparse and narrow patterns are cross-compatible like this change seems to hope for.

@martinvonz maybe take a look at this with @pulkit when you're back?

pulkit added a comment.Aug 9 2018, 2:25 PM

Seems fine to me, though thinking we should probably spend some time to make sure the sparse and narrow patterns are cross-compatible like this change seems to hope for.

I already did that. https://www.mercurial-scm.org/repo/hg-committed/rev/f64ebe7d2259

ping for review.

needs rebased, but looks fine to me

martinvonz added inline comments.Aug 22 2018, 1:33 PM
hgext/narrow/narrowcommands.py
330

nit: --extend may be too generic. How about --addfromfile? Can be done in a follow-up if you agree.

379

Why is it required to be in the working directory? Why not accept any kind of path? I don't know if it's intentional or not, but I think this also means that cd dir; hg tracked --extend foo will attempt to read foo from the repo root and not from the current directory, which may be confusing to users.

387

could you send a patch to fix the typoed "supported" on narrowspec.py too?

388–389

nit: is the list conversion necessary?

tests/test-narrow-trackedcmd.t
4–7

are treemanifests relevant to this test?

53–87

any reason not to add these from the beginning (i.e. before creating the clone)?

92

This is part of why I don't like --extend: it sounds like it can be given without a parameter

119

nit: s/>>/>/

pulkit planned changes to this revision.Aug 23 2018, 5:03 PM
pulkit added inline comments.
hgext/narrow/narrowcommands.py
330

I thought about --addfromfile and dropped it because you can both add and remove includes and excludes using this option. Add in other flags means adding either include or exclude specs.

I am fine if that looks good to you. I don't feel strongly.

379

That's a nice idea. I just copied sparse behavior here. I will update with suggested idea.

387

Sure!

388–389

not sure, I will check and remove if not required.

tests/test-narrow-trackedcmd.t
4–7

Nope, I will use testcase here and add test for both cases.

53–87

I just copied these from one of the other existing test files.

92

hm, do you have a functionality in mind for hg tracked --extend without a parameter?

martinvonz added inline comments.Aug 23 2018, 5:07 PM
hgext/narrow/narrowcommands.py
330

It looks like it just adds includes and excludes (but yes, adding excludes means removing files), so I think the name makes sense

tests/test-narrow-trackedcmd.t
92

Nope, nothing in mind :) It just sounds like it might do something without a parameter.

foozy added a subscriber: foozy.Aug 24 2018, 9:08 AM
foozy added inline comments.
hgext/narrow/narrowcommands.py
330

Just curious, is it intentional that the name of this option is different from --import-rules of debugsparse ?

pulkit added inline comments.Aug 24 2018, 10:06 AM
hgext/narrow/narrowcommands.py
330

I didn't know about that. That likes a good name, @martinvonz what do you think?

pulkit edited the summary of this revision. (Show Details)Aug 25 2018, 3:35 PM
pulkit retitled this revision from narrow: add '--extend' flag to tracked command to narrow: add '--import-rules' flag to tracked command.
pulkit updated this revision to Diff 10564.
pulkit added a comment.Sep 3 2018, 6:10 AM

gentle ping for review

martinvonz requested changes to this revision.Sep 4 2018, 11:36 AM
martinvonz added inline comments.
hgext/narrow/narrowcommands.py
377–381

Perhaps we should use util.readfile(). I saw that that's what e.g. hg commit --logfile does. See https://www.mercurial-scm.org/repo/hg/file/2df3271ef139/mercurial/cmdutil.py#l844 and other callers for examples.

(Even if we decided to not use that helper, we should probably move fp.read() inside the try-block.)

tests/test-narrow-trackedcmd.t
80

We should switch to {rev} here like I did for the other narrow tests in another patch

This revision now requires changes to proceed.Sep 4 2018, 11:36 AM
pulkit updated this revision to Diff 10741.Sep 4 2018, 12:11 PM
pulkit marked 2 inline comments as done.Sep 4 2018, 12:12 PM
martinvonz accepted this revision.Sep 4 2018, 12:17 PM
This revision is now accepted and ready to land.Sep 4 2018, 12:17 PM
This revision was automatically updated to reflect the committed changes.