Patchwork [V3,[RETITLED] ] subrepo: append subrepo path to subrepo error messages

login
register
mail settings
Submitter Angel Ezquerra
Date Dec. 16, 2012, 11:24 p.m.
Message ID <964686dbc23ea2e66c5f.1355700247@Lucia-PC>
Download mbox | patch
Permalink /patch/149/
State Superseded
Commit 9e3910db4e787787c315808f37ceecf695cb18c9
Headers show

Comments

Angel Ezquerra - Dec. 16, 2012, 11:24 p.m.
# HG changeset patch
# User Angel Ezquerra <angel.ezquerra at gmail.com>
# Date 1355438273 -3600
# Node ID 964686dbc23ea2e66c5f86ebffe96425096dd89c
# Parent  34a1a639d8358e43f4bcba7b0cff19f4e4e6958d
subrepo: append subrepo path to subrepo error messages

This change appends the subrepo path to subrepo errors. That is, when there
is an error performing an operation a subrepo, rather than displaying a message
such as:

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.

The "(on subrepo MYSUBREPO)" message has been added to those subrepo methods
were it made sense (by using a decorator).

Because the state() function already printed the subrepo path when it threw an
error, that error has been changed to avoid duplicating the subrepo path in the
error message.

Note that I have also updated test-subrepo.t to reflect these changes. The test
passes on windows.
Angel Ezquerra - Dec. 16, 2012, 11:27 p.m.
On Mon, Dec 17, 2012 at 12:24 AM, Angel Ezquerra
<angel.ezquerra at gmail.com> wrote:
> # HG changeset patch
> # User Angel Ezquerra <angel.ezquerra at gmail.com>
> # Date 1355438273 -3600
> # Node ID 964686dbc23ea2e66c5f86ebffe96425096dd89c
> # Parent  34a1a639d8358e43f4bcba7b0cff19f4e4e6958d
> subrepo: append subrepo path to subrepo error messages
>
> This change appends the subrepo path to subrepo errors. That is, when there
> is an error performing an operation a subrepo, rather than displaying a message
> such as:
>
> 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.
>
> The "(on subrepo MYSUBREPO)" message has been added to those subrepo methods
> were it made sense (by using a decorator).
>
> Because the state() function already printed the subrepo path when it threw an
> error, that error has been changed to avoid duplicating the subrepo path in the
> error message.
>
> Note that I have also updated test-subrepo.t to reflect these changes. The test
> passes on windows.
>

This is a new version of the patch that was titled "".
This new version tries to follow Mads advice of using a decorator and
adding the subrepo that caused the error to all subrepo errors.

I've tried to decorate those subrepo methods were I think it would
make sense to handle errors in this manner. I've also had to chagne an
existing error message to accommodate the way we append the subrepo
that is in error to all error messages.

Cheers,

Angel
Mads Kiilerich - Dec. 17, 2012, 12:35 a.m.
Angel Ezquerra wrote, On 12/17/2012 12:27 AM:
> On Mon, Dec 17, 2012 at 12:24 AM, Angel Ezquerra
> <angel.ezquerra at gmail.com> wrote:
>> # HG changeset patch
>> # User Angel Ezquerra <angel.ezquerra at gmail.com>
>> # Date 1355438273 -3600
>> # Node ID 964686dbc23ea2e66c5f86ebffe96425096dd89c
>> # Parent  34a1a639d8358e43f4bcba7b0cff19f4e4e6958d
>> subrepo: append subrepo path to subrepo error messages
>>
>> This change appends the subrepo path to subrepo errors. That is, when there
>> is an error performing an operation a subrepo, rather than displaying a message
>> such as:
>>
>> 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.
>>
>> The "(on subrepo MYSUBREPO)" message has been added to those subrepo methods
>> were it made sense (by using a decorator).
>>
>> Because the state() function already printed the subrepo path when it threw an
>> error, that error has been changed to avoid duplicating the subrepo path in the
>> error message.
>>
>> Note that I have also updated test-subrepo.t to reflect these changes. The test
>> passes on windows.
>>
> This is a new version of the patch that was titled "".
> This new version tries to follow Mads advice of using a decorator and
> adding the subrepo that caused the error to all subrepo errors.

