[issue2254] EOL Extension translates files during pull, which can cause coherency issues.

[Christian Ebert]

* Martin Geisler on Wednesday, July 07, 2010 at 10:06:49 +0200
> Dan Villiom Podlaski Christiansen <bugs at mercurial.selenic.com> writes:
> 
> (I need to bring this back on the mailinglist where we can discuss
> things... The tracker is no good for discussing patches.)
> 
>> Dan Villiom Podlaski Christiansen <danchr at gmail.com> added the comment:
>> 
>> Oops, please disregard that patch; I uploaded the wrong one. This is
>> the right one…
> 
> I posted a question on the tracker about this patch, but I don't think
> anybody answered it? I'll post them again below.
> 
>> # HG changeset patch
>> # Date 1277639962 -7200
>> # User Dan Villiom Podlaski Christiansen <danchr at gmail.com>
>> # Parent 6478537426e9bebc4d34c6a4c7400052e22e6112
>> eol: make repo.commit use a custom commitctx wrapper (issue2254)
>> 
>> Unconditionally wrapping commitctx() means that EOL-processing is
>> performed for *all* changeset creations; even if done as part of a
>> conversion or a pull using hgsubversion or hg-git.
>> 
>> In most cases, the bug would not be noticed, unless the converted
>> repository happened to contain a .hgeol file and the EOL extension was
>> enabled. Even if this case, EOL expansion should not occur during a
>> conversion; by design, conversions should be exact, unless the user
>> explicitely requests otherwise.
>> 
>> It should be noted, however, that the aforementioned issue was not how
>> this bug was discovered: hgsubversion will raise an IOError whenever
>> the commit callback for a deleted file is called. The EOL extension
>> did not anticipate this situation, causing pulls to fail if a revision
>> deleted any files and it was enabled.
>> 
>> The fix is similar to, and based on, a change by Christian Ebert to
>> the keyword extension in changeset 23e941d7f507.
>> 
>> diff --git a/hgext/eol.py b/hgext/eol.py
>> --- a/hgext/eol.py
>> +++ b/hgext/eol.py
>> @@ -238,7 +238,7 @@ def reposetup(ui, repo):
>>                     if wlock is not None:
>>                         wlock.release()
>> 
>> -        def commitctx(self, ctx, error=False):
>> +        def _eolcommitctx(self, ctx, error=False):
>>             for f in sorted(ctx.added() + ctx.modified()):
>>                 if not self._eolfile(f):
>>                     continue
>> @@ -252,5 +252,15 @@ def reposetup(ui, repo):
>>                     raise util.Abort(_("inconsistent newline style "
>>                                        "in %s\n" % f))
>>             return super(eolrepo, self).commitctx(ctx, error)
>> +
>> +        def commit(self, *args, **opts):
>> +            # use custom commitctx for user commands
>> +            # other extensions can still wrap repo.commitctx directly
>> +            self.commitctx = self._eolcommitctx
>> +            try:
>> +                return super(eolrepo, self).commit(*args, **opts)
>> +            finally:
>> +                del self.commitctx
>> +
>>     repo.__class__ = eolrepo
>>     repo._hgcleardirstate()
> 
> When you do
> 
>  self.commitctx = self._eolcommitctx
> 
> wont you be removing any wrapping done by other extensions? Also, why
> would you want to delete self.commitctx?

http://www.selenic.com/pipermail/mercurial-devel/2009-July/013738.html

Perhaps the whole thread is interesting:

http://www.selenic.com/pipermail/mercurial-devel/2009-July/thread.html#13605

It's all about reference cycles which makes my head cycle.

> You write that this is based on the change by Christian to the keyword
> extension in changeset 23e941d7f507, but Christian thanks you for the
> same strange change there :-)

Sounds like a reference cycle ;-)

> So that's doesn't help me much in understanding why this is a good thing
> to do.

c
-- 
theatre - books - texts - movies
Black Trash Productions at home: http://www.blacktrash.org/
Black Trash Productions on Facebook:
http://www.facebook.com/blacktrashproductions