[PATCH 1 of 2] revert: add support for reverting subrepos

Angel Ezquerra angel.ezquerra at gmail.com
Wed Apr 18 09:12:13 CDT 2012


On Wed, Apr 18, 2012 at 2:20 PM, Patrick Mézard <patrick at mezard.eu> wrote:
> Le 17/04/12 21:21, Angel Ezquerra a écrit :
>> On Tue, Apr 17, 2012 at 6:57 PM, Patrick Mézard <patrick at mezard.eu> wrote:
>>> Le 15/04/12 00:49, Angel Ezquerra a écrit :
>>>> # HG changeset patch
>>>> # User Angel Ezquerra <angel.ezquerra at gmail.com>
>>>> # Date 1332927737 -7200
>>>> # Node ID 6952d4ae3b3d1af884ef8f98dbd78ed5db3abd3b
>>>> # Parent  eb13533a7849604864f42fb7eae3d8f11f0e7ea7
>>>> revert: add support for reverting subrepos
>>>>
>>>> Reverting a subrepo is done by updating it to the revision that is selected on
>>>> the parent repo .hgsubstate file.
>>>>
>>>> * ISSUES/TODO:
>>>>
>>>> - reverting added and removed subrepos is not supported yet.
>>>> - reverting subrepos is only supported if the --no-backup flag is used (this
>>>> limitation will be removed on another patch).
>>>> - The behavior of the --all flag has been changed. It now reverts subrepos as
>>>> well. Note that this may lead to data loss if the user has a dirty subrepo.
>>>>
>>>> diff --git a/mercurial/cmdutil.py b/mercurial/cmdutil.py
>>>> --- a/mercurial/cmdutil.py
>>>> +++ b/mercurial/cmdutil.py
>>>> @@ -1356,8 +1356,6 @@
>>>>              if path in names:
>>>>                  return
>>>>              if path in repo[node].substate:
>>>> -                ui.warn("%s: %s\n" % (m.rel(path),
>>>> -                    'reverting subrepos is unsupported'))
>>>>                  return
>>>>              path_ = path + '/'
>>>>              for f in names:
>>>
>>> Should not we keep at least a warning if the subrepo type does not implement revert?
>>
>> Patrick,
>>
>> I think you are right. What about changing the
>> abstractsubrepo.revert() method as follows?:
>>
>>     def revert(self, ui, substate, *pats, **opts):
>>         ui.warn('cannot revert subrepo %s '
>>             '(revert not implemented yet for %s)' \
>>             % (substate[0],type(self)))
>>         return []
>>
>> This would print something like:
>>
>>     cannot revert subrepo s (revert not implemented yet for <class
>> 'mercurial.subrep
>> o.hgsubrepo'>)
>>
>> but the call would not fail (i.e. the revert on the parent repo would continue).
>>
>> The alternative would be to "raise NotImplementedError"...
>>
>> What do you think?
>
> I would prefer it fails gracefully as it did before, but I have not looked how hard it would be to implement.
>
> --
> Patrick Mézard

The problem with doing that is that it would make "revert --all" fail
on any repository containing non hg subrepos, whilst in the past
"revert --all" would simply ignore all subrepos.

Instead I think that we should simply warn the user that it is just
not possible to revert and move on (i.e. continue reverting the next
specified file or subrepo).

What I could do is reuse the existing message, but add the type of
subrepo that does not support revert. That is, instead of printing:

include\mygitsubrepo: reverting subrepos is unsupported

print:

include\mygitsubrepo: reverting git subrepos is unsupported

(and do the same for all other types of subrepos).

I can send a patch for review.

Angel


More information about the Mercurial-devel mailing list