[PATCH] convert: svn-sink: test if execute bit was actually stored

Maxim Dounin mdounin at mdounin.ru
Thu Jan 3 18:12:22 CST 2008


Hello!

On Wed, Jan 02, 2008 at 10:39:04PM +0100, Patrick M?zard wrote:
>Maxim Dounin a ?crit :
>> On Thu, Dec 27, 2007 at 01:23:08PM -0600, Matt Mackall wrote:
>>> It'd be better if this intelligence were moved into the filectx object.
>> 
>> I agree that this have to be in something more generic.
>> 
>> As for filectx, currently it have to be fixed to preserve original
>> changeset revision for this logic to work. Attached patch
>> "patch-context.txt" does the trick.
>> Note: This change will broke old behaviour that
>> changectx.filectx(file).rev() report last revision where file was
>> modified. I hope nothing in code relies on this (and some cases was
>> already broken anyway).
>
>Shouldn't we add a filectx.linkrev() too ? I was bitten many 
>times by the rev/linkrev ambiguity.

Agreed. I haven't seen linkrev() havily used, but it looks like it 
should be in filectx at least for completeness. Additionally, 
currently legal method of accessing linkrev - 
filectx.filelog().linkrev(filectx.filenode()) - doesn't look very 
natural for me.

>> The only test that fails due to this change is test-issue672. As far as
>> I see it's minor and completely safe debug output change in filectx
>> priting, so I've updated test-issue672.out.
>> Second patch, patch-rename-2.txt, adds function renamedhere() to filectx
>> object and uses it to fix original problem. Notes:
>
>> diff -r 7471976f1ffd -r 94e498fd21e8 mercurial/context.py
>> --- a/mercurial/context.py	Sat Dec 29 16:57:43 2007 +0300
>> +++ b/mercurial/context.py	Sat Dec 29 17:11:48 2007 +0300
>> @@ -250,6 +250,31 @@ class filectx(object):
>>      def size(self): return self._filelog.size(self._filerev)
>>  
>>      def cmp(self, text): return self._filelog.cmp(self._filenode, text)
>
>Why not regroup it with rename() and document the latter too, 
>highlighting the differences ?

Mainly because I'm still not sure if renamedhere() should survive 
as is or it should replace renamed().

Currently it looks like most (if not all) users of filectx.renamed() 
expect it to behave as renamedhere(). I'll try to dig into this further 
as soon as I have some time for it.

>Both patches look good to me.

Thank you for review.

Maxim Dounin


More information about the Mercurial-devel mailing list