RFR: Coalesce hunks that have overlapping context

Erik Duveblad via github.com duke at openjdk.java.net
Tue Sep 3 08:48:00 UTC 2019


On Mon, 2 Sep 2019 14:58:33 GMT, Jorn Vernee via github.com <duke at openjdk.java.net> wrote:

> On Fri, 30 Aug 2019 13:56:52 GMT, Jorn Vernee via github.com <duke at openjdk.java.net> wrote:
> 
>> The `webrev` command can generate patch files with adjacent hunks that have overlapping context. `git apply` rejects these patches.
>> 
>> This PR coalesces adjacent hunks with overlapping context, which is also what's done by `git diff`.
>> 
>> Also added a test for `HunkCoalescer` (since it's a pretty complex class I think it deserved it's own tests).
>> 
>> Resulting webrev applies with both git and hg.
>> 
>> ----------------
>> 
>> Commits:
>>  - 6f5f1071:	Coalesce hunks that have overlapping context, since git rejects these otherwise
>> 
>> Pull request:
>> https://git.openjdk.java.net/skara/pull/113
>> 
>> Webrev:
>> https://webrevs.openjdk.java.net/skara/113/webrev.00
>> 
>> Patch:
>> https://git.openjdk.java.net/skara/pull/113.diff
>> 
>> Fetch command:
>> git fetch https://git.openjdk.java.net/skara pull/113/head:pull/113
> 
> @edvbld I tested with the following script:
> 
> ```powershell
> for ($i = 1; $i -le 20; $i++) {
> 	git webrev -r "HEAD~$i" -N
> 	git checkout "HEAD~$i"
> 	git apply .\webrev\git-jdk2.patch
> 	git checkout master --force
> }
> ```
> This went alright until about `HEAD~16`, which threw a 'corrupt patch' error. Looking into it, the offending line is:
> 
>     diff a/test/hotspot/jtreg/ProblemList-aot.txt b/test/hotspot/jtreg/ProblemList-aot.txt
> 
> I thought it might be the hyphen in the name, but `git diff` generates the same, and that patch does apply cleanly. Even when copying over the line from the git diff to the patch file generated by `webrev` it still complains. Not sure what's going on...
> 
> However, running that script without this PR fails on the first webrev, so I'd say that is a net improvement :), and since the issue seems unrelated, I think we could still move ahead with this PR?
> 
> PR: https://git.openjdk.java.net/skara/pull/113

@JornVernee yeah lets integrate this one since it is an improvement at least

PR: https://git.openjdk.java.net/skara/pull/113


More information about the skara-dev mailing list