Patchwork subrepo: add revert support without backup for git subrepos

login
register
mail settings
Submitter Mathias De Maré
Date Dec. 14, 2014, 3:29 p.m.
Message ID <cd6707c55245bf83d457.1418570945@mathias-Latitude-E6540>
Download mbox | patch
Permalink /patch/7085/
State Accepted
Delegated to: Pierre-Yves David
Headers show

Comments

Mathias De Maré - Dec. 14, 2014, 3:29 p.m.
# HG changeset patch
# User Mathias De Maré <mathias.demare@gmail.com>
# Date 1418553291 -3600
#      Son Dez 14 11:34:51 2014 +0100
# Node ID cd6707c55245bf83d45767b0e685b137f9662e04
# Parent  e33179ad88c9aa4ff241d57d37c227381a38098e
subrepo: add revert support without backup for git subrepos

Previously, git subrepos did not support reverting.
This change adds basic support for reverting
when '--no-backup' is specified.
A warning is given (and the current state is kept)
when a revert is done without the '--no-backup' flag.
Pierre-Yves David - Dec. 14, 2014, 9:43 p.m.
On 12/14/2014 07:29 AM, Mathias De Maré wrote:
> # HG changeset patch
> # User Mathias De Maré <mathias.demare@gmail.com>
> # Date 1418553291 -3600
> #      Son Dez 14 11:34:51 2014 +0100
> # Node ID cd6707c55245bf83d45767b0e685b137f9662e04
> # Parent  e33179ad88c9aa4ff241d57d37c227381a38098e
> subrepo: add revert support without backup for git subrepos
>
> Previously, git subrepos did not support reverting.
> This change adds basic support for reverting
> when '--no-backup' is specified.
> A warning is given (and the current state is kept)
> when a revert is done without the '--no-backup' flag.

I'm a bit confused by the --no-backup business (I've the gut feeling we 
could be doing something else). But you patch seems a certain move forward.

Pushed to the clowncopter. Thanks.

> diff --git a/mercurial/subrepo.py b/mercurial/subrepo.py
> --- a/mercurial/subrepo.py
> +++ b/mercurial/subrepo.py
> @@ -1651,16 +1651,27 @@ class gitsubrepo(abstractsubrepo):
>           if match.always():
>               ui.write(self._gitcommand(cmd))
>           elif match.files():
>               for f in match.files():
>                   ui.write(self._gitcommand(cmd + [f]))
>           elif match(gitprefix): #Subrepo is matched
>               ui.write(self._gitcommand(cmd))
>
> +    def revert(self, ui, substate, *pats, **opts):
> +        ui.status(_('reverting subrepo %s\n') % substate[0])
> +        if not opts.get('no_backup'):
> +            ui.warn('%s: reverting %s subrepos without ' \
> +                    '--no-backup is unsupported\n' \
> +                % (substate[0], substate[2]))

I've dropped the used \ and fixed the identation.

> +            return []
> +
> +        self.get(substate, overwrite=True)
> +        return []
> +
>       def shortid(self, revid):
>           return revid[:7]
>
>   types = {
>       'hg': hgsubrepo,
>       'svn': svnsubrepo,
>       'git': gitsubrepo,
>       }
> diff --git a/tests/test-subrepo-git.t b/tests/test-subrepo-git.t
> --- a/tests/test-subrepo-git.t
> +++ b/tests/test-subrepo-git.t
> @@ -787,9 +787,27 @@ the output contains a regex, because git
>     \s*barfoo |\s*1 + (re)
>     \s*foobar |\s*2 +- (re)
>      2 files changed, 2 insertions(+), 2 deletions(-) (no-eol)
>
>   ensure adding include/exclude ignores the subrepo
>     $ hg diff --subrepos -I s/foobar
>     $ hg diff --subrepos -X s/foobar
>
> +
> +revert the subrepository
> +  $ hg revert --all
> +  reverting subrepo ../gitroot (glob)
> +  ../gitroot: reverting git subrepos without --no-backup is unsupported (glob)
> +
> +  $ hg status --subrepos
> +  M s/foobar
> +  A s/barfoo
> +
> +  $ hg revert --no-backup --all
> +  reverting subrepo ../gitroot (glob)
> +  $ hg revert --no-backup s
> +  reverting subrepo ../gitroot (glob)
> +
> +  $ hg status --subrepos
> +  ? s/barfoo
> +
Mathias De Maré - Dec. 15, 2014, 6:22 a.m.
On Sun, Dec 14, 2014 at 10:43 PM, Pierre-Yves David <
pierre-yves.david@ens-lyon.org> wrote:
>
>
>
> On 12/14/2014 07:29 AM, Mathias De Maré wrote:
>
>> # HG changeset patch
>> # User Mathias De Maré <mathias.demare@gmail.com>
>> # Date 1418553291 -3600
>> #      Son Dez 14 11:34:51 2014 +0100
>> # Node ID cd6707c55245bf83d45767b0e685b137f9662e04
>> # Parent  e33179ad88c9aa4ff241d57d37c227381a38098e
>> subrepo: add revert support without backup for git subrepos
>>
>> Previously, git subrepos did not support reverting.
>> This change adds basic support for reverting
>> when '--no-backup' is specified.
>> A warning is given (and the current state is kept)
>> when a revert is done without the '--no-backup' flag.
>>
>
> I'm a bit confused by the --no-backup business (I've the gut feeling we
> could be doing something else). But you patch seems a certain move forward.
>