I think it was more of an idea than an advice. And not necessarily a 
good idea ;-)

You annotate a lot of functions. Are they really all relevant? And if 
so: Shouldn't there be some test coverage of all the places where you 
add handling? And are you sure you don't end up with some Abort messages 
being annotated twice?

> +def handlessubrepoerror(func):
> +    def decoratedmethod(self, *args, **kargs):
> +        try:
> +            res = func(self, *args, **kargs)
> +        except error.Abort, ex:
> +            errormsg = _('%s (on subrepo %s)' % (ex.message, subrelpath(self)))

The _() should only wrap the text, not the % expansion.

> @@ -567,14 +587,18 @@
>           self._repo.ui.status(_('pushing subrepo %s to %s\n') %
>               (subrelpath(self), dsturl))
>           other = hg.peer(self._repo, {'ssh': ssh}, dsturl)
> -        return self._repo.push(other, force, newbranch=newbranch)
> +        res = self._repo.push(other, force, newbranch=newbranch)
> +        return res

That is an unnecessary change.

> @@ -645,7 +645,7 @@
>     added 2 changesets with 3 changes to 2 files
>     (run 'hg update' to get a working copy)
>     $ hg -R issue1852b update
> -  abort: default path for subrepository sub/repo not found (glob)
> +  abort: default path for subrepository not found (on subrepo sub\repo)
>     [255]

This is a first. We usually have problems forgetting that windows paths 
use \ ... but here it is the other way around. The \ should be a / and 
(glob) should be appended to the line so run-tests knows that both 
variants are accepted.


test-subrepo-git.t and test-subrepo-svn.t also needs updating (but might 
be a bit tricky to run on windows).


A message like
   abort: destination '$TESTTMP/almost-empty/foo' is not empty (on 
subrepo foo)
mig be ok, but a commit failing with
   abort: subrepo s is missing (on subrepo s)
seems a bit like it is repeating itself.

/Mads
Angel Ezquerra - Dec. 17, 2012, 6:41 a.m.
On Mon, Dec 17, 2012 at 1:35 AM, Mads Kiilerich <mads at kiilerich.com> wrote:
> Angel Ezquerra wrote, On 12/17/2012 12:27 AM:
>
>> On Mon, Dec 17, 2012 at 12:24 AM, Angel Ezquerra
>> <angel.ezquerra at gmail.com> wrote:
>>>
>>> # HG changeset patch
>>> # User Angel Ezquerra <angel.ezquerra at gmail.com>
>>> # Date 1355438273 -3600
>>> # Node ID 964686dbc23ea2e66c5f86ebffe96425096dd89c
>>> # Parent  34a1a639d8358e43f4bcba7b0cff19f4e4e6958d
>>> subrepo: append subrepo path to subrepo error messages
>>>
>>> This change appends the subrepo path to subrepo errors. That is, when
>>> there
>>> is an error performing an operation a subrepo, rather than displaying a
>>> message
>>> such as:
>>>
>>> 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.
>>>
>>> The "(on subrepo MYSUBREPO)" message has been added to those subrepo
>>> methods
>>> were it made sense (by using a decorator).
>>>
>>> Because the state() function already printed the subrepo path when it
>>> threw an
>>> error, that error has been changed to avoid duplicating the subrepo path
>>> in the
>>> error message.
>>>
>>> Note that I have also updated test-subrepo.t to reflect these changes.
>>> The test
>>> passes on windows.
>>>
>> This is a new version of the patch that was titled "".
>> This new version tries to follow Mads advice of using a decorator and
>> adding the subrepo that caused the error to all subrepo errors.
>
>
> I think it was more of an idea than an advice. And not necessarily a good
> idea ;-)

LOL. Let's see if we can make this more complete version work. If not
I can go back to the simple, initial proposal.

> You annotate a lot of functions. Are they really all relevant?

Since pretty much every mercurial command can throw an exception (e.g.
if the repisitory is broken) I thought that it would be a good idea to
wrap most subrepo methods. The ones I did not wrap are those that are
either private or which do not seem to possibly throw an error.

