[PATCH 1 of 2 RFC V2] largefiles: abort push on client when file fit largefiles (issue3245)

[Yuya Nishihara]

On Fri, 26 Aug 2016 08:23:12 +0200, liscju wrote:
> # HG changeset patch
> # User liscju <piotr.listkiewicz at gmail.com>
> # Date 1472122462 -7200
> #      Thu Aug 25 12:54:22 2016 +0200
> # Node ID 7fa71874c771a76bcc0e04d77e435242b9b3ad12
> # Parent  6a98f9408a504be455d4382801610daceac429e6
> largefiles: abort push on client when file fit largefiles  (issue3245)
> 
> So far setting minsize and patterns for largefiles worked only
> locally changing which files were added/committed as largefiles.
> It did not prevent pushing ordinary files that exceeded minsize
> or matched pattern for largefiles on server.
> 
> To solve this there are two possible solutions:
> 1) Check ordinary files before pushing, if they match pattern
> for largefiles on server - abort push
> 2) Check ordinary files when there are unbundled on server,
> abort when there are files that match pattern for largefiles
> 
> This commit is an implementation of #1 idea to discuss and needs
> some changes before being merged. The #2 idea is covered in
> consecutive commit.
> 
> This solution at first downloads minsize and patterns from server,
> check if any of files exceeds minsize or match pattern and if it
> does aborts push.
> 
> This solution works only when server/client has version of hg that
> support this checking. Idea #2 works for any client as far as
> server has support for this, but the drawback of this idea is that
> checks are done after sending files to server.

I don't have any better idea, but I think it's too late to exchange minsize
and patterns on push. Ideally these limits should be enforced when files are
added or committed. In order to do that, these values have to be pulled
beforehand and cached somewhere.

> +        if pats:
> +            for rev in revs:
> +                ctx = self.repo[rev]
> +                # cwd parameter is equal to repo.root to prevent
> +                # 'abort: pattern not under root' when hg used
> +                # with -R option
> +                lfmatcher = matchmod.match(self.repo.root,
> +                                           self.repo.root, pats, ctx=ctx)

That is match(repo.root, '', pats, ctx=ctx).

> +                for file in filesmatching:
> +                    raise error.Abort(_("Files %s in revision %s should be "
> +                                        "marked as largefiles")
> +                                      % (file, rev))

Nit: an error message generally starts with lowercase letter.

> +def listlfilespattern(repo):
> +    lfsize = repo.ui.config(lfutil.longname, 'minsize', default=None)

The default appears to be 10MB.

> +    if lfsize:
> +        try:
> +            lfsize = util.sizetoint(lfsize + "MB")
> +        except error.ParseError:
> +            lfsize = None
> +
> +    lfpats = repo.ui.configlist(
> +        lfutil.longname, 'patterns', default=[])
> +
> +    keys = {}
> +
> +    if lfsize:
> +        keys['lfminsize'] = str(lfsize)
> +    else:
> +        keys['lfminsize'] = ''
> +
> +    keys['lfpats'] = ','.join(lfpats)

What if a pattern contain ',' ?

> @@ -350,6 +351,14 @@ def reposetup(ui, repo):
>              actualfiles += regulars
>              return actualfiles
>  
> +        def checkpush(self, pushop):
> +            super(lfilesrepo, self).checkpush(pushop)
> +            store = storefactory.openstore(pushop.repo, pushop.remote)
> +            revs = pushop.revs
> +            if revs is None:
> +                revs = pushop.repo.revs('all()')

Falling back to all() would be too expensive. Perhaps we can use
prepushoutgoinghooks.