[PATCH] largefiles: Commit directories that only contain largefiles (issue3548)

[Matt Harbison]

On Mon, 10 Dec 2012 15:01:31 +0100, Levi Bard wrote:

> # HG changeset patch
> # User Levi Bard <levi at unity3d.com>
> # Date 1355147922 -3600
> # Node ID fcf5fda9138ea94bce52a03c7af618904b50f16c
> # Parent  40374059d227850ec2f5fb4f21a1b619136e2a6a
> largefiles: Commit directories that only contain largefiles (issue3548)
              ^

One small issue- this command used to get rejected:

  $ hg ci -m "standin" .hglf
  abort: .hglf: no match under directory!
  [255]

But now it succeeds, sort of:

  $ hg ci -m "standin" .hglf
  Invoking status precommit hook
  M large3
  A large5
  A sub2/large6
  A sub2/large7
  A z/y/x/m/normal
  $ hg st
  M large3
  A large5
  A sub2/large6
  A sub2/large7
  A z/y/x/m/normal

(No error/exit failure, but no apparent change either, in which case the
exit code should be 1.  I'm not sure which of the before and after is the
proper behavior.)

A question below, but otherwise looks good.
 
> diff -r 40374059d227 -r fcf5fda9138e hgext/largefiles/reposetup.py
> --- a/hgext/largefiles/reposetup.py	Tue Nov 27 22:24:02 2012 +0100
> +++ b/hgext/largefiles/reposetup.py	Mon Dec 10 14:58:42 2012 +0100
...
> @@ -467,6 +463,60 @@
>              return super(lfilesrepo, self).push(remote, force, revs,
>                  newbranch)
>  
> +        def _subdirlfs(self, files, lfiles):
> +            '''
> +            Adjust matched file list
> +            If we pass a directory to commit whose only commitable files
> +            are largefiles, the core commit code aborts before finding
> +            the largefiles.
> +            So we do the following:
> +            For directories that only have largefiles as matches,
> +            we explicitly add the largefiles to the matchlist and remove
> +            the directory.
> +            In other cases, we leave the match list unmodified.
> +            '''
> +            actualfiles = []
> +            dirs = []
> +            regulars = []
> +
> +            for f in files:
> +                if lfutil.isstandin(f):
> +                    raise util.Abort(
> +                        _('file "%s" is a largefile standin') % f,
> +                        hint=('commit the largefile itself instead'))
> +                # Scan directories
> +                if os.path.isdir(self.wjoin(f)):
> +                    dirs.append(f)
> +                else:
> +                    regulars.append(f)
> +
> +            for f in dirs:
> +                matcheddir = False
> +                d = self.dirstate.normalize(f) + '/'
> +                # Check for matched normal files
> +                for mf in regulars:
> +                    if self.dirstate.normalize(mf).startswith(d):
> +                        actualfiles.append(f)
> +                        matcheddir = True
> +                        break
> +                if not matcheddir:
> +                    # If no normal match, manually append
> +                    # any matching largefiles
> +                    for lf in lfiles:
> +                        if self.dirstate.normalize(lf).startswith(d):
> +                            actualfiles.append(lf)
> +                            if not matcheddir:
> +                                actualfiles.append(lfutil.standin(f))
> +                                matcheddir = True

I can't figure out why the previous 3 lines are needed.  I commented them
out and the test blew up, so they are needed.  It appears that all of the
largefiles are added to actualfiles, and at the same time, only the first
corresponding standin (per directory) is.  Why are the standins needed at
all here, since the largefiles are provided?  And since at least one
seems to be needed, why aren't all of the standins needed?

> +                # Nothing in dir, so readd it
> +                # and let commit reject it
> +                if not matcheddir:
> +                    actualfiles.append(f)
> +
> +            # Always add normal files
> +            actualfiles += regulars
> +            return actualfiles
> +
>      repo.__class__ = lfilesrepo
>  
>      def checkrequireslfiles(ui, repo, **kwargs):