Do you think the error should be a bit more targeted, and just
decorate some key methods (e.g. push, get, commit, merge)?

> And if so:
> Shouldn't there be some test coverage of all the places where you add
> handling?

I don't think that should be necessary. The new functionality that I
add, which is the decorator is tested by the existing tests that I
modified already. The decorator works the same regardless of the
method that it decorates so why do more tests?

> And are you sure you don't end up with some Abort messages being
> annotated twice?

That is a good point. Most decorated functions are "top level methods"
which do not seem to be called by other subrepo methods, but there may
be some cases were that could happen. Also it could possibly happen if
there were several nested subrepos.

One solution would be to subclass the util.Abort exception (e.g. as
SubrepoAbort), and only append '(in subrepo %s)' when the catched
exception is not of the subclass exeception type.

>> +def handlessubrepoerror(func):
>> +    def decoratedmethod(self, *args, **kargs):
>> +        try:
>> +            res = func(self, *args, **kargs)
>> +        except error.Abort, ex:
>> +            errormsg = _('%s (on subrepo %s)' % (ex.message,
>> subrelpath(self)))
>
>
> The _() should only wrap the text, not the % expansion.

Thanks, I'll fix that.

>> @@ -567,14 +587,18 @@
>>           self._repo.ui.status(_('pushing subrepo %s to %s\n') %
>>               (subrelpath(self), dsturl))
>>           other = hg.peer(self._repo, {'ssh': ssh}, dsturl)
>> -        return self._repo.push(other, force, newbranch=newbranch)
>> +        res = self._repo.push(other, force, newbranch=newbranch)
>> +        return res
>
>
> That is an unnecessary change.

Left over from the simple version of the patch, sorry. I'll fix that as well.

>
>> @@ -645,7 +645,7 @@
>>     added 2 changesets with 3 changes to 2 files
>>     (run 'hg update' to get a working copy)
>>     $ hg -R issue1852b update
>> -  abort: default path for subrepository sub/repo not found (glob)
>> +  abort: default path for subrepository not found (on subrepo sub\repo)
>>     [255]
>
>
> This is a first. We usually have problems forgetting that windows paths use
> \ ... but here it is the other way around. The \ should be a / and (glob)
> should be appended to the line so run-tests knows that both variants are
> accepted.

:-) I guess that proves the value of getting the test suite to work on
windows, as hard as that has been.

I did not know what "(glob)" does. In hindsight it makes a lot of
sense. I'll fix that as well.

> test-subrepo-git.t and test-subrepo-svn.t also needs updating (but might be
> a bit tricky to run on windows).

Yeah, I don't think those work. I don't even have git, much less svn installed.
I could split this patch in three, resubmit it with the hg only
changes, which I can test right of the bat, and maybe resend the git
and svn changes if I get them to work somehow (perhaps on a Linux VM)?

> A message like
>   abort: destination '$TESTTMP/almost-empty/foo' is not empty (on subrepo
> foo)
> mig be ok, but a commit failing with
>   abort: subrepo s is missing (on subrepo s)
> seems a bit like it is repeating itself.

I agree. If I create the SubrepoAbort class that could be fixed by
throwing SubrepoAbort wherever the error message already includes the
subrepo path info. What do you think?

Thanks again for your review,

