[PATCH 2 of 2] commands.push: use paths API

[Durham Goode]

On 8/24/15 7:30 PM, Gregory Szorc wrote:
> On Mon, Aug 24, 2015 at 9:51 PM, Durham Goode <durham at fb.com 
> <mailto:durham at fb.com>> wrote:
>
>     On 8/18/15 10:07 PM, Gregory Szorc wrote:
>
>         # HG changeset patch
>         # User Gregory Szorc <gregory.szorc at gmail.com
>         <mailto:gregory.szorc at gmail.com>>
>         # Date 1439012387 25200
>         #      Fri Aug 07 22:39:47 2015 -0700
>         # Node ID d75d57eea9af5d62a24356f129aa2d774ec32c03
>         # Parent  1699cd0bac167bd24f710c849d78d6a6c0089ac9
>         commands.push: use paths API
>
>         ui.path instances now collect most of the data used by
>         commands.push().
>         Move away from ui.expandpath() and call ui.paths.getpath() to
>         get a
>         path instance.
>
>         Some "pushing to" output was dropped as one test demonstrates.
>         I believe
>         the dropped message was redundant with the error message and
>         the change
>         to be acceptable.
>
>         diff --git a/mercurial/commands.py b/mercurial/commands.py
>         --- a/mercurial/commands.py
>         +++ b/mercurial/commands.py
>         @@ -5273,20 +5273,16 @@ def push(ui, repo, dest=None, **opts):
>                           # if we try to push a deleted bookmark,
>         translate it to null
>                           # this lets simultaneous -r, -b options
>         continue working
>                           opts.setdefault('rev', []).append("null")
>           -    dest = ui.expandpath(dest or 'default-push', dest or
>         'default')
>         -    dest, branches = hg.parseurl(dest, opts.get('branch'))
>         +    path = ui.paths.getpath(dest or 'default-push',
>         default='default')
>         +    if not path:
>         +        raise util.Abort(_('default repository not configured!'),
>         +                         hint=_('see the "path" section in
>         "hg help config"'))
>
>     This actually breaks hggit.  hggit wraps hg.peer to produce a
>     gitrepo instance when given a file path to a gitrepo.  With this
>     patch, the exception above is thrown because ui.paths.getpath()
>     returns None since it validates that the given path has a .hg
>     directory (which is not the case in hggit).
>
>     What's the path-y way of fixing this?  Wrapping the logic that
>     does the .hg validation is tough since it exists inside the path
>     class constructor and throws an exception (so the wrapping
>     function doesn't see the returned 'self' when it catches the
>     exceptions).  I could move the path validation logic to another
>     function on ui.path, then hggit could wrap that?
>
>
> First, it is unclear whether this validation should be performed in 
> path.__init__ or in paths.getpath(). I think I put it in path.__init__ 
> because it was slightly easier given the commit arc I took. I wouldn't 
> be surprised if this gets changed as I do more paths foo.
>
> I would move the path validation logic (the line startiing with "if 
> (not name and not u.scheme" into a module-level function on ui.py and 
> have hggit wrap it.
>
> Do I dare look at hggit's source to see how it was bypassing this 
> check before?
I don't think this logic existed before.  The old code just went 
straight to calling hg.peer(...) (which hggit handles), and only threw 
the 'default repository not configured' error when hg.peer(...) failed.

Actually, the fact that it now says "default repository not configured" 
when you do 'hg push somebadpath' seems like an incorrect error 
message.  I just tested it and this seems like a regression:

~/myrepo> hg-3.5 push doesntexist
pushing to doesntexist
abort: repository doesntexist not found!

~/myrepo> hg-latest push doesntexist
pushing to ssh://myserver.com//data/myrepo
searching for changes
...

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://selenic.com/pipermail/mercurial-devel/attachments/20150824/b69e6331/attachment.html>