Dead code when reverting largefiles?

Greg Ward greg at gerg.ca
Tue Oct 25 08:17:42 CDT 2011


Hi --

I just sent in a patch to test "hg revert" with largefiles, and
there's a bit of override_revert() that I could not get the tests to
reach. I think it's unreachable code.

Details: largefiles wraps revert in order to keep track of what
happens to standins in .hglf/. Then, after the real revert is done, it
mirrors those actions (un-modify, un-remove, un-add) on the actual
largefiles.

Then it does this:

        for lfile in modified:
            if lfile in lfileslist:
                if os.path.exists(repo.wjoin(lfutil.standin(lfile))) and lfile\
                        in repo['.']:
                    lfutil.writestandin(repo, lfutil.standin(lfile),
                        repo['.'][lfile].data().strip(),
                        'x' in repo['.'][lfile].flags())

But I think the inner "if" condition is always false, because

  lfile in repo['.']

is always false. As near as I can tell, largefiles does nothing to
changectx objects: they only contain standins, never largefiles. So
the name of a largefile should never be in a changectx. I think that
code can be changed to

        for lfile in modified:
            if lfile in lfileslist:
                if os.path.exists(repo.wjoin(lfutil.standin(lfile))) and False:
                    lfutil.writestandin(repo, lfutil.standin(lfile),
                        repo['.'][lfile].data().strip(),
                        'x' in repo['.'][lfile].flags())

in which case the whole loop is pointless.

Am I missing something?

Related: I also couldn't reach the innermost line of the loop over added files:

       for lfile in added:
            standin = lfutil.standin(lfile)
            if standin not in ctx and (standin in matches or opts.get('all')):
                if lfile in lfdirstate:
                    lfdirstate.drop(lfile)
                util.unlinkpath(repo.wjoin(standin))

because I was never able to get a formerly-added-now-forgotten
largefile to be still present in lfdirstate. This smells like a corner
case that I just don't know how to reproduce rather than logically
unreachable code, though. Any hints on how to beef up the test to
reach that line?

Greg


More information about the Mercurial-devel mailing list