Patchwork [2,of,2] subrepo: return only the manifest keys from hgsubrepo.files()

login
register
mail settings
Submitter Matt Harbison
Date Feb. 28, 2015, 5:47 a.m.
Message ID <31179e583de4692c9668.1425102439@Envy>
Download mbox | patch
Permalink /patch/7860/
State Accepted
Headers show

Comments

Matt Harbison - Feb. 28, 2015, 5:47 a.m.
# HG changeset patch
# User Matt Harbison <matt_harbison@yahoo.com>
# Date 1424039350 18000
#      Sun Feb 15 17:29:10 2015 -0500
# Node ID 31179e583de4692c966819b4fea6f14286921b67
# Parent  d36fb8953555e4e0c7849b1f9e7b61a31e790c28
subrepo: return only the manifest keys from hgsubrepo.files()

This is in line with the other subrepo classes (i.e. only the filenames are
returned).  Archive iterates over the manifest keys, and since subrepo archive()
uses this method, it does too now.  This will allow largefiles to override its
manifest keys to present the largefiles instead of the standins.  Once in place,
we shouldn't need the copy/paste overrides of archive and subrepo archive in
largefiles, that don't quite behave like the core methods they are overriding.
Martin von Zweigbergk - Feb. 28, 2015, 5:58 a.m.
This series LGTM.

I could only find one subrepo class that returns paths, namely the
svnsubrepo (the gitsubrepo seems not to be implemented). But the new return
value is also in line with the docstring, which says "return filename
iterator".

On Fri, Feb 27, 2015 at 9:49 PM Matt Harbison <mharbison72@gmail.com> wrote:

> # HG changeset patch
> # User Matt Harbison <matt_harbison@yahoo.com>
> # Date 1424039350 18000
> #      Sun Feb 15 17:29:10 2015 -0500
> # Node ID 31179e583de4692c966819b4fea6f14286921b67
> # Parent  d36fb8953555e4e0c7849b1f9e7b61a31e790c28
> subrepo: return only the manifest keys from hgsubrepo.files()
>
> This is in line with the other subrepo classes (i.e. only the filenames are
> returned).  Archive iterates over the manifest keys, and since subrepo
> archive()
> uses this method, it does too now.  This will allow largefiles to override
> its
> manifest keys to present the largefiles instead of the standins.  Once in
> place,
> we shouldn't need the copy/paste overrides of archive and subrepo archive
> in
> largefiles, that don't quite behave like the core methods they are
> overriding.
>
> diff --git a/mercurial/subrepo.py b/mercurial/subrepo.py
> --- a/mercurial/subrepo.py
> +++ b/mercurial/subrepo.py
> @@ -836,7 +836,7 @@
>      def files(self):
>          rev = self._state[1]
>          ctx = self._repo[rev]
> -        return ctx.manifest()
> +        return ctx.manifest().keys()
>
>      def filedata(self, name):
>          rev = self._state[1]
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel
>
Matt Harbison - Feb. 28, 2015, 6:34 a.m.
On Sat, 28 Feb 2015 00:58:20 -0500, Martin von Zweigbergk  
<martinvonz@google.com> wrote:

> This series LGTM.
>
> I could only find one subrepo class that returns paths, namely the
> svnsubrepo (the gitsubrepo seems not to be implemented). But the new  
> return
> value is also in line with the docstring, which says "return filename
> iterator".

Yep.  I definitely elided 'docstring says' with 'svn implementation does'.

Thanks for the quick review.


> On Fri, Feb 27, 2015 at 9:49 PM Matt Harbison <mharbison72@gmail.com>  
> wrote:
>
>> # HG changeset patch
>> # User Matt Harbison <matt_harbison@yahoo.com>
>> # Date 1424039350 18000
>> #      Sun Feb 15 17:29:10 2015 -0500
>> # Node ID 31179e583de4692c966819b4fea6f14286921b67
>> # Parent  d36fb8953555e4e0c7849b1f9e7b61a31e790c28
>> subrepo: return only the manifest keys from hgsubrepo.files()
>>
>> This is in line with the other subrepo classes (i.e. only the filenames  
>> are
>> returned).  Archive iterates over the manifest keys, and since subrepo
>> archive()
>> uses this method, it does too now.  This will allow largefiles to  
>> override
>> its
>> manifest keys to present the largefiles instead of the standins.  Once  
>> in
>> place,
>> we shouldn't need the copy/paste overrides of archive and subrepo  
>> archive
>> in
>> largefiles, that don't quite behave like the core methods they are
>> overriding.
>>
>> diff --git a/mercurial/subrepo.py b/mercurial/subrepo.py
>> --- a/mercurial/subrepo.py
>> +++ b/mercurial/subrepo.py
>> @@ -836,7 +836,7 @@
>>      def files(self):
>>          rev = self._state[1]
>>          ctx = self._repo[rev]
>> -        return ctx.manifest()
>> +        return ctx.manifest().keys()
>>
>>      def filedata(self, name):
>>          rev = self._state[1]
>> _______________________________________________
>> Mercurial-devel mailing list
>> Mercurial-devel@selenic.com
>> http://selenic.com/mailman/listinfo/mercurial-devel
Pierre-Yves David - March 2, 2015, 8:45 p.m.
On 02/28/2015 05:58 AM, Martin von Zweigbergk wrote:
> This series LGTM.

This series is pushed to the clowncopter.

Thanks.

Patch

diff --git a/mercurial/subrepo.py b/mercurial/subrepo.py
--- a/mercurial/subrepo.py
+++ b/mercurial/subrepo.py
@@ -836,7 +836,7 @@ 
     def files(self):
         rev = self._state[1]
         ctx = self._repo[rev]
-        return ctx.manifest()
+        return ctx.manifest().keys()
 
     def filedata(self, name):
         rev = self._state[1]