[PATCH] convert: svn-sink: test if execute bit was actually stored
mdounin at mdounin.ru
Mon Jan 7 20:10:32 CST 2008
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
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
> 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.
-------------- 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():
if fnode == p.filenode(name):
- return None
+ return False
More information about the Mercurial-devel