[PATCH 2 of 3] largefiles: enable islfilesrepo() prior to a commit (issue3541)

[Matt Harbison]

Na'Tosha Bard wrote:
> 2012/8/9 Matt Harbison <matt_harbison at yahoo.com
> <mailto:matt_harbison at yahoo.com>>
>
>     Na'Tosha Bard wrote:
>
>         2012/8/7 Matt Harbison <matt_harbison at yahoo.com
>         <mailto:matt_harbison at yahoo.com>
>         <mailto:matt_harbison at yahoo.__com <mailto:matt_harbison at yahoo.com>>>
>
>
>              # HG changeset patch
>              # User Matt Harbison <matt_harbison at yahoo.com
>         <mailto:matt_harbison at yahoo.com>
>         <mailto:matt_harbison at yahoo.__com <mailto:matt_harbison at yahoo.com>>>
>
>              # Date 1343696201 14400
>              # Node ID 40fc3ecfffc7727537a11f5a26064e__97f1dd0e7b
>              # Parent  542cfb521b1297a887410173f5971a__fc537f2fb3
>              largefiles: enable islfilesrepo() prior to a commit (issue3541)
>
>              -def openlfdirstate(ui, repo):
>              +def openlfdirstate(ui, repo, create=True):
>         '''
>                    Return a dirstate object that tracks largefiles: i.e.
>         its root is
>                    the repo root, but it is saved in
>         .hg/largefiles/dirstate.
>              @@ -154,7 +154,7 @@
>                    # If the largefiles dirstate does not exist, populate
>         and create
>                    # it. This ensures that we create it on the first
>         meaningful
>                    # largefiles operation in a new clone.
>              -    if not os.path.exists(os.path.join(__admin, 'dirstate')):
>              +    if create and not os.path.exists(os.path.join(__admin,
>         'dirstate')):
>                        util.makedirs(admin)
>                        matcher = getstandinmatcher(repo)
>                        for standin in dirstatewalk(repo.dirstate, matcher):
>              @@ -435,8 +435,11 @@
>                    return util.pconvert(os.path.__normpath(path))
>
>                def islfilesrepo(repo):
>              -    return ('largefiles' in repo.requirements and
>              -            util.any(shortname + '/' in f[0] for f in
>              repo.store.datafiles()))
>              +    if ('largefiles' in repo.requirements and
>              +            util.any(shortname + '/' in f[0] for f in
>              repo.store.datafiles())):
>              +        return True
>              +
>              +    return util.any(openlfdirstate(repo.__ui, repo, False))
>
>
>         Does this have any effect on performance?
>
>     I didn't run any benchmarks (I'm not sure how to collect numbers or
>     structure the tests to avoid caching and the usual benchmarking
>     caveats), but I didn't notice any issues with admittedly trivial use.
>
>     My thinking was that by passing false here, the dirstate walk and
>     initial population of lfdirstate is avoided, even if the file
>     doesn't exist.  All openlfdirstate() does in the case is:
>
>     1) join a path
>     2) create an opener instance
>     3) create a largefilesdirstate instance
>
>     all of which seem trivial to me.  Any particular cases you're
>     worried about?
>
>
> Nevermind, when I pull down the patch and apply it, and am able to see
> the diff in context, it seems fine.  I guess it's still possible to be
> in the situation of adding a file as a large file, then changing your
> mind and not comitting it -- but the file is now a largefiles repo?

In that case, you either have to go with it and commit, or forget the 
large file to get out of it.  Check out the test on line 1500 in this 
patch- I think that is what you mean.

> Probably we need to handle that somehow; likely it's related to the
> other issue we're discussing about adding --large to init.

I don't think its related, because this patch keeps the previous 
definition of islfilesrepo() intact, but then allows for the additional 
case where something is in lfdirstate, but not yet committed.  I'm 
assuming that is cleared out after a strip.  The other patch stopped 
looking at the store in order to handle the case of no files yet added.

But it doesn't get to this check after a strip.  I just put in prints, 
and repo.store.datafiles() still has the standins of the previously 
added largefiles, so that function still returns true.  Here is a short 
test without any of the patches applied in this series:

   $ cat >> $HGRCPATH <<EOF
   > [extensions]
   > largefiles=
   > mq=
   > [largefiles]
   > minsize=2
   > patterns=glob:**.dat
   > usercache=${USERCACHE}
   > [hooks]
   > precommit=sh -c "echo \"Invoking status precommit hook\"; hg status"
   > EOF

   $ hg init addrm2
   $ cd addrm2
   $ touch large.dat
   $ hg add --large large.dat
   $ hg ci -mfoo
   Invoking status precommit hook
   A large.dat
   $ hg strip -r 0 --no-backup
   0 files updated, 0 files merged, 1 files removed, 0 files unresolved
   getting changed largefiles
   0 largefiles updated, 1 removed
   $ touch large.dat
   $ hg addremove -v
   adding large.dat as a largefile
   $ cd ..

So this seems like a separate bug.  I went back to 2.1 and there's no 
change from above; the test under 2.0.2 never completes on Linux.  I'm 
not sure what the unintended consequences of the standins remaining 
behind is on various other parts of the code.  (And there's probably no 
way to fix this in existing repos).

--Matt