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! (in 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 "(in subrepo MYSUBREPO)" message has been added to those subrepo methods
were it made sense (by using a decorator). We avoid appending "(in subrepo XXX)"
multiple times when subrepos are nexted by throwing a "SubrepoAbort" exception
after the extra message is appended. The decorator will then "ignore" (i.e. just
re-raise) the exception and never add the message again.

A small drawback of this method is that part of the exception trace is lost when
the exception is catched and re-raised by the annotatesubrepoerror decorator.

Also, 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 several subrepo related tests to reflect these
changes.

mercurial/subrepo.py

@@ -14,6 +14,23 b' propertycache = util.propertycache'
 
 nullstate = ('', '', 'empty')
 
+class SubrepoAbort(error.Abort):
+    """Exception class used to avoid handling a subrepo error more than once"""
+
+def annotatesubrepoerror(func):
+    def decoratedmethod(self, *args, **kargs):
+        try:
+            res = func(self, *args, **kargs)
+        except SubrepoAbort, ex:
+            # This exception has already been handled
+            raise ex
+        except error.Abort, ex:
+            errormsg = _('%s (in subrepo %s)') % (str(ex), subrelpath(self))
+            # avoid handling this exception by raising a SubrepoAbort exception
+            raise SubrepoAbort(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 +261,7 b' def _abssource(repo, push=False, abort=T'
         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 +418,7 b' class hgsubrepo(abstractsubrepo):'
                 self._repo.ui.setconfig(s, k, v)
         self._initrepo(r, state[0], create)
 
+    @annotatesubrepoerror
     def _initrepo(self, parentrepo, source, create):
         self._repo._subparent = parentrepo
         self._repo._subsource = source
@@ -422,10 +439,12 b' class hgsubrepo(abstractsubrepo):'
                 addpathconfig('default-push', defpushpath)
             fp.close()
 
+    @annotatesubrepoerror
     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)
 
+    @annotatesubrepoerror
     def status(self, rev2, **opts):
         try:
             rev1 = self._state[1]
@@ -437,6 +456,7 b' class hgsubrepo(abstractsubrepo):'
                                % (inst, subrelpath(self)))
             return [], [], [], [], [], [], []
 
+    @annotatesubrepoerror
     def diff(self, ui, diffopts, node2, match, prefix, **opts):
         try:
             node1 = node.bin(self._state[1])
@@ -452,6 +472,7 b' class hgsubrepo(abstractsubrepo):'
             self._repo.ui.warn(_('warning: error "%s" in subrepository "%s"\n')
                                % (inst, subrelpath(self)))
 
+    @annotatesubrepoerror
     def archive(self, ui, archiver, prefix, match=None):
         self._get(self._state + ('hg',))
         abstractsubrepo.archive(self, ui, archiver, prefix, match)
@@ -463,6 +484,7 b' class hgsubrepo(abstractsubrepo):'
             submatch = matchmod.narrowmatcher(subpath, match)
             s.archive(ui, archiver, os.path.join(prefix, self._path), submatch)
 
+    @annotatesubrepoerror
     def dirty(self, ignoreupdate=False):
         r = self._state[1]
         if r == '' and not ignoreupdate: # no state recorded
@@ -479,6 +501,7 b' class hgsubrepo(abstractsubrepo):'
     def checknested(self, path):
         return self._repo._checknested(self._repo.wjoin(path))
 
+    @annotatesubrepoerror
     def commit(self, text, user, date):
         # don't bother committing in the subrepo if it's only been
         # updated
@@ -490,6 +513,7 b' class hgsubrepo(abstractsubrepo):'
             return self._repo['.'].hex() # different version checked out
         return node.hex(n)
 
+    @annotatesubrepoerror
     def remove(self):
         # we can't fully delete the repository as it may contain
         # local-only history
@@ -519,12 +543,14 b' class hgsubrepo(abstractsubrepo):'
                 bookmarks.updatefromremote(self._repo.ui, self._repo, other,
                                            srcurl)
 
