[PATCH 2 of 2] util: move checknlink away from the dependency of a hard-coded filename

Tony Tung tonytung at instagram.com
Tue Aug 23 13:30:46 EDT 2016


> On Aug 23, 2016, at 8:28 AM, Yuya Nishihara <yuya at tcha.org> wrote:
> 
> On Fri, 19 Aug 2016 13:58:40 -0700, ttung at fb.com wrote:
>> # HG changeset patch
>> # User Tony Tung <tonytung at merly.org>
>> # Date 1471639931 25200
>> #      Fri Aug 19 13:52:11 2016 -0700
>> # Node ID 4aaeb57d5f4497ef35b48e23b80e43b914afd819
>> # Parent  2efc5a05d80a6d4253767f2ce0c2fb062ba83cb6
>> util: move checknlink away from the dependency of a hard-coded filename
> 
>> --- a/mercurial/util.py
>> +++ b/mercurial/util.py
>> @@ -1305,34 +1305,33 @@
>> def checknlink(testfile):
>>     '''check whether hardlink count reporting works properly'''
>> 
>> +    dirpath, filename = os.path.split(testfile)
>> +
>>     # testfile may be open, so we need a separate file for checking to
>>     # work around issue2543 (or testfile may get lost on Samba shares)
>> -    f1 = testfile + ".hgtmp1"
>> -    if os.path.lexists(f1):
>> -        return False
>> +    f1handle, f1path = tempfile.mkstemp(prefix=filename, dir=dirpath)
>> +    os.close(f1handle)
>> +
>> +    f2handle, f2path = tempfile.mkstemp(prefix=filename, dir=dirpath)
>> +    os.close(f2handle)
>> +    f2handle = None
> 
> As you've mentioned in the previous patch, we should delete f1/f2path on error.
> Other than that, this series looks good to me.

There’s the unlinking of f1/f2path in the finally clause, but I guess that doesn’t cover earlier errors.  Let me fix that.

> 
>> +    # there's a small race condition that another file can jam itself in between
>> +    # the time we remove f2 and the time we create the hard link.  in the
>> +    # unlikely scenario that happens, we'll treat it as nlink being unreliable.
>>     try:
> [...]
>> +        os.unlink(f2path)
>> +        oslink(f1path, f2path)
> 
> I would use f2path = f1path + '.2' assuming f1path has enough random bits, but
> your version, mkstemp+unlink+link, should be more robust for a race condition.

I think I can actually make this more robust. :)

Thanks,
Tony


More information about the Mercurial-devel mailing list