[PATCH STABLE] largefiles: fix an explicit largefile commit after a remove (issue4969)

Matt Harbison mharbison72 at gmail.com
Mon Jan 25 19:29:09 CST 2016


On Mon, 25 Jan 2016 10:07:21 -0500, Yuya Nishihara <yuya at tcha.org> wrote:

> On Sat, 23 Jan 2016 20:58:00 -0500, Matt Harbison wrote:
>> # HG changeset patch
>> # User Matt Harbison <matt_harbison at yahoo.com>
>> # Date 1453600277 18000
>> #      Sat Jan 23 20:51:17 2016 -0500
>> # Branch stable
>> # Node ID d07901a07264cc495565bce44eccb7d80afb7a25
>> # Parent  4c6053a6b17d682b34fb88bbeb5e94ed9085d900
>> largefiles: fix an explicit largefile commit after a remove (issue4969)
>>
>> The change in b68797f244e4 to handle a normal -> largefile switch was  
>> too
>> aggressive in preserving the original matcher names.  If a largefile is
>> explicitly provided by the user, but only the standin exists in  
>> dirstate, then
>> only the standin can be committed.
>>
>> There's still maybe an issue when the largefile is deleted outside of  
>> Mercurial:
>>
>>   $ rm large
>>   $ hg ci -m "oops" large
>>   large: The system cannot find the file specified
>>   nothing changed
>>   [1]
>>
>> That may be OK, since the standin file could have been changed since  
>> the last
>> commit, and this seems safer since the largefile isn't present to  
>> regenerate it.
>>
>> diff --git a/hgext/largefiles/lfutil.py b/hgext/largefiles/lfutil.py
>> --- a/hgext/largefiles/lfutil.py
>> +++ b/hgext/largefiles/lfutil.py
>> @@ -575,11 +575,15 @@
>>          fstandin = standin(f)
>>
>>          # For largefiles, only one of the normal and standin should be
>> -        # committed (except if one of them is a remove).
>> +        # committed (except if one of them is a remove).  In the case  
>> of a
>> +        # standin removal, drop the normal file if it is unknown to  
>> dirstate.
>>          # Thus, skip plain largefile names but keep the standin.
>> -        if (f in lfiles or fstandin in standins) and \
>> -            repo.dirstate[f] != 'r' and repo.dirstate[fstandin] != 'r':
>> -            continue
>> +        if f in lfiles or fstandin in standins:
>> +            if repo.dirstate[fstandin] != 'r':
>> +                if repo.dirstate[f] != 'r':
>> +                    continue
>> +            elif repo.dirstate[f] == '?':
>> +                continue
>
> Looks good to me, but I'm not confident. I want to clarify my  
> understanding:
>
>   (dirstate[f], dirstate[fstandin])
>     = ('?', 'r') => removal of 'f', exclude 'f' but not 'fstandin'

Yes, this is the case of removing only a largefile.  The tracked standin  
is committed, and the largefile is untracked in dirstate.

>       ('r', _) | (_, 'r') => might be add/remove pair, include both

I believe so.  This was the previous logic to handle changing between  
large and normal for 'f', and then it makes sense to commit both.  All I  
did was unroll the conditional to tack on the elif branch for the case  
above.

> Right?


More information about the Mercurial-devel mailing list