+    @annotatesubrepoerror
     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)
 
+    @annotatesubrepoerror
     def merge(self, state):
         self._get(state)
         cur = self._repo['.']
@@ -551,6 +577,7 b' class hgsubrepo(abstractsubrepo):'
         else:
             mergefunc()
 
+    @annotatesubrepoerror
     def push(self, opts):
         force = opts.get('force')
         newbranch = opts.get('new_branch')
@@ -569,12 +596,15 b' class hgsubrepo(abstractsubrepo):'
         other = hg.peer(self._repo, {'ssh': ssh}, dsturl)
         return self._repo.push(other, force, newbranch=newbranch)
 
+    @annotatesubrepoerror
     def outgoing(self, ui, dest, opts):
         return hg.outgoing(ui, self._repo, _abssource(self._repo, True), opts)
 
+    @annotatesubrepoerror
     def incoming(self, ui, source, opts):
         return hg.incoming(ui, self._repo, _abssource(self._repo, False), opts)
 
+    @annotatesubrepoerror
     def files(self):
         rev = self._state[1]
         ctx = self._repo[rev]
@@ -593,10 +623,12 b' class hgsubrepo(abstractsubrepo):'
         ctx = self._repo[None]
         return ctx.walk(match)
 
+    @annotatesubrepoerror
     def forget(self, ui, match, prefix):
         return cmdutil.forget(ui, self._repo, match,
                               os.path.join(prefix, self._path), True)
 
+    @annotatesubrepoerror
     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 +783,7 b' class svnsubrepo(abstractsubrepo):'
                 pass
         return rev
 
+    @annotatesubrepoerror
     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 +811,7 b' class svnsubrepo(abstractsubrepo):'
         self._ui.status(self._svncommand(['update', '-r', newrev])[0])
         return newrev
 
