Patchwork [2,of,2] addremove: add --subrepos flag

login
register
mail settings
Submitter Angel Ezquerra
Date Aug. 30, 2014, 3:09 p.m.
Message ID <2d92ae26a1e326bb6210.1409411349@angels-macbook-pro.local>
Download mbox | patch
Permalink /patch/5639/
State Changes Requested
Headers show

Comments

Angel Ezquerra - Aug. 30, 2014, 3:09 p.m.
# HG changeset patch
# User Angel Ezquerra <angel.ezquerra@gmail.com>
# Date 1409411084 -7200
#      Sat Aug 30 17:04:44 2014 +0200
# Node ID 2d92ae26a1e326bb6210d429f1996989c5ce01e0
# Parent  7e317616adbe47d79fbf2243711b68f97d941f29
addremove: add --subrepos flag

This new option makes mercurial recursively run addremove on every mercurial
subrepository. Just as with commit --subrepos --adremove non mercurial
subrepositories are skipped.
Pierre-Yves David - Sept. 2, 2014, 7:11 p.m.
On 08/30/2014 05:09 PM, Angel Ezquerra wrote:
> # HG changeset patch
> # User Angel Ezquerra <angel.ezquerra@gmail.com>
> # Date 1409411084 -7200
> #      Sat Aug 30 17:04:44 2014 +0200
> # Node ID 2d92ae26a1e326bb6210d429f1996989c5ce01e0
> # Parent  7e317616adbe47d79fbf2243711b68f97d941f29
> addremove: add --subrepos flag
>
> This new option makes mercurial recursively run addremove on every mercurial
> subrepository. Just as with commit --subrepos --adremove non mercurial
> subrepositories are skipped.

This will seems obviously right and will get accepted when its parent 
actually skip instead of aborting with drums and trumpets.

>
> diff --git a/mercurial/commands.py b/mercurial/commands.py
> old mode 100644
> new mode 100755
> --- a/mercurial/commands.py
> +++ b/mercurial/commands.py
> @@ -196,7 +196,7 @@
>       return rejected and 1 or 0
>
>   @command('addremove',
> -    similarityopts + walkopts + dryrunopts,
> +    similarityopts + walkopts + subrepoopts + dryrunopts,
>       _('[OPTION]... [FILE]...'),
>       inferrepo=True)
>   def addremove(ui, repo, *pats, **opts):
> diff --git a/tests/test-subrepo-recursion.t b/tests/test-subrepo-recursion.t
> --- a/tests/test-subrepo-recursion.t
> +++ b/tests/test-subrepo-recursion.t
> @@ -28,6 +28,15 @@
>     adding x.txt
>     adding foo/y.txt (glob)
>
> +Test addremove with subrepos
> +
> +  $ echo z2 > foo/bar/z2.txt
> +  $ hg addremove -S
> +  adding foo/bar/z2.txt
> +  $ rm foo/bar/z2.txt
> +  $ hg addremove -S
> +  removing foo/bar/z2.txt
> +
>   Test recursive status without committing anything:
>
>     $ hg status -S
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel
>
Angel Ezquerra - Sept. 3, 2014, 8:22 p.m.
El 02/09/2014 21:11, "Pierre-Yves David" <pierre-yves.david@ens-lyon.org>
escribió:
>
>
>
> On 08/30/2014 05:09 PM, Angel Ezquerra wrote:
>>
>> # HG changeset patch
>> # User Angel Ezquerra <angel.ezquerra@gmail.com>
>> # Date 1409411084 -7200
>> #      Sat Aug 30 17:04:44 2014 +0200
>> # Node ID 2d92ae26a1e326bb6210d429f1996989c5ce01e0
>> # Parent  7e317616adbe47d79fbf2243711b68f97d941f29
>> addremove: add --subrepos flag
>>
>> This new option makes mercurial recursively run addremove on every
mercurial
>> subrepository. Just as with commit --subrepos --adremove non mercurial
>> subrepositories are skipped.
>
>
> This will seems obviously right and will get accepted when its parent
actually skip instead of aborting with drums and trumpets.
>
>
>>
>> diff --git a/mercurial/commands.py b/mercurial/commands.py
>> old mode 100644
>> new mode 100755
>> --- a/mercurial/commands.py
>> +++ b/mercurial/commands.py
>> @@ -196,7 +196,7 @@
>>       return rejected and 1 or 0
>>
>>   @command('addremove',
>> -    similarityopts + walkopts + dryrunopts,
>> +    similarityopts + walkopts + subrepoopts + dryrunopts,
>>       _('[OPTION]... [FILE]...'),
>>       inferrepo=True)
>>   def addremove(ui, repo, *pats, **opts):
>> diff --git a/tests/test-subrepo-recursion.t
b/tests/test-subrepo-recursion.t
>> --- a/tests/test-subrepo-recursion.t
>> +++ b/tests/test-subrepo-recursion.t
>> @@ -28,6 +28,15 @@
>>     adding x.txt
>>     adding foo/y.txt (glob)
>>
>> +Test addremove with subrepos
>> +
>> +  $ echo z2 > foo/bar/z2.txt
>> +  $ hg addremove -S
>> +  adding foo/bar/z2.txt
>> +  $ rm foo/bar/z2.txt
>> +  $ hg addremove -S
>> +  removing foo/bar/z2.txt
>> +
>>   Test recursive status without committing anything:
>>
>>     $ hg status -S