One thing I was thinking about was to use the subrepo 'status' function to
get a list of all the modified files, and copy those to .orig files. Not
sure if that sounds like a good plan, comments welcome :-)

>
> Pushed to the clowncopter. Thanks.
>

Thanks for the push!

>
>  diff --git a/mercurial/subrepo.py b/mercurial/subrepo.py
>> --- a/mercurial/subrepo.py
>> +++ b/mercurial/subrepo.py
>> @@ -1651,16 +1651,27 @@ class gitsubrepo(abstractsubrepo):
>>           if match.always():
>>               ui.write(self._gitcommand(cmd))
>>           elif match.files():
>>               for f in match.files():
>>                   ui.write(self._gitcommand(cmd + [f]))
>>           elif match(gitprefix): #Subrepo is matched
>>               ui.write(self._gitcommand(cmd))
>>
>> +    def revert(self, ui, substate, *pats, **opts):
>> +        ui.status(_('reverting subrepo %s\n') % substate[0])
>> +        if not opts.get('no_backup'):
>> +            ui.warn('%s: reverting %s subrepos without ' \
>> +                    '--no-backup is unsupported\n' \
>> +                % (substate[0], substate[2]))
>>
>
> I've dropped the used \ and fixed the identation.
>
>
>  +            return []
>> +
>> +        self.get(substate, overwrite=True)
>> +        return []
>> +
>>       def shortid(self, revid):
>>           return revid[:7]
>>
>>   types = {
>>       'hg': hgsubrepo,
>>       'svn': svnsubrepo,
>>       'git': gitsubrepo,
>>       }
>> diff --git a/tests/test-subrepo-git.t b/tests/test-subrepo-git.t
>> --- a/tests/test-subrepo-git.t
>> +++ b/tests/test-subrepo-git.t
>> @@ -787,9 +787,27 @@ the output contains a regex, because git
>>     \s*barfoo |\s*1 + (re)
>>     \s*foobar |\s*2 +- (re)
>>      2 files changed, 2 insertions(+), 2 deletions(-) (no-eol)
>>
>>   ensure adding include/exclude ignores the subrepo
>>     $ hg diff --subrepos -I s/foobar
>>     $ hg diff --subrepos -X s/foobar
>>
>> +
>> +revert the subrepository
>> +  $ hg revert --all
>> +  reverting subrepo ../gitroot (glob)
>> +  ../gitroot: reverting git subrepos without --no-backup is unsupported
>> (glob)
>> +
>> +  $ hg status --subrepos
>> +  M s/foobar
>> +  A s/barfoo
>> +
>> +  $ hg revert --no-backup --all
>> +  reverting subrepo ../gitroot (glob)
>> +  $ hg revert --no-backup s
>> +  reverting subrepo ../gitroot (glob)
>> +
>> +  $ hg status --subrepos
>> +  ? s/barfoo
>> +
>>
>
> --
> Pierre-Yves David
>

Patch

diff --git a/mercurial/subrepo.py b/mercurial/subrepo.py
--- a/mercurial/subrepo.py
+++ b/mercurial/subrepo.py
@@ -1651,16 +1651,27 @@  class gitsubrepo(abstractsubrepo):
         if match.always():
             ui.write(self._gitcommand(cmd))
         elif match.files():
             for f in match.files():
                 ui.write(self._gitcommand(cmd + [f]))
         elif match(gitprefix): #Subrepo is matched
             ui.write(self._gitcommand(cmd))
 
+    def revert(self, ui, substate, *pats, **opts):
+        ui.status(_('reverting subrepo %s\n') % substate[0])
+        if not opts.get('no_backup'):
+            ui.warn('%s: reverting %s subrepos without ' \
+                    '--no-backup is unsupported\n' \
+                % (substate[0], substate[2]))
+            return []
+
+        self.get(substate, overwrite=True)
+        return []
+
     def shortid(self, revid):
         return revid[:7]
 
 types = {
     'hg': hgsubrepo,
     'svn': svnsubrepo,
     'git': gitsubrepo,
     }
diff --git a/tests/test-subrepo-git.t b/tests/test-subrepo-git.t
--- a/tests/test-subrepo-git.t
+++ b/tests/test-subrepo-git.t
@@ -787,9 +787,27 @@  the output contains a regex, because git
   \s*barfoo |\s*1 + (re)
   \s*foobar |\s*2 +- (re)
    2 files changed, 2 insertions(+), 2 deletions(-) (no-eol)
 
 ensure adding include/exclude ignores the subrepo
   $ hg diff --subrepos -I s/foobar
   $ hg diff --subrepos -X s/foobar
 
+
+revert the subrepository
+  $ hg revert --all
+  reverting subrepo ../gitroot (glob)
+  ../gitroot: reverting git subrepos without --no-backup is unsupported (glob)
+
+  $ hg status --subrepos
+  M s/foobar
+  A s/barfoo
+
+  $ hg revert --no-backup --all
+  reverting subrepo ../gitroot (glob)
+  $ hg revert --no-backup s
+  reverting subrepo ../gitroot (glob)
+
+  $ hg status --subrepos
+  ? s/barfoo
+
   $ cd ..