[PATCH V2] subrepo: append subrepo path to subrepo push error messages

Angel Ezquerra angel.ezquerra at gmail.com
Sun Dec 16 16:43:45 CST 2012


On Sun, Dec 16, 2012 at 2:57 AM, Mads Kiilerich <mads at kiilerich.com> wrote:
> Angel Ezquerra wrote, On 12/16/2012 01:29 AM:
>
>> # HG changeset patch
>> # User Angel Ezquerra <angel.ezquerra at gmail.com>
>> # Date 1355438273 -3600
>> # Node ID cec32425a026c4aeb9a6495086b1a1fbe916be24
>> # 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.
>>
>> Note that I have also updated test-subrepo.t to reflect this change. The
>> test
>> passes on windows.
>>
>> diff --git a/mercurial/subrepo.py b/mercurial/subrepo.py
>> --- a/mercurial/subrepo.py
>> +++ b/mercurial/subrepo.py
>> @@ -567,7 +567,12 @@
>>           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)
>> +        try:
>> +            res = self._repo.push(other, force, newbranch=newbranch)
>> +        except error.Abort, ex:
>> +            errormsg = ex.message + (' (on subrepo %s)' %
>> subrelpath(self))
>
>
> The message should be marked up for localization.

You are right. I believe that it would not be enough to put a "_" call
around the ' (on subrepo %s)' string, because it would not be found,
right? Instead I must do:

    errormsg = _('%s (on subrepo %s)' % (ex.message, subrelpath(self)))

Is that correct?

> I also wonder ... I assume a lot of other commands and exceptions from
> subrepo handling have the same issue. They should perhaps be solved while we
> are at it and with the same method. A decorator could perhaps be such a more
> generic method.decorator.
>
> Or is push more important than other commands, making this the only fix of
> this kind that is necessary?

I think push is more important in the sense that it is (in my
experience) the command that most commonly fails. That being said
probably most (if not all) other commands could benefit from this. I
decided to send this simple patch that fixes the main problem but if
you guys agree I can try to do the same for other commands.

As for using a decorator, are you thinking of something like this?:

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

If so, would we need to decorate every method on all the subrepo
subclasses? Maybe there is a simpler way to apply such a generator to
all methods? Also, I worry that adding ' (on subrepo %s)' to every
error message may result on weird error messages? Maybe it should be
changed to '(error happened on subrepo %s)' or something of the sort?

Cheers,

Angel


More information about the Mercurial-devel mailing list