This is an archive of the discontinued Mercurial Phabricator instance.

narrow: only send includepats and excludepats if they are not empty
ClosedPublic

Authored by pulkit on Oct 17 2018, 10:53 AM.

Download Raw Diff

Details

Reviewers

durin42
martinvonz

Group Reviewers

hg-reviewers

Commits

rHGa2c4502e409b: narrow: only send includepats and excludepats if they are not empty

Summary

If we send an empty includepats or excludepats argument to getbundle, it's
translated to [''] on the server which causes problems because even though
it's empty, bool of that value if True and we end up creating differencematcher
with narrowspec.match() which results in unexpected behavior.

Diff Detail

Repository

rHG Mercurial

Lint

Automatic diff as part of commit; lint not applicable.

Unit

Automatic diff as part of commit; unit tests not applicable.

Event Timeline

pulkit created this revision.Oct 17 2018, 10:53 AM

Herald added a reviewer: durin42. · View Herald TranscriptOct 17 2018, 10:53 AM

Herald added a reviewer: martinvonz. · View Herald Transcript

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

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

pulkit added a child revision: D5139: store: introduce _matchtrackedpath() and use it to filter store files.Oct 17 2018, 10:53 AM

pulkit mentioned this in D5137: streamclone: new server config and some API changes for narrow stream clones.Oct 18 2018, 5:21 AM

pulkit updated this revision to Diff 12232.Oct 18 2018, 6:38 AM

pulkit edited parent revisions, added: D5119: exchange: pass includepats and excludepats as arguments to getbundle(); removed: D4850: store: pass matcher to store.datafiles().Oct 18 2018, 6:40 AM

Closed by commit rHGa2c4502e409b: narrow: only send includepats and excludepats if they are not empty (authored by pulkit). · Explain WhyNov 5 2018, 11:15 AM

This revision was automatically updated to reflect the committed changes.

I noticed I had an unsubmitted comment on this patch. Can be done in a follow-up (if at all).

hgext/narrow/narrowcommands.py
142–144	should these also be changed?

pulkit added inline comments.Nov 12 2018, 1:52 PM

hgext/narrow/narrowcommands.py
142–144	From these, do you mean the oldincludepats and oldexcludepats?

Revision Contents
Changeset List

			Path	Packages
M			hgext/narrow/narrowcommands.py (6 lines)

Diff	ID	Description	Created	Lint	Unit
Base		Base
Diff 1	12211		Oct 17 2018, 10:53 AM	★	★
Diff 2	12232		Oct 18 2018, 6:38 AM	★	★
Diff 3	12398	rHGa2c4502e409b72b22053fc31772368416c876b66	Oct 17 2018, 10:24 AM	★	★

Status	Author	Revision
Abandoned	pulkit	D5147 config: make 'experimental' the section instead of 'experimental.server'
Closed	pulkit	D5139 store: introduce _matchtrackedpath() and use it to filter store files
Closed	pulkit	D5138 narrow: only send includepats and excludepats if they are not empty
Closed	pulkit	D5119 exchange: pass includepats and excludepats as arguments to getbundle()

Diff 12398

hgext/narrow/narrowcommands.py

	if repository.NARROW_REQUIREMENT not in repo.requirements:			if repository.NARROW_REQUIREMENT not in repo.requirements:
	return orig(pullop, kwargs)			return orig(pullop, kwargs)

	if wireprototypes.NARROWCAP not in pullop.remote.capabilities():			if wireprototypes.NARROWCAP not in pullop.remote.capabilities():
	raise error.Abort(_("server does not support narrow clones"))			raise error.Abort(_("server does not support narrow clones"))
	orig(pullop, kwargs)			orig(pullop, kwargs)
	kwargs['narrow'] = True			kwargs['narrow'] = True
	include, exclude = repo.narrowpats			include, exclude = repo.narrowpats
	kwargs['oldincludepats'] = include			kwargs['oldincludepats'] = include
	kwargs['oldexcludepats'] = exclude			kwargs['oldexcludepats'] = exclude
				if include:
				martinvonzUnsubmitted Not Done should these also be changed? martinvonz: should these also be changed?
				pulkitAuthorUnsubmitted Not Done From these, do you mean the oldincludepats and oldexcludepats? pulkit: From these, do you mean the oldincludepats and oldexcludepats?
	kwargs['includepats'] = include			kwargs['includepats'] = include
				if exclude:
	kwargs['excludepats'] = exclude			kwargs['excludepats'] = exclude
	# calculate known nodes only in ellipses cases because in non-ellipses cases			# calculate known nodes only in ellipses cases because in non-ellipses cases
	# we have all the nodes			# we have all the nodes
	if wireprototypes.ELLIPSESCAP in pullop.remote.capabilities():			if wireprototypes.ELLIPSESCAP in pullop.remote.capabilities():
	kwargs['known'] = [node.hex(ctx.node()) for ctx in			kwargs['known'] = [node.hex(ctx.node()) for ctx in
	repo.set('::%ln', pullop.common)			repo.set('::%ln', pullop.common)
	if ctx.node() != node.nullid]			if ctx.node() != node.nullid]
	if not kwargs['known']:			if not kwargs['known']:
	# Mercurial serializes an empty list as '' and deserializes it as			# Mercurial serializes an empty list as '' and deserializes it as