RFR: Convert path separators in patch file to unix explicitly

Erik Duveblad via github.com duke at openjdk.java.net
Mon Aug 5 14:12:07 UTC 2019


On Mon, 5 Aug 2019 13:51:53 GMT, JornVernee via github.com <duke at openjdk.java.net> wrote:

> On Mon, 5 Aug 2019 11:34:13 GMT, JornVernee via github.com <duke at openjdk.java.net> wrote:
> 
>> On Mon, 5 Aug 2019 10:35:23 GMT, Erik Duveblad via github.com <duke at openjdk.java.net> wrote:
>> 
>>> On Thu, 1 Aug 2019 19:30:30 GMT, JornVernee via github.com <duke at openjdk.java.net> wrote:
>>> 
>>>> I ran into trouble when trying to apply a patch file generated with Windows Git to a mercurial repository through cygwin. It wasn't recognizing the files since the paths in the patch file were using Windows path separators.
>>>> 
>>>> The spec doesn't seem to mention path separators: https://www.gnu.org/software/diffutils/manual/html_node/Detailed-Unified.html#Detailed-Unified
>>>> 
>>>> But, both the Windows Git and Mercurial clients I have installed are handling unix path separators just fine. So, in this PR I've changed the patch file generators to use unix path separators explicitly, which solves the problem.
>>>> 
>>>> ----------------
>>>> 
>>>> Commits:
>>>>  - cefd6ae1:	Convert path separators in patch file to unix explicitly
>>>> 
>>>> Pull request:
>>>> http://git.openjdk.java.net/skara/pull/35
>>>> 
>>>> Webrev:
>>>> https://openjdk.github.io/cr/skara/35/webrev.00
>>>> 
>>>> Patch:
>>>> http://git.openjdk.java.net/skara/pull/35.diff
>>>> 
>>>> Fetch command:
>>>> git fetch https://github.com/openjdk/skara.git pull/35/head:pull/35
>>> 
>>> Hi Jorn,
>>> 
>>> thanks for testing out the tools using Cygwin!
>>> 
>>> The patch itself looks good, but maybe this patch should take the opportunity to fix https://bugs.openjdk.java.net/browse/SKARA-7 as well? I would prefer moving away from `Webrev` having its own logic for creating `.patch` files, it should use `Diff::write` and `Diff::toString` instead. I suspect we have the same exact issue in `Diff::write`, so please fix the bug there as well, even if you don't refactor `Webrev` to use `Diff::write` :bowing_man:
>>> 
>>> PR: http://git.openjdk.java.net/skara/pull/35
>> 
>> @edvbld I'll take a look :)
>> 
>> PR: http://git.openjdk.java.net/skara/pull/35
> 
> I've just fixed the problem for `Patch::write` as well, since switching `Webrev` to use `Diff::write` is not a simple switch. The latter does not have support for printing hunk context lines it seems. Doesn't seem like a quick fix, and I don't really have time to look into it right now :/
> 
> I ran the `vcs` module tests, but this is the one that had a lot of failures. I can say that at least there were no additional failures from the `Patch::write` change, but you might want to test on your side as well.
> 
> PR: http://git.openjdk.java.net/skara/pull/35

Ok, thanks, I will take the patch for a spin.

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


More information about the skara-dev mailing list