Page MenuHomePhabricator

narrow: make hg manifest only print things inside the narrowspec
Needs RevisionPublic

Authored by charlesetc on Thu, Sep 23, 1:51 PM.

Details

Reviewers
durin42
Alphare
Group Reviewers
hg-reviewers
Summary

The performance of these commands seem roughly unchanged from the manual
testing I've done.

Diff Detail

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

Event Timeline

charlesetc created this revision.Thu, Sep 23, 1:51 PM
charlesetc retitled this revision from Make hg manifest only print things inside the narrowspec to narrow: Make hg manifest only print things inside the narrowspec.Thu, Sep 23, 1:52 PM
Alphare requested changes to this revision.Fri, Sep 24, 6:19 AM
Alphare added a subscriber: Alphare.

The change looks good aside from my one comment. The commit description shouldn't be capitalized (after the topic part), I'm not sure why test-check-commit.t stopped picking that up.

mercurial/commands.py
4776

I would add a check for alwaysmatcher to not run the filter if it's going to be useless anyway. I expect that large repositories could gain a little by doing this.

This revision now requires changes to proceed.Fri, Sep 24, 6:19 AM

Nevermind, it seems that I dreamed that rule and it was never enforced, just convention.

charlesetc retitled this revision from narrow: Make hg manifest only print things inside the narrowspec to Make hg manifest only print things inside the narrowspec.Fri, Sep 24, 2:02 PM
charlesetc updated this revision to Diff 30403.
charlesetc retitled this revision from Make hg manifest only print things inside the narrowspec to narrow: make hg manifest only print things inside the narrowspec.Fri, Sep 24, 2:02 PM

Thanks! How's that look?

Alphare accepted this revision.Mon, Sep 27, 3:58 AM

Thanks! (also it's not just large repos, it's also those that just don't have narrow)

This revision is now accepted and ready to land.Mon, Sep 27, 3:58 AM
Alphare requested changes to this revision.Mon, Sep 27, 4:29 AM

This breaks test output: https://foss.heptapod.net/octobus/mercurial-devel/-/jobs/248654

Maybe the manifest not being narrowed was on purpose for some reason? @durin42 could you shed some light?

This revision now requires changes to proceed.Mon, Sep 27, 4:29 AM

I think that's a buggy test? @martinvonz and @spectral might have an opinion.

I think that's a buggy test? @martinvonz and @spectral might have an opinion.

I think I did it that way because I felt that hg manifest is a debug command so it might be useful to have the information about excluded trees there. I never use it, however, so I don't mind if you change it.

PS. I wonder if @charlesetc actually wanted to use hg files. If not, what's the reason for using hg manifest?

I think that's a buggy test? @martinvonz and @spectral might have an opinion.

I think I did it that way because I felt that hg manifest is a debug command so it might be useful to have the information about excluded trees there.

This comment highlights when it's useful to have hg manifest produce the more complete information: when using tree manifests :), otherwise hg manifest and hg files seem roughly equivalent to me (except that hg manifest can be coerced to print permissions/flags/hashes, and files can't. I think I'd slightly prefer if we could have a flag, possibly one that's only available when the narrow extension is loaded, that controls this behavior? I don't have any strong preferences for what the default is.

I never use it, however, so I don't mind if you change it.
PS. I wonder if @charlesetc actually wanted to use hg files. If not, what's the reason for using hg manifest?

The problem we've encountered is that people use hg manifest not as a debug command but as an equivalent to hg files - a way to list the files in a revision / repo.

The problem we've encountered is that people use hg manifest not as a debug command but as an equivalent to hg files - a way to list the files in a revision / repo.

Seems to me that the manifest command is a "lower-level" command than files and really relates to the underlying datastructure, so it should probably not be affected by narrow (aside from potentially signaling in some way that some files are outside the narrow spec).