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

[Maxim Dounin]

Hello!

On Sun, Jan 06, 2008 at 04:20:16PM +0100, Patrick M?zard wrote:

[...]

>> 5. Changectx.filectxs() will produce filectx objects without changeset's
>> in them (I haven't find any users of this though). There are also other
>> cases where filectx may be erroneously created without changeset.

Just to note: I was wrong here, changectx.filectxs() will produce 
correct filectx objects. So basically when you are following 
changesets though changectx you are safe with recent patches.

> > 2. We should use logic when calculating renamed() when we are traveling
> > though changesets, but shouldn't when we are following filelog. Current
> > implementation in filectx introduce ambiguity, since we never know (or
> > should keep it in mind, or should test if rev == linkrev) if filectx
> > object was created from changectx and thus holds changeset, or it
> > wasn't. Example of this - Brendan's guard in filectx.annotate().
> > 
> > Probably we should consider moving this into changectx class instead,
> > and maintaining rev == linkrev invariant in filectx.
> 
> This is a breaking change and a complicated one but it sounds 
>good to me, more logical.

Actually, I see two generic ways here:

1. Filectx is a part of changectx mostly related to one particular 
file. It should be created from changeset (but may be created from 
filenode, and in this case we fallback to linkrev changeset).

2. Filectx is context for filelog's state (as changectx is context 
for whole repo state). It knows nothing about changesets except 
linkrev.

Both ways have pros and cons. And it looks like we have to 
maintain both. The current problem as I see it is that we try to 
maintain them in single class (filectx) without making it clear.

May be just documenting this clearly (and suggested workarounds, 
e.g. checking if rev() == linkrev()) will help?

>Let's start with this renamed thing, 
>here is an attempt to rollback filectx.renamed() behaviour (good 
>for third-parties code) and move the new code into changectx. 
>Requiring a filectx to be passed to changectx.renamed() may look a 
>little verbose but I don't want this operation to look as a cheap 
>one (you need a filelog here) and it allows filelog/filectx to be 
>reused.
>
> What do you think ?

There were some comments for your patch, proposed changes and so 
on, but I ripped them. I really dislike my original idea now - it 
looks like it will introduce more problems than it may resolve.

And, thanks, I've noticed that I erroneously used None in 
renamed() where False should be used. Trivial patch attached.

Maxim Dounin
-------------- next part --------------
# HG changeset patch
# User Maxim Dounin <mdounin at mdounin.ru>
# Date 1199757829 -10800
# Node ID af5da167a92046c029ce4d253183b777f5083221
# Parent  3ef279074c77c3cf3f6b35f0f73dee2fdba5aa41
context: fix filectx.renamed() to return False, not None

diff -r 3ef279074c77 -r af5da167a920 mercurial/context.py
--- a/mercurial/context.py	Sun Jan 06 15:40:32 2008 +0100
+++ b/mercurial/context.py	Tue Jan 08 05:03:49 2008 +0300
@@ -271,7 +271,7 @@ class filectx(object):
         for p in self._changectx.parents():
             try:
                 if fnode == p.filenode(name):
-                    return None
+                    return False
             except revlog.LookupError:
                 pass
         return renamed