Patchwork [v3] outgoing: respect ":pushurl" paths (issue5365)

login
register
mail settings
Submitter Hollis Blanchard
Date Dec. 15, 2017, 12:23 a.m.
Message ID <a71c6d5ea84933324e0c.1513297404@aurora.wv.mentorg.com>
Download mbox | patch
Permalink /patch/26290/
State Superseded
Headers show

Comments

Hollis Blanchard - Dec. 15, 2017, 12:23 a.m.

Yuya Nishihara - Dec. 15, 2017, 1:15 p.m.
On Thu, 14 Dec 2017 16:23:24 -0800, Hollis Blanchard wrote:
> # HG changeset patch
> # User Hollis Blanchard <hollis_blanchard@mentor.com>
> # Date 1513292635 28800
> #      Thu Dec 14 15:03:55 2017 -0800
> # Node ID a71c6d5ea84933324e0c516ea2e4967e06616c31
> # Parent  4937db58b663faa6893c51a41cec28114a165dd0
> outgoing: respect ":pushurl" paths (issue5365)

> diff --git a/mercurial/hg.py b/mercurial/hg.py
> --- a/mercurial/hg.py
> +++ b/mercurial/hg.py
> @@ -912,8 +912,20 @@ def incoming(ui, repo, source, opts):
>      return _incoming(display, subreporecurse, ui, repo, source, opts)
>  
>  def _outgoing(ui, repo, dest, opts):
> -    dest = ui.expandpath(dest or 'default-push', dest or 'default')
> -    dest, branches = parseurl(dest, opts.get('branch'))
> +    path = None
> +    try:
> +        path = ui.paths.getpath(dest, default=('default-push', 'default'))

Here we need to test "not path" because it may be None.

> +        dest = path.pushloc or path.loc
> +        branches = path.branch, opts.get('branch') or []
> +    except error.RepoError:
> +        # Fake up empty state to avoid changing the user-facing error message.
> +        path = dest
> +        branches = (None, [])

I think raising RepoError should be fine. The error message isn't wrong.
Hollis Blanchard - Dec. 15, 2017, 4:20 p.m.
On 12/15/2017 05:15 AM, Yuya Nishihara wrote:
> On Thu, 14 Dec 2017 16:23:24 -0800, Hollis Blanchard wrote:
>> # HG changeset patch
>> # User Hollis Blanchard <hollis_blanchard@mentor.com>
>> # Date 1513292635 28800
>> #      Thu Dec 14 15:03:55 2017 -0800
>> # Node ID a71c6d5ea84933324e0c516ea2e4967e06616c31
>> # Parent  4937db58b663faa6893c51a41cec28114a165dd0
>> outgoing: respect ":pushurl" paths (issue5365)
>> diff --git a/mercurial/hg.py b/mercurial/hg.py
>> --- a/mercurial/hg.py
>> +++ b/mercurial/hg.py
>> @@ -912,8 +912,20 @@ def incoming(ui, repo, source, opts):
>>       return _incoming(display, subreporecurse, ui, repo, source, opts)
>>   
>>   def _outgoing(ui, repo, dest, opts):
>> -    dest = ui.expandpath(dest or 'default-push', dest or 'default')
>> -    dest, branches = parseurl(dest, opts.get('branch'))
>> +    path = None
>> +    try:
>> +        path = ui.paths.getpath(dest, default=('default-push', 'default'))
> Here we need to test "not path" because it may be None.
Oops, thanks.
>> +        dest = path.pushloc or path.loc
>> +        branches = path.branch, opts.get('branch') or []
>> +    except error.RepoError:
>> +        # Fake up empty state to avoid changing the user-facing error message.
>> +        path = dest
>> +        branches = (None, [])
> I think raising RepoError should be fine. The error message isn't wrong.
On IRC, the RepoError's message kicked off a verging-on-religious 
discussion about backwards compatibility.

In the full context of the patch:

  * Original error message: "abort: repository green not found!"
  * New error message: "abort: repository green does not exist!"
  * The error code, which is more likely to be used in scripts than the
    error message, doesn't change.
  * It's more complex to preserve the "not found" message, which is how
    missed the 'not path' case you pointed out above. Less complexity is
    better.

Separately, I noticed that I missed the 'outgoing()' revset predicate, 
so v4 will fix that too. That means extra "error message preservation" 
complexity would be introduced in two places, not just one...

I'll send v4 momentarily.

Hollis Blanchard <hollis_blanchard@mentor.com>
Mentor Graphics Emulation Division

Patch

