RFR: Fix start of hunk range off-by-one error when parsing range string

[JornVernee via github.com]

On Mon, 5 Aug 2019 14:32:33 GMT, Erik Duveblad via github.com <duke at openjdk.java.net> wrote:

> On Thu, 1 Aug 2019 19:14:28 GMT, JornVernee via github.com <duke at openjdk.java.net> wrote:
> 
>> I kept running into off-by-one errors in hunk headers when exporting a patch from a git repo, and then importing into a mercurial repo (for pushing, since git repos are currently read-only)
>> 
>> The problem seems to be that unified diffs report a line number 1 before the modified line if the line count is 0. For instance, if I just remove a few lines from a file, the hunk destination start line is one before the first deleted line. (similarly when only adding lines).
>> 
>> The problem is that when hunks are then coalesced together, the starting line, and computed line counts are off-by-one as well, and as a result, the generated patch fails to apply.
>> 
>> I noticed there was already some code dealing with this in HunkCoalescer, but this wasn't catching all the cases. I first made a version that patched the starting line in the missing cases, but it seemed cleaner to try and fix this at the source, i.e. when parsing the Range string from the unified diff. So, that is what I've done in this PR, as well as removing the previous patching code in HunkCoalescer, which was no longer needed.
>> 
>> If wanted I could also go the other route of patching the starting line in HunkCoalescer in the missing cases (there were 4 of these).
>> 
>> ----------------
>> 
>> Commits:
>>  - 7a093687:	Fixed some off-by-one errors caused by unified diff indicating the 'wrong' line when no lines were changes (in source or target)
>> 
>> Pull request:
>> http://git.openjdk.java.net/skara/pull/34
>> 
>> Webrev:
>> https://openjdk.github.io/cr/skara/34/webrev.00
>> 
>> Patch:
>> http://git.openjdk.java.net/skara/pull/34.diff
>> 
>> Fetch command:
>> git fetch https://github.com/openjdk/skara.git pull/34/head:pull/34
> 
> Wow, thanks for digging into this! This is indeed a bug in git, as you probably noticed in my comments. There is also another underflow bug in git when generating combined diffs with zero context (my comment and workaround is right above your fix).
> 
> I agree with fixing this within the `vcs` module, the `webrev` module should not have to know about this issue.
> 
> Regarding your little tool, would you want to upstream it? I was thinking something like
> 
> ```bash
> $ git transplant --target /path/to/hg/repo <commit>
> ```
> (loosely based on `hg transplant`, but with a `--target` flag instead of `--source` flag)
> 
> PR: http://git.openjdk.java.net/skara/pull/34

> Regarding your little tool, would you want to upstream it? I was thinking something like
> 
> ```shell
> $ git transplant --target /path/to/hg/repo <commit>
> ```
> 
> (loosely based on `hg transplant`, but with a `--target` flag instead of `--source` flag)

Not using a tool for that currently, just exporting a webrev (also used in the review), and then importing the generated `.patch` file with `qimport` & `qpush` in the hg repo.

Maybe having a tool would be nice though...

PR: http://git.openjdk.java.net/skara/pull/34