+    @annotatesubrepoerror
     def remove(self):
         if self.dirty():
             self._ui.warn(_('not removing repo %s because '
@@ -802,6 +836,7 b' class svnsubrepo(abstractsubrepo):'
         except OSError:
             pass
 
+    @annotatesubrepoerror
     def get(self, state, overwrite=False):
         if overwrite:
             self._svncommand(['revert', '--recursive'])
@@ -822,6 +857,7 b' class svnsubrepo(abstractsubrepo):'
             raise util.Abort((status or err).splitlines()[-1])
         self._ui.status(status)
 
+    @annotatesubrepoerror
     def merge(self, state):
         old = self._state[1]
         new = state[1]
@@ -835,6 +871,7 b' class svnsubrepo(abstractsubrepo):'
         # push is a no-op for SVN
         return True
 
+    @annotatesubrepoerror
     def files(self):
         output = self._svncommand(['list', '--recursive', '--xml'])[0]
         doc = xml.dom.minidom.parseString(output)
@@ -1021,6 +1058,7 b' class gitsubrepo(abstractsubrepo):'
             raise util.Abort(_("revision %s does not exist in subrepo %s\n") %
                                (revision, self._relpath))
 
+    @annotatesubrepoerror
     def dirty(self, ignoreupdate=False):
         if self._gitmissing():
             return self._state[1] != ''
@@ -1037,6 +1075,7 b' class gitsubrepo(abstractsubrepo):'
     def basestate(self):
         return self._gitstate()
 
+    @annotatesubrepoerror
     def get(self, state, overwrite=False):
         source, revision, kind = state
         if not revision:
@@ -1120,6 +1159,7 b' class gitsubrepo(abstractsubrepo):'
             # a real merge would be required, just checkout the revision
             rawcheckout()
 
+    @annotatesubrepoerror
     def commit(self, text, user, date):
         if self._gitmissing():
             raise util.Abort(_("subrepo %s is missing") % self._relpath)
@@ -1137,6 +1177,7 b' class gitsubrepo(abstractsubrepo):'
         # circumstances
         return self._gitstate()
 
+    @annotatesubrepoerror
     def merge(self, state):
         source, revision, kind = state
         self._fetch(source, revision)
@@ -1159,6 +1200,7 b' class gitsubrepo(abstractsubrepo):'
         else:
             mergefunc()
 
+    @annotatesubrepoerror
     def push(self, opts):
         force = opts.get('force')
 
@@ -1198,6 +1240,7 b' class gitsubrepo(abstractsubrepo):'
                           (self._relpath, self._state[1]))
             return False
 
+    @annotatesubrepoerror
     def remove(self):
         if self._gitmissing():
             return
@@ -1247,6 +1290,7 b' class gitsubrepo(abstractsubrepo):'
         ui.progress(_('archiving (%s)') % relpath, None)
 
 
+    @annotatesubrepoerror
     def status(self, rev2, **opts):
         rev1 = self._state[1]
         if self._gitmissing() or not rev1:

tests/test-subrepo-git.t

@@ -331,10 +331,10 b" Don't crash if the subrepo is missing"
   $ hg sum | grep commit
   commit: 1 subrepos
   $ hg push -q
-  abort: subrepo s is missing
+  abort: subrepo s is missing (in subrepo s)
   [255]
   $ hg commit --subrepos -qm missing
-  abort: subrepo s is missing
+  abort: subrepo s is missing (in subrepo s)
   [255]
   $ hg update -C
   cloning subrepo s from $TESTTMP/gitroot

tests/test-subrepo-recursion.t

@@ -386,7 +386,7 b' created by archive:'
   $ echo f > foo/f
   $ hg archive --subrepos -r tip archive
   cloning subrepo foo from $TESTTMP/empty/foo
-  abort: destination '$TESTTMP/almost-empty/foo' is not empty (glob)
+  abort: destination '$TESTTMP/almost-empty/foo' is not empty (in subrepo foo) (glob)
   [255]
 
 Clone and test outgoing:

tests/test-subrepo-svn.t

@@ -119,7 +119,7 b' missing svn file, commit should fail'
   $ rm s/alpha
   $ hg commit --subrepos -m 'abort on missing file'
   committing subrepository s
-  abort: cannot commit missing svn entries
+  abort: cannot commit missing svn entries (in subrepo s)
   [255]
   $ svn revert s/alpha > /dev/null
 
@@ -180,7 +180,7 b' this commit fails because of externals c'
   $ echo zzz > s/externals/other
   $ hg ci --subrepos -m 'amend externals from hg'
   committing subrepository s
-  abort: cannot commit svn externals
+  abort: cannot commit svn externals (in subrepo s)
   [255]
   $ hg diff --subrepos -r 1:2 | grep -v diff
   --- a/.hgsubstate	Thu Jan 01 00:00:00 1970 +0000
@@ -202,7 +202,7 b' this commit fails because of externals m'
   property 'svn:mime-type' set on 's/externals/other' (glob)
   $ hg ci --subrepos -m 'amend externals from hg'
   committing subrepository s
-  abort: cannot commit svn externals
+  abort: cannot commit svn externals (in subrepo s)
   [255]
   $ svn revert -q s/externals/other
 

tests/test-subrepo.t

@@ -320,7 +320,7 b' push -f'
   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! (in subrepo s)
   (did you forget to merge? use push -f to force)
   [255]
   $ hg push -f
@@ -587,7 +587,7 b' Issue1977: multirepo push should fail if'
   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! (in subrepo s)
   (did you forget to merge? use push -f to force)
   [255]
   $ hg -R repo update
@@ -599,7 +599,7 b' test if untracked file is not overwritte'
   $ 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 (in subrepo s)
   [255]
 
   $ cat repo/s/b
@@ -645,7 +645,7 b' Create repo without default path, pull t'
   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 (in subrepo sub/repo) (glob)
   [255]
 
 Pull -u now doesn't help