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

[Patrick Mézard]

Le 18/04/12 16:12, Angel Ezquerra a écrit :
> 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.
> 
> 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.

You are right, I misread your change. So yes, reverting to the previous behaviour for subrepos not supporting revert command sounds good to me.

--
Patrick Mézard