This is an archive of the discontinued Mercurial Phabricator instance.

Differential D4579

basestore: stop looking at the Nth item of an N-length revlog
ClosedPublic

Authored by durin42 on Sep 13 2018, 3:02 PM.

Download Raw Diff

Details

Reviewers

quark

Group Reviewers

Restricted Project

Commits

rFBHGXc83b2b6688c6: basestore: stop looking at the Nth item of an N-length revlog

Summary

That's off the end of the revlog, and previously I think this was
getting lucky because the off-the-end spot was occupied by the nullrev
or something like that.

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

durin42 created this revision.Sep 13 2018, 3:02 PM

Herald added a reviewer: Restricted Project. · View Herald TranscriptSep 13 2018, 3:02 PM

durin42 added a child revision: D4580: shallowrepo: use correct api to get manifestlog storage if available.Sep 13 2018, 3:02 PM

indygreg added a subscriber: indygreg.Sep 13 2018, 3:26 PM

indygreg added inline comments.

remotefilelog/basestore.py
162	This should be using `cl.revs()`, which does support reverse iteration if the start rev is larger than end rev.

quark accepted this revision.Sep 13 2018, 3:32 PM

This revision is now accepted and ready to land.Sep 13 2018, 3:32 PM

quark added inline comments.Sep 13 2018, 3:40 PM

remotefilelog/basestore.py
162	Good suggestion!

Closed by commit rFBHGXc83b2b6688c6: basestore: stop looking at the Nth item of an N-length revlog (authored by durin42). · Explain WhySep 13 2018, 4:54 PM

This revision was automatically updated to reflect the committed changes.

Probably a consequence of https://www.mercurial-scm.org/repo/hg/rev/a3dacabd476b or some other commit in that series.

Revision Contents
Changeset List

			Path	Packages
M			remotefilelog/basestore.py (2 lines)

Status	Author	Revision
Closed	durin42	D4583 shallowbundle: dump this revision prefetching logic
Closed	durin42	D4581 remotefilelog: update for core changegroup packing changes
Closed	durin42	D4580 shallowrepo: use correct api to get manifestlog storage if available
Closed	durin42	D4579 basestore: stop looking at the Nth item of an N-length revlog
Closed	durin42	D4578 remotefilelogserver: allow signature of generatefiles() to change
Closed	durin42	D4577 remotefilelogserver: adapt for there being only one cgpacker
Closed	durin42	D4561 tests: accept new hg output about local changesets being published
Closed	durin42	D4560 remotefilelogserver: use modern pattern for registering commands
Closed	durin42	D4559 cleanup: only look for the capabilities we care about in tests
Closed	durin42	D4558 tests: update remotefilelog http test to require two fetches

Diff 11028

remotefilelog/basestore.py

	for filename in self.repo['tip'].manifest():			for filename in self.repo['tip'].manifest():
	sha = hashlib.sha1(filename).digest()			sha = hashlib.sha1(filename).digest()
	if sha in missingfilename:			if sha in missingfilename:
	filenames[filename] = sha			filenames[filename] = sha
	missingfilename.discard(sha)			missingfilename.discard(sha)

	# Scan the changelog until we've found every file name			# Scan the changelog until we've found every file name
	cl = self.repo.unfiltered().changelog			cl = self.repo.unfiltered().changelog
	for rev in xrange(len(cl), -1, -1):			for rev in xrange(len(cl)-1, -1, -1):
				indygregUnsubmitted Not Done This should be using `cl.revs()`, which does support reverse iteration if the start rev is larger than end rev. indygreg: This should be using `cl.revs()`, which does support reverse iteration if the start rev is…
				quarkUnsubmitted Not Done Good suggestion! quark: Good suggestion!
	if not missingfilename:			if not missingfilename:
	break			break
	files = cl.readfiles(cl.node(rev))			files = cl.readfiles(cl.node(rev))
	for filename in files:			for filename in files:
	sha = hashlib.sha1(filename).digest()			sha = hashlib.sha1(filename).digest()
	if sha in missingfilename:			if sha in missingfilename:
	filenames[filename] = sha			filenames[filename] = sha
	missingfilename.discard(sha)			missingfilename.discard(sha)

Diff	ID	Description	Created	Lint	Unit
Base		Base
Diff 1	11017		Sep 13 2018, 3:02 PM	★	★
Diff 2	11028	rFBHGXc83b2b6688c65ee9919fab7d69981c78117b8c23	Sep 13 2018, 11:40 AM	★	★