Angel
Angel Ezquerra - Dec. 18, 2012, 10:32 p.m.
On Mon, Dec 17, 2012 at 1:35 AM, Mads Kiilerich <mads at kiilerich.com> wrote:
> Angel Ezquerra wrote, On 12/17/2012 12:27 AM:
>
>> On Mon, Dec 17, 2012 at 12:24 AM, Angel Ezquerra
>> <angel.ezquerra at gmail.com> wrote:
>>>
>>> # HG changeset patch
>>> # User Angel Ezquerra <angel.ezquerra at gmail.com>
>>> # Date 1355438273 -3600
>>> # Node ID 964686dbc23ea2e66c5f86ebffe96425096dd89c
>>> # Parent  34a1a639d8358e43f4bcba7b0cff19f4e4e6958d
>>> subrepo: append subrepo path to subrepo error messages
>>>
>>> This change appends the subrepo path to subrepo errors. That is, when
>>> there
>>> is an error performing an operation a subrepo, rather than displaying a
>>> message
>>> such as:
>>>
>>> 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.
>>>
>>> The "(on subrepo MYSUBREPO)" message has been added to those subrepo
>>> methods
>>> were it made sense (by using a decorator).
>>>
>>> Because the state() function already printed the subrepo path when it
>>> threw an
>>> error, that error has been changed to avoid duplicating the subrepo path
>>> in the
>>> error message.
>>>
>>> Note that I have also updated test-subrepo.t to reflect these changes.
>>> The test
>>> passes on windows.
>>>
>> This is a new version of the patch that was titled "".
>> This new version tries to follow Mads advice of using a decorator and
>> adding the subrepo that caused the error to all subrepo errors.
>
>
> I think it was more of an idea than an advice. And not necessarily a good
> idea ;-)
>
> You annotate a lot of functions. Are they really all relevant? And if so:
> Shouldn't there be some test coverage of all the places where you add
> handling? And are you sure you don't end up with some Abort messages being
> annotated twice?
>
>
>> +def handlessubrepoerror(func):
>> +    def decoratedmethod(self, *args, **kargs):
>> +        try:
>> +            res = func(self, *args, **kargs)
>> +        except error.Abort, ex:
>> +            errormsg = _('%s (on subrepo %s)' % (ex.message,
>> subrelpath(self)))
>
>
> The _() should only wrap the text, not the % expansion.
>
>
>> @@ -567,14 +587,18 @@
>>           self._repo.ui.status(_('pushing subrepo %s to %s\n') %
>>               (subrelpath(self), dsturl))
>>           other = hg.peer(self._repo, {'ssh': ssh}, dsturl)
>> -        return self._repo.push(other, force, newbranch=newbranch)
>> +        res = self._repo.push(other, force, newbranch=newbranch)
>> +        return res
>
>
> That is an unnecessary change.
>
>
>> @@ -645,7 +645,7 @@
>>     added 2 changesets with 3 changes to 2 files
>>     (run 'hg update' to get a working copy)
>>     $ hg -R issue1852b update
>> -  abort: default path for subrepository sub/repo not found (glob)
>> +  abort: default path for subrepository not found (on subrepo sub\repo)
>>     [255]
>
>
> This is a first. We usually have problems forgetting that windows paths use
> \ ... but here it is the other way around. The \ should be a / and (glob)
> should be appended to the line so run-tests knows that both variants are
> accepted.
>
>
> test-subrepo-git.t and test-subrepo-svn.t also needs updating (but might be
> a bit tricky to run on windows).
>
>
> A message like
>   abort: destination '$TESTTMP/almost-empty/foo' is not empty (on subrepo
> foo)
> mig be ok, but a commit failing with
>   abort: subrepo s is missing (on subrepo s)
> seems a bit like it is repeating itself.
>
> /Mads

Mads,

I forgot to mention that I think it is not bad to show the local path
even when you show the remote path on the error message. These are
usually very similar but if you use absolute subrepo paths they can be
very different as well.

Cheers,

Angel

Patch

diff --git a/mercurial/subrepo.py b/mercurial/subrepo.py
--- a/mercurial/subrepo.py
+++ b/mercurial/subrepo.py
@@ -14,6 +14,16 @@ 
 
 nullstate = ('', '', 'empty')
 
