Patchwork subrepo: append subrepo path to subrepo push error messages

login
register
mail settings
Submitter Kevin Bullock
Date Dec. 14, 2012, 4:30 p.m.
Message ID <FD745E06-397D-459D-8B7E-787A6D390F23@ringworld.org>
Download mbox | patch
Permalink /patch/106/
State Not Applicable
Headers show

Comments

Kevin Bullock - Dec. 14, 2012, 4:30 p.m.
On Dec 14, 2012, at 2:41 AM, Angel Ezquerra wrote:

> # HG changeset patch
> # User Angel Ezquerra <angel.ezquerra at gmail.com>
> # Date 1355438273 -3600
> # Node ID b5af4658272135a1a9bbe67affc1955dd478861f
> # Parent  34a1a639d8358e43f4bcba7b0cff19f4e4e6958d
> subrepo: append subrepo path to subrepo push error messages
> 
> This change appends the subrepo path to subrepo push errors. That is, when there
> is an error pushing a subrepo, rather than displaying:
> 
> pushing subrepo MYSUBREPO to PATH
> searching for changes
> abort: push creates new remote head HEADHASH!
> hint: did you forget to merge? use push -f to force
> 
> mercurial will show:
> 
> pushing subrepo MYSUBREPO to PATH
> searching for changes
> abort: push creates new remote head HEADHASH! (on subrepo MYSUBREPO)
> hint: did you forget to merge? use push -f to force
> 
> The rationale for this change is that the current error messages make it hard
> for TortoiseHg (and similar tools) to tell the user which subrepo caused the
> push failure.

Your test does not do what you think it does:


ERROR: /Users/kbullock/Source/Projects/hg/hg/tests/test-subrepo.t output changed

...which indicates to me that your patch might not be doing what you think it does? That is, it's showing the _local_ path of the subrepo. I think that behavior is reasonable, but could you clarify?

pacem in terris / ??? / ?????? / ????????? / ??
Kevin R. Bullock
Angel Ezquerra - Dec. 16, 2012, 12:27 a.m.
On Fri, Dec 14, 2012 at 5:30 PM, Kevin Bullock
<kbullock+mercurial at ringworld.org> wrote:
> On Dec 14, 2012, at 2:41 AM, Angel Ezquerra wrote:
>
>> # HG changeset patch
>> # User Angel Ezquerra <angel.ezquerra at gmail.com>
>> # Date 1355438273 -3600
>> # Node ID b5af4658272135a1a9bbe67affc1955dd478861f
>> # Parent  34a1a639d8358e43f4bcba7b0cff19f4e4e6958d
>> subrepo: append subrepo path to subrepo push error messages
>>
>> This change appends the subrepo path to subrepo push errors. That is, when there
>> is an error pushing a subrepo, rather than displaying:
>>
>> pushing subrepo MYSUBREPO to PATH
>> searching for changes
>> abort: push creates new remote head HEADHASH!
>> hint: did you forget to merge? use push -f to force
>>
>> mercurial will show:
>>
>> pushing subrepo MYSUBREPO to PATH
>> searching for changes
>> abort: push creates new remote head HEADHASH! (on subrepo MYSUBREPO)
>> hint: did you forget to merge? use push -f to force
>>
>> The rationale for this change is that the current error messages make it hard
>> for TortoiseHg (and similar tools) to tell the user which subrepo caused the
>> push failure.
>
> Your test does not do what you think it does:
>
> --- /Users/kbullock/Source/Projects/hg/hg/tests/test-subrepo.t
> +++ /Users/kbullock/Source/Projects/hg/hg/tests/test-subrepo.t.err
> @@ -320,7 +320,7 @@
>    no changes found
>    pushing subrepo s to $TESTTMP/t/s (glob)
>    searching for changes
> -  abort: push creates new remote head 12a213df6fa9! (on subrepo $TESTTMP/t/s (glob))
> +  abort: push creates new remote head 12a213df6fa9! (on subrepo s)
>    (did you forget to merge? use push -f to force)
>    [255]
>    $ hg push -f
> @@ -587,7 +587,7 @@
>    created new head
>    $ hg -R repo2 ci -m3
>    $ hg -q -R repo2 push
> -  abort: push creates new remote head cc505f09a8b2!
> +  abort: push creates new remote head cc505f09a8b2! (on subrepo s)
>    (did you forget to merge? use push -f to force)
>    [255]
>    $ hg -R repo update
>
> ERROR: /Users/kbullock/Source/Projects/hg/hg/tests/test-subrepo.t output changed
>
> ...which indicates to me that your patch might not be doing what you think it does? That is, it's showing the _local_ path of the subrepo. I think that behavior is reasonable, but could you clarify?
>
> pacem in terris / ??? / ?????? / ????????? / ??
> Kevin R. Bullock
>

Thank you Kevin,

my intention is to show the local path, so the test is wrong as you
said. Thanks for reviewing the patch and pointing this out. I've now
been able to run the tests on my PC and I got a new version of the
patch that uses Matt Harbison's suggestion of using "subrelpath(self)"
rather than "self._path" and also fixes the test. I'll resend the
patch shortly.

Thank you both for your comments,

Angel

Patch

--- /Users/kbullock/Source/Projects/hg/hg/tests/test-subrepo.t
+++ /Users/kbullock/Source/Projects/hg/hg/tests/test-subrepo.t.err
@@ -320,7 +320,7 @@ 
   no changes found
   pushing subrepo s to $TESTTMP/t/s (glob)
   searching for changes
-  abort: push creates new remote head 12a213df6fa9! (on subrepo $TESTTMP/t/s (glob))
+  abort: push creates new remote head 12a213df6fa9! (on subrepo s)
   (did you forget to merge? use push -f to force)
   [255]
   $ hg push -f
@@ -587,7 +587,7 @@ 
   created new head
   $ hg -R repo2 ci -m3
   $ hg -q -R repo2 push
-  abort: push creates new remote head cc505f09a8b2!
+  abort: push creates new remote head cc505f09a8b2! (on subrepo s)
   (did you forget to merge? use push -f to force)
   [255]
   $ hg -R repo update