Issue415

Title Mercurial integration: regression in diff generation
Priority urgent Status resolved
Superseder Nosy List eric.araujo, loewis, ncoghlan, pitrou, tshepang
Assigned To Topics

Created on 2011-08-06.14:55:12 by eric.araujo, last changed 2012-04-02.15:04:18 by loewis.

Messages
msg2207 (view) Author: eric.araujo Date: 2011-08-06.14:55:12
There seems to be a regression with diff generation: if you look at http://bugs.python.org/file22839/bc362109eed8.diff (created from the repo http://bitbucket.org/jaraco/cpython-issue12666 added to http://bugs.python.org/issue12666), you’ll find first that the diff uses the old format, not the git format, and second that the diff seems to revert some upstream changes.
msg2310 (view) Author: loewis Date: 2011-11-24.21:23:03
Using the mercurial diff format is intentional - the git format is utterly broken and must not be used.

As for changes that are reverted: what changes specifically? (or: what versions should it have compared instead of comparing 0c1c9bb590a9 and bc362109eed8?)
msg2361 (view) Author: eric.araujo Date: 2012-01-07.15:39:19
> Using the mercurial diff format is intentional - the git format is utterly broken and must not be used.
“utterly broken” is a very strong opinion.  The git diff format supports things that unified diff does not and has one drawback to my knowledge (I forgot exactly what—it was one missing piece of information that you mentioned during one of the python-dev threads about Mercurial migration or Roundup-Mercurial integration).  Everyone recommends always using git diffs.

> As for changes that are reverted: what changes specifically? (or: what versions should it have compared
> instead of comparing 0c1c9bb590a9 and bc362109eed8?)
I’m sorry I don’t know exactly.  I have this comment from the bug report:

> I'm not sure how the bugtracker patch mechanism works, but the patch it produced included a lot of changes > that I didn't make (changesets already committed to the master repo).
msg2362 (view) Author: loewis Date: 2012-01-07.18:11:25
> “utterly broken” is a very strong opinion.  The git diff format
> supports things that unified diff does not and has one drawback to my
> knowledge (I forgot exactly what—it was one missing piece of
> information that you mentioned during one of the python-dev threads
> about Mercurial migration or Roundup-Mercurial integration).
> Everyone recommends always using git diffs.

That's because nobody (except for the Rietveld integration) needs to
apply patches automatically, apparently - else people would have
noticed that the format is just unusable, for this kind of application.

Notice that this holds for Mercurial's implementation of git-style
diffs. The ones generated by git don't have this issue.

> 
>> As for changes that are reverted: what changes specifically? (or:
>> what versions should it have compared instead of comparing
>> 0c1c9bb590a9 and bc362109eed8?)
> I’m sorry I don’t know exactly.  I have this comment from the bug
> report:
> 
>> I'm not sure how the bugtracker patch mechanism works, but the
>> patch it produced included a lot of changes > that I didn't make
>> (changesets already committed to the master repo).

I think that claim is incorrect. Unless it can be demonstrated with
specific chunks that are there but ought not, I propose to close
this issue as invalid.
msg2366 (view) Author: ncoghlan Date: 2012-01-21.13:00:25
If you get repeated application of upstream changes in generated patches, it's a sign that the patch generator is having trouble finding the right base version to compare against. Usually this happens due to daisy chaining of merges, so a CPython version gets merged, but never appears directly as an ancestor of the revision that is being merge (instead, the relevant CPython version appears only as an ancestor of the alternate branch of a merge commit).

For example, I used to have this problem all the time when I was running two feature branches in my sandbox repo, where feature B depended on feature A. If I did the merge flow of upstream changes as default->A->B then the diffs generated for the B branch were always useless (they included changes already part of upstream). After much back and forth with Martin, we figured out that this was the problem and changing my merge workflow to be default->A, default->B, and only doing A->B merges when A itself changed, would allow the automatic patch generator to do the right thing.

Another way to get yourself in trouble is to merge directly from hg.python.org into a feature branch, then merge that feature branch into your default branch - again, the patch generator can't see the correct CPython revision in that case (because it is off on the feature branch, not on default).

So nothing's actually changed here, but the constraints on merge practices if you want the patch generator to produce correct patches could use a solid explanation somewhere in the devguide.
msg2421 (view) Author: loewis Date: 2012-04-02.15:04:17
Closing this report as invalid: the misgeneration of the patch has not been demonstrated.
History
Date User Action Args
2012-04-02 15:04:18loewissetstatus: chatting -> resolved
messages: + msg2421
2012-01-25 08:32:52tshepangsetnosy: + tshepang
2012-01-21 13:00:25ncoghlansetnosy: + ncoghlan
messages: + msg2366
2012-01-07 18:11:26loewissetmessages: + msg2362
2012-01-07 15:39:20eric.araujosetmessages: + msg2361
2011-11-24 21:23:04loewissetstatus: unread -> chatting
messages: + msg2310
2011-08-06 14:55:12eric.araujocreate