( )⚙ D611 checknlink: use a random temp file name for checking

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.

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
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

quark created this revision.Sep 1 2017, 8:29 PM
yuja requested changes to this revision.Sep 3 2017, 10:20 AM
yuja added a subscriber: yuja.
yuja added inline comments.
mercurial/util.py
1461–1462

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.
yuja accepted this revision.Sep 6 2017, 10:41 AM

Queued, thanks.

mercurial/util.py
1465

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
1465

Good catch!

Revision Contents
Changeset List

PathPackages
Mmercurial/util.py (24 lines)

Diff 1633

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" f1, f2, fd = None, None, None
if os.path.lexists(f1):
return False
try:
posixfile(f1, 'w').close()
except IOError:
try: try:
os.unlink(f1) 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.
except OSError: suffix='1~', dir=os.path.dirname(testfile))
pass os.close(fd)
return False
f2 = testfile + ".hgtmp2"
fd = None fd = None
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!
try: f2 = '%s2~' % f1[:-2]
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.
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):
try: try:
if f is not None:
os.unlink(f) os.unlink(f)
except OSError: except OSError:
pass pass
def endswithsep(path): def endswithsep(path):
'''Check path ends with os.sep or os.altsep.''' '''Check path ends with os.sep or os.altsep.'''
return (path.endswith(pycompat.ossep) return (path.endswith(pycompat.ossep)
or pycompat.osaltsep and path.endswith(pycompat.osaltsep)) or pycompat.osaltsep and path.endswith(pycompat.osaltsep))