[PATCH] largefiles: makes verify to work on local content by default (issue4242) (BC)

[Sean Farley]

liscju <piotr.listkiewicz at gmail.com> writes:

> # HG changeset patch
> # User liscju <piotr.listkiewicz at gmail.com>
> # Date 1456917749 -3600
> #      Wed Mar 02 12:22:29 2016 +0100
> # Node ID 184b0386fad4aff1ec64f0076c74c13e2cf5d036
> # Parent  c7f89ad87baef87f00c507545dfd4cc824bc3131
> largefiles: makes verify to work on local content by default (issue4242) (BC)

Wow, awesome first patch for largefiles!

> Before this patch largefiles verify options always tried to do
> verification on remote store by default. This patch makes verification
> always done locally unless user specify --remote option.
>
> Implementation of checking locally was implemented before this
> patch in basestore._openstore as special case when ui.expandpath
> had problem with expanding lfpullsource or default. This patch adds
> forcelocal parameter to basestore._openstore function making it
> (when set) returns localstore class with remote assigned to
> local repository.

This seems like a good step in the direction in making the remote
largefiles storage separable from the remote server code storage. Mads?

Code review follows:

> diff -r c7f89ad87bae -r 184b0386fad4 hgext/largefiles/basestore.py
> --- a/hgext/largefiles/basestore.py	Mon Feb 29 17:52:17 2016 -0600
> +++ b/hgext/largefiles/basestore.py	Wed Mar 02 12:22:29 2016 +0100
> @@ -174,27 +174,31 @@ import localstore, wirestore
>  # During clone this function is passed the src's ui object
>  # but it needs the dest's ui object so it can read out of
>  # the config file. Use repo.ui instead.
> -def _openstore(repo, remote=None, put=False):
> +def _openstore(repo, remote=None, put=False, forcelocal=False):
>      ui = repo.ui
>  
>      if not remote:
> -        lfpullsource = getattr(repo, 'lfpullsource', None)
> -        if lfpullsource:
> -            path = ui.expandpath(lfpullsource)
> -        elif put:
> -            path = ui.expandpath('default-push', 'default')
> -        else:
> -            path = ui.expandpath('default')
> -
> -        # ui.expandpath() leaves 'default-push' and 'default' alone if
> -        # they cannot be expanded: fallback to the empty string,
> -        # meaning the current directory.
> -        if path == 'default-push' or path == 'default':
> +        if forcelocal:
>              path = ''
>              remote = repo
>          else:
> -            path, _branches = hg.parseurl(path)
> -            remote = hg.peer(repo, {}, path)
> +            lfpullsource = getattr(repo, 'lfpullsource', None)
> +            if lfpullsource:
> +                path = ui.expandpath(lfpullsource)
> +            elif put:
> +                path = ui.expandpath('default-push', 'default')
> +            else:
> +                path = ui.expandpath('default')
> +
> +            # ui.expandpath() leaves 'default-push' and 'default' alone if
> +            # they cannot be expanded: fallback to the empty string,
> +            # meaning the current directory.
> +            if path == 'default-push' or path == 'default':
> +                path = ''
> +                remote = repo
> +            else:
> +                path, _branches = hg.parseurl(path)
> +                remote = hg.peer(repo, {}, path)
>  
>      # The path could be a scheme so use Mercurial's normal functionality
>      # to resolve the scheme to a repository and use its path
> diff -r c7f89ad87bae -r 184b0386fad4 hgext/largefiles/lfcommands.py
> --- a/hgext/largefiles/lfcommands.py	Mon Feb 29 17:52:17 2016 -0600
> +++ b/hgext/largefiles/lfcommands.py	Wed Mar 02 12:22:29 2016 +0100
> @@ -357,7 +357,7 @@ def uploadlfiles(ui, rsrc, rdst, files):
>          at += 1
>      ui.progress(_('uploading largefiles'), None)
>  
> -def verifylfiles(ui, repo, all=False, contents=False):
> +def verifylfiles(ui, repo, all=False, contents=False, forcelocal=False):
>      '''Verify that every largefile revision in the current changeset
>      exists in the central store.  With --contents, also verify that
>      the contents of each local largefile file revision are correct (SHA-1 hash
> @@ -368,7 +368,7 @@ def verifylfiles(ui, repo, all=False, co
>      else:
>          revs = ['.']
>  
> -    store = basestore._openstore(repo)
> +    store = basestore._openstore(repo, forcelocal=forcelocal)
>      return store.verify(revs, contents=contents)
>  
>  def cachelfiles(ui, repo, node, filelist=None):
> diff -r c7f89ad87bae -r 184b0386fad4 hgext/largefiles/overrides.py
> --- a/hgext/largefiles/overrides.py	Mon Feb 29 17:52:17 2016 -0600
> +++ b/hgext/largefiles/overrides.py	Wed Mar 02 12:22:29 2016 +0100
> @@ -395,10 +395,12 @@ def overrideverify(orig, ui, repo, *pats
>      large = opts.pop('large', False)
>      all = opts.pop('lfa', False)
>      contents = opts.pop('lfc', False)
> +    forcelocal = not opts.pop('remote', False)
>  
>      result = orig(ui, repo, *pats, **opts)
>      if large or all or contents:
> -        result = result or lfcommands.verifylfiles(ui, repo, all, contents)
> +        result = result or \
> +                 lfcommands.verifylfiles(ui, repo, all, contents, forcelocal)

Minor nit: we usually split this different:

result = result or lfcommands.verifylfiles(ui, repo, all,
                                           contents, forcelocal)

(avoiding the \ in this case)