This is an archive of the discontinued Mercurial Phabricator instance.

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

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

Download Raw Diff

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.Sep 23 2021, 1:51 PM

Herald added a reviewer: durin42. · View Herald TranscriptSep 23 2021, 1:51 PM

Herald added a reviewer: hg-reviewers. · View Herald Transcript

Herald added a subscriber: mercurial-patches. · View Herald Transcript

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.Sep 23 2021, 1:52 PM

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.Sep 24 2021, 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.Sep 24 2021, 2:02 PM

charlesetc updated this revision to Diff 30403.

Thanks! How's that look?

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.Sep 27 2021, 3:58 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.Sep 27 2021, 4:29 AM

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

In D11495#176284, @durin42 wrote:

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?

In D11495#176286, @martinvonz wrote:

In D11495#176284, @durin42 wrote:

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.

In D11495#176365, @charlesetc wrote:

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).

In D11495#176520, @Alphare wrote:

In D11495#176365, @charlesetc wrote:

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

It is mostly an older command. files is expected to replace hg manifest, but older user still use hg manifest

Revision Contents
Changeset List

			Path	Packages
M			mercurial/commands.py (8 lines)
M			tests/test-narrow.t (10 lines)

Commit	Parents	Author	Summary	Date
0ce14b41abb6	4e6f27230aee	Charles Chamberlain		Sep 20 2021, 2:21 PM

Diff 30403

mercurial/commands.py

This file is larger than 256 KB, so syntax highlighting is disabled by default.

	If option --all is specified, the list of all files from all revisions			If option --all is specified, the list of all files from all revisions
	is printed. This includes deleted and renamed files.			is printed. This includes deleted and renamed files.

	Returns 0 on success.			Returns 0 on success.
	"""			"""
	opts = pycompat.byteskwargs(opts)			opts = pycompat.byteskwargs(opts)
	fm = ui.formatter(b'manifest', opts)			fm = ui.formatter(b'manifest', opts)

				m = repo.narrowmatch()
	if opts.get(b'all'):			if opts.get(b'all'):
	if rev or node:			if rev or node:
	raise error.InputError(_(b"can't specify a revision with --all"))			raise error.InputError(_(b"can't specify a revision with --all"))

	res = set()			res = set()
	for rev in repo:			for rev in repo:
	ctx = repo[rev]			ctx = repo[rev]
	res \|= set(ctx.files())			files = ctx.files()
				if not m.always():
				AlphareUnsubmitted Not Done 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. Alphare: I would add a check for `alwaysmatcher` to not run the filter if it's going to be useless…
				files = filter(m, files)
				res \|= set(files)

	ui.pager(b'manifest')			ui.pager(b'manifest')
	for f in sorted(res):			for f in sorted(res):
	fm.startitem()			fm.startitem()
	fm.write(b"path", b'%s\n', f)			fm.write(b"path", b'%s\n', f)
	fm.end()			fm.end()
	return			return

	if rev and node:			if rev and node:
	raise error.InputError(_(b"please specify just one revision"))			raise error.InputError(_(b"please specify just one revision"))

	if not node:			if not node:
	node = rev			node = rev

	char = {b'l': b'@', b'x': b'*', b'': b'', b't': b'd'}			char = {b'l': b'@', b'x': b'*', b'': b'', b't': b'd'}
	mode = {b'l': b'644', b'x': b'755', b'': b'644', b't': b'755'}			mode = {b'l': b'644', b'x': b'755', b'': b'644', b't': b'755'}
	if node:			if node:
	repo = scmutil.unhidehashlikerevs(repo, [node], b'nowarn')			repo = scmutil.unhidehashlikerevs(repo, [node], b'nowarn')
	ctx = scmutil.revsingle(repo, node)			ctx = scmutil.revsingle(repo, node)
	mf = ctx.manifest()			mf = ctx.manifest()
	ui.pager(b'manifest')			ui.pager(b'manifest')
	for f in ctx:			for f in ctx.matches(m):
	fm.startitem()			fm.startitem()
	fm.context(ctx=ctx)			fm.context(ctx=ctx)
	fl = ctx[f].flags()			fl = ctx[f].flags()
	fm.condwrite(ui.debugflag, b'hash', b'%s ', hex(mf[f]))			fm.condwrite(ui.debugflag, b'hash', b'%s ', hex(mf[f]))
	fm.condwrite(ui.verbose, b'mode type', b'%s %1s ', mode[fl], char[fl])			fm.condwrite(ui.verbose, b'mode type', b'%s %1s ', mode[fl], char[fl])
	fm.write(b'path', b'%s\n', f)			fm.write(b'path', b'%s\n', f)
	fm.end()			fm.end()

tests/test-narrow.t

	$ hg --config 'hooks.pretxnopen = echo modified >> d0/f' tracked --removeinclude d0			$ hg --config 'hooks.pretxnopen = echo modified >> d0/f' tracked --removeinclude d0
	comparing with ssh://user@dummy/master			comparing with ssh://user@dummy/master
	searching for changes			searching for changes
	looking for local changes to affected paths			looking for local changes to affected paths
	deleting data/d0/f.i			deleting data/d0/f.i
	deleting meta/d0/00manifest.i (tree !)			deleting meta/d0/00manifest.i (tree !)
	deleting unwanted files from working copy			deleting unwanted files from working copy
	not deleting possibly dirty file d0/f			not deleting possibly dirty file d0/f


				Test manifest only prints files inside the narrowspec

				$ hg status
				$ hg tracked -q --addinclude "" --addexclude d2
				$ { hg manifest ; hg manifest -r . ; hg manifest --all ; } \| grep -e d2 -e d4
				d4/f
				d4/f
				d4/f

Diff	ID	Description	Created	Lint	Unit
Base		Base
Diff 1	30391		Sep 23 2021, 1:51 PM	★	★
Diff 2	30403		Sep 24 2021, 2:02 PM	★	★