[PATCH] ui: change default path fallback mechanism (issue4796)

[Augie Fackler]

On Sun, Sep 06, 2015 at 11:35:32AM -0700, Gregory Szorc wrote:
> # HG changeset patch
> # User Gregory Szorc <gregory.szorc at gmail.com>
> # Date 1441564128 25200
> #      Sun Sep 06 11:28:48 2015 -0700
> # Node ID d68f9e4ef7c19251fbfe807ea66e7c84b5c378e2
> # Parent  be1a87fe8d849b8d904ce46c0ab99d1d280b1946
> ui: change default path fallback mechanism (issue4796)

queued, many thanks

>
> The previous paths API code always fell back to the default path. This
> was wrong because if a requested path doesn't exist, that should error.
> Only if no path was requested should we fall back to the default.
>
> As part of implementing the test case for issue 4796, it was discovered
> that the "repository does not exist" error message raised by
> localrepository.__init__ wasn't being seen because the paths API
> validates paths before localrepository.__init__ was being called.
> The exception and error message from localrepository.__init__ has
> been introduced to getpath(). This necessitated rewriting
> expandpath() both to catch the exception and to have proper
> default fallback.
>
> This code is more complicated than I'd like. But making all tests pass
> was a big chore. As more code moves to getpath(), there will likely be
> opportunities to improve things a bit.
>
> diff --git a/mercurial/ui.py b/mercurial/ui.py
> --- a/mercurial/ui.py
> +++ b/mercurial/ui.py
> @@ -549,11 +549,23 @@ class ui(object):
>          return user
>
>      def expandpath(self, loc, default=None):
>          """Return repository location relative to cwd or from [paths]"""
> -        p = self.paths.getpath(loc, default=default)
> -        if p:
> -            return p.rawloc
> +        try:
> +            p = self.paths.getpath(loc)
> +            if p:
> +                return p.rawloc
> +        except error.RepoError:
> +            pass
> +
> +        if default:
> +            try:
> +                p = self.paths.getpath(default)
> +                if p:
> +                    return p.rawloc
> +            except error.RepoError:
> +                pass
> +
>          return loc
>
>      @util.propertycache
>      def paths(self):
> @@ -1013,27 +1025,37 @@ class paths(dict):
>
>          ``name`` can be a named path or locations. Locations are filesystem
>          paths or URIs.
>
> -        Returns the first of ``name`` or ``default`` that is present, or None
> -        if neither is present.
> +        Returns None if ``name`` is not a registered path, a URI, or a local
> +        path to a repo.
>          """
> +        # Only fall back to default if no path was requested.
> +        if name is None:
> +            if default:
> +                try:
> +                    return self[default]
> +                except KeyError:
> +                    return None
> +            else:
> +                return None
> +
> +        # Most likely empty string.
> +        # This may need to raise in the future.
> +        if not name:
> +            return None
> +
>          try:
>              return self[name]
>          except KeyError:
>              # Try to resolve as a local path or URI.
>              try:
>                  return path(None, rawloc=name)
>              except ValueError:
> -                pass
> +                raise error.RepoError(_('repository %s does not exist') %
> +                                        name)
>
> -            if default is not None:
> -                try:
> -                    return self[default]
> -                except KeyError:
> -                    pass
> -
> -        return None
> +        assert False
>
>  class path(object):
>      """Represents an individual path and its configuration."""
>
> diff --git a/tests/test-default-push.t b/tests/test-default-push.t
> --- a/tests/test-default-push.t
> +++ b/tests/test-default-push.t
> @@ -44,4 +44,10 @@ Push should push to 'default-push' when
>    adding changesets
>    adding manifests
>    adding file changes
>    added 1 changesets with 1 changes to 1 files
> +
> +Pushing to a path that isn't defined should not fall back to default
> +
> +  $ hg --cwd b push doesnotexist
> +  abort: repository doesnotexist does not exist!
> +  [255]
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel at selenic.com
> https://selenic.com/mailman/listinfo/mercurial-devel