+def handlessubrepoerror(func):
+    def decoratedmethod(self, *args, **kargs):
+        try:
+            res = func(self, *args, **kargs)
+        except error.Abort, ex:
+            errormsg = _('%s (on subrepo %s)' % (ex.message, subrelpath(self)))
+            raise util.Abort(errormsg, hint=ex.hint)
+        return res
+    return decoratedmethod
+
 def state(ctx, ui):
     """return a state dict, mapping subrepo paths configured in .hgsub
     to tuple: (source from .hgsub, revision from .hgsubstate, kind
@@ -244,8 +254,7 @@ 
         if repo.ui.config('paths', 'default'):
             return repo.ui.config('paths', 'default')
     if abort:
-        raise util.Abort(_("default path for subrepository %s not found") %
-            reporelpath(repo))
+        raise util.Abort(_("default path for subrepository not found"))
 
 def itersubrepos(ctx1, ctx2):
     """find subrepos in ctx1 or ctx2"""
@@ -402,6 +411,7 @@ 
                 self._repo.ui.setconfig(s, k, v)
         self._initrepo(r, state[0], create)
 
+    @handlessubrepoerror
     def _initrepo(self, parentrepo, source, create):
         self._repo._subparent = parentrepo
         self._repo._subsource = source
@@ -422,10 +432,12 @@ 
                 addpathconfig('default-push', defpushpath)
             fp.close()
 
+    @handlessubrepoerror
     def add(self, ui, match, dryrun, listsubrepos, prefix, explicitonly):
         return cmdutil.add(ui, self._repo, match, dryrun, listsubrepos,
                            os.path.join(prefix, self._path), explicitonly)
 
+    @handlessubrepoerror
     def status(self, rev2, **opts):
         try:
             rev1 = self._state[1]
@@ -437,6 +449,7 @@ 
                                % (inst, subrelpath(self)))
             return [], [], [], [], [], [], []
 
+    @handlessubrepoerror
     def diff(self, ui, diffopts, node2, match, prefix, **opts):
         try:
             node1 = node.bin(self._state[1])
@@ -452,6 +465,7 @@ 
             self._repo.ui.warn(_('warning: error "%s" in subrepository "%s"\n')
                                % (inst, subrelpath(self)))
 
+    @handlessubrepoerror
     def archive(self, ui, archiver, prefix, match=None):
         self._get(self._state + ('hg',))
         abstractsubrepo.archive(self, ui, archiver, prefix, match)
@@ -463,6 +477,7 @@ 
             submatch = matchmod.narrowmatcher(subpath, match)
             s.archive(ui, archiver, os.path.join(prefix, self._path), submatch)
 
+    @handlessubrepoerror
     def dirty(self, ignoreupdate=False):
         r = self._state[1]
         if r == '' and not ignoreupdate: # no state recorded
@@ -479,6 +494,7 @@ 
     def checknested(self, path):
         return self._repo._checknested(self._repo.wjoin(path))
 
+    @handlessubrepoerror
     def commit(self, text, user, date):
         # don't bother committing in the subrepo if it's only been
         # updated
@@ -490,6 +506,7 @@ 
             return self._repo['.'].hex() # different version checked out
         return node.hex(n)
 
+    @handlessubrepoerror
     def remove(self):
         # we can't fully delete the repository as it may contain
         # local-only history
@@ -519,12 +536,14 @@ 
                 bookmarks.updatefromremote(self._repo.ui, self._repo, other,
                                            srcurl)
 
+    @handlessubrepoerror
     def get(self, state, overwrite=False):
         self._get(state)
         source, revision, kind = state
         self._repo.ui.debug("getting subrepo %s\n" % self._path)
         hg.updaterepo(self._repo, revision, overwrite)
 
+    @handlessubrepoerror
     def merge(self, state):
         self._get(state)
         cur = self._repo['.']
@@ -551,6 +570,7 @@ 
         else:
             mergefunc()
 
+    @handlessubrepoerror
     def push(self, opts):
         force = opts.get('force')
         newbranch = opts.get('new_branch')
@@ -567,14 +587,18 @@ 
         self._repo.ui.status(_('pushing subrepo %s to %s\n') %
             (subrelpath(self), dsturl))
         other = hg.peer(self._repo, {'ssh': ssh}, dsturl)
-        return self._repo.push(other, force, newbranch=newbranch)
+        res = self._repo.push(other, force, newbranch=newbranch)
+        return res
 
+    @handlessubrepoerror
     def outgoing(self, ui, dest, opts):
         return hg.outgoing(ui, self._repo, _abssource(self._repo, True), opts)
 
+    @handlessubrepoerror
     def incoming(self, ui, source, opts):
         return hg.incoming(ui, self._repo, _abssource(self._repo, False), opts)
 
+    @handlessubrepoerror
     def files(self):
         rev = self._state[1]
         ctx = self._repo[rev]
@@ -593,10 +617,12 @@ 
         ctx = self._repo[None]
         return ctx.walk(match)
 
+    @handlessubrepoerror
     def forget(self, ui, match, prefix):
         return cmdutil.forget(ui, self._repo, match,
                               os.path.join(prefix, self._path), True)
 
+    @handlessubrepoerror
     def revert(self, ui, substate, *pats, **opts):
         # reverting a subrepo is a 2 step process:
         # 1. if the no_backup is not set, revert all modified
@@ -751,6 +777,7 @@ 
                 pass
         return rev
 
+    @handlessubrepoerror
     def commit(self, text, user, date):
         # user and date are out of our hands since svn is centralized
         changed, extchanged, missing = self._wcchanged()
@@ -778,6 +805,7 @@ 
         self._ui.status(self._svncommand(['update', '-r', newrev])[0])
         return newrev
 
+    @handlessubrepoerror
     def remove(self):
         if self.dirty():
             self._ui.warn(_('not removing repo %s because '
@@ -802,6 +830,7 @@ 
         except OSError:
             pass
 
+    @handlessubrepoerror
     def get(self, state, overwrite=False):
         if overwrite:
             self._svncommand(['revert', '--recursive'])
@@ -822,6 +851,7 @@ 
             raise util.Abort((status or err).splitlines()[-1])
         self._ui.status(status)
 
+    @handlessubrepoerror
     def merge(self, state):
         old = self._state[1]
         new = state[1]
@@ -835,6 +865,7 @@ 
         # push is a no-op for SVN
         return True
 
+    @handlessubrepoerror
     def files(self):
         output = self._svncommand(['list', '--recursive', '--xml'])[0]
         doc = xml.dom.minidom.parseString(output)
@@ -1021,6 +1052,7 @@ 
             raise util.Abort(_("revision %s does not exist in subrepo %s\n") %
                                (revision, self._relpath))
 
+    @handlessubrepoerror
     def dirty(self, ignoreupdate=False):
         if self._gitmissing():
             return self._state[1] != ''
@@ -1037,6 +1069,7 @@ 
     def basestate(self):
         return self._gitstate()
 
+    @handlessubrepoerror
     def get(self, state, overwrite=False):
         source, revision, kind = state
         if not revision:
@@ -1120,6 +1153,7 @@ 
             # a real merge would be required, just checkout the revision
             rawcheckout()
 
+    @handlessubrepoerror
     def commit(self, text, user, date):
         if self._gitmissing():
             raise util.Abort(_("subrepo %s is missing") % self._relpath)
@@ -1137,6 +1171,7 @@ 
         # circumstances
         return self._gitstate()
 
+    @handlessubrepoerror
     def merge(self, state):
         source, revision, kind = state
         self._fetch(source, revision)
@@ -1159,6 +1194,7 @@ 
         else:
             mergefunc()
 
+    @handlessubrepoerror
     def push(self, opts):
         force = opts.get('force')
 
@@ -1198,6 +1234,7 @@ 
                           (self._relpath, self._state[1]))
             return False
 
+    @handlessubrepoerror
     def remove(self):
         if self._gitmissing():
             return
@@ -1247,6 +1284,7 @@ 
         ui.progress(_('archiving (%s)') % relpath, None)
 
 
+    @handlessubrepoerror
     def status(self, rev2, **opts):
         rev1 = self._state[1]
         if self._gitmissing() or not rev1:
diff --git a/tests/test-subrepo.t b/tests/test-subrepo.t
--- a/tests/test-subrepo.t
+++ b/tests/test-subrepo.t
@@ -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!
+  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
@@ -599,7 +599,7 @@ 
   $ hg -R repo2 push -f -q
   $ hg -R repo update
   b: untracked file differs
-  abort: untracked files in working directory differ from files in requested revision
+  abort: untracked files in working directory differ from files in requested revision (on subrepo s)
   [255]
 
   $ cat repo/s/b
@@ -645,7 +645,7 @@ 
   added 2 changesets with 3 changes to 2 files
   (run 'hg update' to get a working copy)
   $ hg -R issue1852b update
-  abort: default path for subrepository sub/repo not found (glob)
+  abort: default path for subrepository not found (on subrepo sub\repo)
   [255]
 
 Pull -u now doesn't help