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

[Martin Geisler]

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?

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 :-)

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

-- 
Martin Geisler

aragost Trifork
Professional Mercurial support
http://aragost.com/mercurial/