[PATCH 2 of 4] keyword: don't delete possible wrappers for commictx() (see issue2254)

Dan Villiom Podlaski Christiansen danchr at gmail.com
Fri Jul 16 03:41:28 CDT 2010


On 14 Jul 2010, at 03:33, Christian Ebert wrote:

> I'm not up to the major act to install svn python bindings, so I
> can't try out hgsubversion.

I understand that :)

> For the convert extension commitctx is called only twice, and
> only from within convert/hg.py, in the context of tags and an
> "octopus" merge if I am not mistaken.

I think you misread the source code. If you look at the loop in question[1], the message related to octopus merges is set *after* commitctx() is called. That is, commitctx() is used to commit all converted revisions to the repository.

> So far, whenever I used hg convert, the working directory was
> never touched or updated. Are there use cases when this happens?
> If not, the call to keyword's commitctx would be noticed by a
> change in the working directory. For some time, albeit a while
> ago, I converted the vim cvs repo regularly with keyword
> expansion enable for all files. There was never any write action
> to the working directory.

Indeed, the convert extension never updates the working directory during its ‘convert’ command. Blacklisting that command would prevent keyword from affecting its conversion. Such a solution, however, would not work for neither hgsubversion nor hg-git. They do their work during the ‘pull’ command, which — thanks to the wonders of the -u/--update option — can affect the working directory. For this reason, blacklisting — or even whitelisting — is not a sufficient solution.

> Note that my failure to exactly grasp the problem shouldn't
> prevent you in any way from going forward with your patch.
> Ideally I'd like to create a test case where there is breakage
> without the (double)wrapper. I'd already like that for the
> current state (kwcommitctx instead of simply overriding
> commitctx).

Thanks :) As I mentioned earlier, I tried to make the EOL extension affect a conversion, but failed. The shortcoming that Martin identified with the current double-wrapper was that it would remove any wrapping of commitctx() done using wrapfunction(). I'm not aware of any extension doing so, so I don't know how to test it :/

We were originally made aware of this shortcoming through the EOL extension. hgsubversion raises an IOError if the data callback is called on a deleted file. I don't know why the EOL extension did this, but from my perspective, that's secondary to the fact that it just shouldn't be intercepting, processing and validating our commitctx() calls.

[1] <http://hg-scm.org/hg/hg/file/tip/hgext/convert/hg.py#l172>

--

Dan Villiom Podlaski Christiansen
danchr at gmail.com

-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/pkcs7-signature
Size: 1943 bytes
Desc: not available
URL: <http://selenic.com/pipermail/mercurial-devel/attachments/20100716/4a39d07e/attachment.bin>


More information about the Mercurial-devel mailing list