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

[Yuya Nishihara]

On Mon, 25 Jan 2016 20:29:09 -0500, Matt Harbison wrote:
> 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.

Thanks, pushed to the clowncopter.