# HG changeset patch
# User Hollis Blanchard <hollis_blanchard@mentor.com>
# Date 1513292635 28800
#      Thu Dec 14 15:03:55 2017 -0800
# Node ID a71c6d5ea84933324e0c516ea2e4967e06616c31
# Parent  4937db58b663faa6893c51a41cec28114a165dd0
outgoing: respect ":pushurl" paths (issue5365)

Make 'hg outgoing' respect "paths.default:pushurl" in addition to
"paths.default-push".

'hg outgoing' has always meant "what will happen if I run 'hg push'?" and it's
still documented that way:

    Show changesets not found in the specified destination repository or the
    default push location. These are the changesets that would be pushed if a
    push was requested.

If the user uses the now-deprecated "paths.default-push" path, it continues to
work that way. However, as described at
https://bz.mercurial-scm.org/show_bug.cgi?id=5365, it doesn't behave the same
with "paths.default:pushurl".

Why does it matter? Similar to the bugzilla reporter, I have a read-only mirror
of a non-Mercurial repository:

  upstream -> imported mirror -> user clone
         ^-----------------------/

Users push directly to upstream, and that content is then imported into the
mirror. However, those repositories are not the same; it's possible that the
mirroring has either broken completely, or an import process is running and not
yet complete. In those cases, 'hg outgoing' will list changesets that have
already been pushed.

Mozilla's desired behavior described in bug 5365 can be accomplished through
other means (e.g. 'hg outgoing default'), preserving the consistency and
meaning of 'hg outgoing'.

diff --git a/mercurial/hg.py b/mercurial/hg.py
--- a/mercurial/hg.py
+++ b/mercurial/hg.py
@@ -912,8 +912,20 @@  def incoming(ui, repo, source, opts):
     return _incoming(display, subreporecurse, ui, repo, source, opts)
 
 def _outgoing(ui, repo, dest, opts):
-    dest = ui.expandpath(dest or 'default-push', dest or 'default')
-    dest, branches = parseurl(dest, opts.get('branch'))
+    path = None
+    try:
+        path = ui.paths.getpath(dest, default=('default-push', 'default'))
+        dest = path.pushloc or path.loc
+        branches = path.branch, opts.get('branch') or []
+    except error.RepoError:
+        # Fake up empty state to avoid changing the user-facing error message.
+        path = dest
+        branches = (None, [])
+
+    if not path:
+        raise error.Abort(_('default repository not configured!'),
+                hint=_("see 'hg help config.paths'"))
+
     ui.status(_('comparing with %s\n') % util.hidepassword(dest))
     revs, checkout = addbranchrevs(repo, repo, branches, opts.get('rev'))
     if revs:
diff --git a/tests/test-incoming-outgoing.t b/tests/test-incoming-outgoing.t
--- a/tests/test-incoming-outgoing.t
+++ b/tests/test-incoming-outgoing.t
@@ -491,3 +491,63 @@  incoming from empty remote repository
   searching for changes
   no changes found
   [1]
+
+Create a "split" repo that pulls from r1 and pushes to r2, using default-push
+
+  $ hg clone r1 split
+  updating to branch default
+  1 files updated, 0 files merged, 0 files removed, 0 files unresolved
+  $ cat > split/.hg/hgrc << EOF
+  > [paths]
+  > default = $TESTTMP/r3
+  > default-push = $TESTTMP/r2
+  > EOF
+  $ hg -R split outgoing
+  comparing with $TESTTMP/r2
+  searching for changes
+  changeset:   0:3e92d79f743a
+  tag:         tip
+  user:        test
+  date:        Thu Jan 01 00:00:00 1970 +0000
+  summary:     a
+  
+
+Use default:pushurl instead of default-push
+
+Windows needs a leading slash to make a URL that passes all of the checks
+  $ WD=`pwd`
+#if windows
+  $ WD="/$WD"
+#endif
+  $ cat > split/.hg/hgrc << EOF
+  > [paths]
+  > default = $WD/r3
+  > default:pushurl = file://$WD/r2
+  > EOF
+  $ hg -R split outgoing
+  comparing with file:/*/$TESTTMP/r2 (glob)
+  searching for changes
+  changeset:   0:3e92d79f743a
+  tag:         tip
+  user:        test
+  date:        Thu Jan 01 00:00:00 1970 +0000
+  summary:     a
+  
+
+Push and then double-check outgoing
+
+  $ echo a >> split/foo
+  $ hg -R split commit -Ama
+  $ hg -R split push
+  pushing to file:/*/$TESTTMP/r2 (glob)
+  searching for changes
+  adding changesets
+  adding manifests
+  adding file changes
+  added 2 changesets with 2 changes to 1 files
+  $ hg -R split outgoing
+  comparing with file:/*/$TESTTMP/r2 (glob)
+  searching for changes
+  no changes found
+  [1]
+