This is an archive of the discontinued Mercurial Phabricator instance.

Differential D611

checknlink: use a random temp file name for checking
ClosedPublic

Authored by quark on Sep 1 2017, 8:29 PM.

Download Raw Diff

Details

Reviewers

yuja

Group Reviewers

hg-reviewers

Commits

rHG1104718fb090: checknlink: use a random temp file name for checking

Summary

Previously, if .hg/store/00manifest.d.hgtmp1 exists, hg will copy the
entire 00manifest.d every time when appending new manifest revisions.
That could happen if Mercurial or the machine crashed when .hgtmp1 was
just created but not deleted yet.

This patch changes the fixed name to a random generated name. To be
consistent with D468, ~ suffix was used.

Diff Detail

Repository

rHG Mercurial

Lint

Lint Skipped

Unit

Unit Tests Skipped

Event Timeline

quark created this revision.Sep 1 2017, 8:29 PM

Herald added a reviewer: hg-reviewers. · View Herald TranscriptSep 1 2017, 8:29 PM

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

yuja requested changes to this revision.Sep 3 2017, 10:20 AM

yuja added a subscriber: yuja.

yuja added inline comments.

mercurial/util.py
1458	mkstemp() may raise OSError, which should be caught as before.

This revision now requires changes to proceed.Sep 3 2017, 10:20 AM

quark edited edge metadata.Sep 5 2017, 4:07 PM

quark retitled this revision from nlink: use a random temp file name for checking to checknlink: use a random temp file name for checking.

quark updated this revision to Diff 1616.

Queued, thanks.

mercurial/util.py
1466	Nit: here fd is a file descriptor, but later it's changed to a file object. fd.close() would raise AttributeError if os.close(fd) failed. Maybe we should rename the latter to fp or something. Can you send a follow up?

This revision is now accepted and ready to land.Sep 6 2017, 10:41 AM

Closed by commit rHG1104718fb090: checknlink: use a random temp file name for checking (authored by quark). · Explain WhySep 6 2017, 10:51 AM

This revision was automatically updated to reflect the committed changes.

quark added inline comments.Sep 6 2017, 3:16 PM

mercurial/util.py
1466	Good catch!

Revision Contents
Changeset List

			Path	Packages
M			mercurial/util.py (17 lines)

Diff	ID	Description	Created	Lint	Unit
Base		Base
Diff 1	1569		Sep 1 2017, 8:29 PM	★	★
Diff 2	1616		Sep 5 2017, 4:07 PM	★	★
Diff 3	1633	rHG1104718fb0907a4a841e6a24006c0c7fcb9caa9e	Sep 1 2017, 8:09 PM	★	★

Diff 1569

mercurial/util.py

	'''			'''
	return getattr(osutil, 'getfstype', lambda x: None)(dirpath)			return getattr(osutil, 'getfstype', lambda x: None)(dirpath)

	def checknlink(testfile):			def checknlink(testfile):
	'''check whether hardlink count reporting works properly'''			'''check whether hardlink count reporting works properly'''

	# testfile may be open, so we need a separate file for checking to			# testfile may be open, so we need a separate file for checking to
	# work around issue2543 (or testfile may get lost on Samba shares)			# work around issue2543 (or testfile may get lost on Samba shares)
	f1 = testfile + ".hgtmp1"			fd, f1 = tempfile.mkstemp(prefix='.%s-' % os.path.basename(testfile),
				yujaUnsubmitted Not Done mkstemp() may raise OSError, which should be caught as before. yuja: mkstemp() may raise OSError, which should be caught as before.
	if os.path.lexists(f1):			suffix='1~', dir=os.path.dirname(testfile))
	return False			os.close(fd)
	try:
	posixfile(f1, 'w').close()
	except IOError:
	try:
	os.unlink(f1)
	except OSError:
	pass
	return False

	f2 = testfile + ".hgtmp2"
	fd = None			fd = None
				f2 = '%s2~' % f1[:-2]
	try:			try:
	oslink(f1, f2)			oslink(f1, f2)
	# nlinks() may behave differently for files on Windows shares if			# nlinks() may behave differently for files on Windows shares if
	# the file is open.			# the file is open.
				yujaUnsubmitted Not Done Nit: here fd is a file descriptor, but later it's changed to a file object. fd.close() would raise AttributeError if os.close(fd) failed. Maybe we should rename the latter to fp or something. Can you send a follow up? yuja: Nit: here fd is a file descriptor, but later it's changed to a file object. fd.close() would…
				quarkAuthorUnsubmitted Not Done Good catch! quark: Good catch!
	fd = posixfile(f2)			fd = posixfile(f2)
	return nlinks(f2) > 1			return nlinks(f2) > 1
	except OSError:			except OSError:
	return False			return False
	finally:			finally:
	if fd is not None:			if fd is not None:
	fd.close()			fd.close()
	for f in (f1, f2):			for f in (f1, f2):