I discussed this with Kevin during the sprint and he suggested that it
might be best to abort rather than skipping subrepos of unsupported types
(which was what I had done initially, hence the mismatch between what the
commit messages in this patch series say and what the actual patches do).

I believe his reasoning was that it would still be possible to revert back
to the original behavior by not using the --subrepos flag. I don't have a
strong opinion either way (I can see good points for both alternatives).
What do you guys think would be best?

Cheers,

Angel
Pierre-Yves David - Sept. 9, 2014, 9:18 a.m.
On 09/03/2014 09:22 PM, Angel Ezquerra wrote:
>
> El 02/09/2014 21:11, "Pierre-Yves David" <pierre-yves.david@ens-lyon.org
> <mailto:pierre-yves.david@ens-lyon.org>> escribió:
>  >
>  >
>  >
>  > On 08/30/2014 05:09 PM, Angel Ezquerra wrote:
>  >>
>  >> # HG changeset patch
>  >> # User Angel Ezquerra <angel.ezquerra@gmail.com
> <mailto:angel.ezquerra@gmail.com>>
>  >> # Date 1409411084 -7200
>  >> #      Sat Aug 30 17:04:44 2014 +0200
>  >> # Node ID 2d92ae26a1e326bb6210d429f1996989c5ce01e0
>  >> # Parent  7e317616adbe47d79fbf2243711b68f97d941f29
>  >> addremove: add --subrepos flag
>  >>
>  >> This new option makes mercurial recursively run addremove on every
> mercurial
>  >> subrepository. Just as with commit --subrepos --adremove non mercurial
>  >> subrepositories are skipped.
>  >
>  >
>  > This will seems obviously right and will get accepted when its parent
> actually skip instead of aborting with drums and trumpets.
>  >
>  >
>  >>
>  >> diff --git a/mercurial/commands.py b/mercurial/commands.py
>  >> old mode 100644
>  >> new mode 100755
>  >> --- a/mercurial/commands.py
>  >> +++ b/mercurial/commands.py
>  >> @@ -196,7 +196,7 @@
>  >>       return rejected and 1 or 0
>  >>
>  >>   @command('addremove',
>  >> -    similarityopts + walkopts + dryrunopts,
>  >> +    similarityopts + walkopts + subrepoopts + dryrunopts,
>  >>       _('[OPTION]... [FILE]...'),
>  >>       inferrepo=True)
>  >>   def addremove(ui, repo, *pats, **opts):
>  >> diff --git a/tests/test-subrepo-recursion.t
> b/tests/test-subrepo-recursion.t
>  >> --- a/tests/test-subrepo-recursion.t
>  >> +++ b/tests/test-subrepo-recursion.t
>  >> @@ -28,6 +28,15 @@
>  >>     adding x.txt
>  >>     adding foo/y.txt (glob)
>  >>
>  >> +Test addremove with subrepos
>  >> +
>  >> +  $ echo z2 > foo/bar/z2.txt
>  >> +  $ hg addremove -S
>  >> +  adding foo/bar/z2.txt
>  >> +  $ rm foo/bar/z2.txt
>  >> +  $ hg addremove -S
>  >> +  removing foo/bar/z2.txt
>  >> +
>  >>   Test recursive status without committing anything:
>  >>
>  >>     $ hg status -S
>
> I discussed this with Kevin during the sprint and he suggested that it
> might be best to abort rather than skipping subrepos of unsupported
> types (which was what I had done initially, hence the mismatch between
> what the commit messages in this patch series say and what the actual
> patches do).

I would be sad if haveing and svn or git repo would void all possibility 
to do addremove on subrepository. I believe a warning is better.

I CCed Kevin so he can defend his original statement.

Patch

diff --git a/mercurial/commands.py b/mercurial/commands.py
old mode 100644
new mode 100755
--- a/mercurial/commands.py
+++ b/mercurial/commands.py
@@ -196,7 +196,7 @@ 
     return rejected and 1 or 0
 
 @command('addremove',
-    similarityopts + walkopts + dryrunopts,
+    similarityopts + walkopts + subrepoopts + dryrunopts,
     _('[OPTION]... [FILE]...'),
     inferrepo=True)
 def addremove(ui, repo, *pats, **opts):
diff --git a/tests/test-subrepo-recursion.t b/tests/test-subrepo-recursion.t
--- a/tests/test-subrepo-recursion.t
+++ b/tests/test-subrepo-recursion.t
@@ -28,6 +28,15 @@ 
   adding x.txt
   adding foo/y.txt (glob)
 
+Test addremove with subrepos
+
+  $ echo z2 > foo/bar/z2.txt
+  $ hg addremove -S
+  adding foo/bar/z2.txt
+  $ rm foo/bar/z2.txt
+  $ hg addremove -S
+  removing foo/bar/z2.txt
+
 Test recursive status without committing anything:
 
   $ hg status -S