Cabal revision online form silently converts file to Windows line endings, breaking patches

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
7 messages Options
Reply | Threaded
Open this post in threaded view
|

Cabal revision online form silently converts file to Windows line endings, breaking patches

Niklas Hambüchen
I noticed that if you use Hackage's online Cabal file editor, it changes all your \n line endings to \r\n.

This breaks all patches written against upstream .cabal files.

For example, if I write a patch against `cachix-0.2.0` git repo's .cabal file, I can no longer apply it to whatever revisions of it Hackage has, even if there's no real conflict.

Here's a reproduction (I changed only 1 line in the editor, and it shows the whole file as changed):

    diff -u <(curl https://hackage.haskell.org/package/AesonBson-0.2.0/revision/0.cabal) <(curl https://hackage.haskell.org/package/AesonBson-0.2.0/revision/1.cabal)

I found this especially problematic for nixpkgs, where patching packages is a common practice.
_______________________________________________
Haskell-Cafe mailing list
To (un)subscribe, modify options or view archives go to:
http://mail.haskell.org/cgi-bin/mailman/listinfo/haskell-cafe
Only members subscribed via the mailman list are allowed to post.
Reply | Threaded
Open this post in threaded view
|

Re: Cabal revision online form silently converts file to Windows line endings, breaking patches

Dan Burton
Two thoughts:
1. this is annoying and I wish it didn't do this
2. one workaround would be to always fetch v0 of the patch, and maintain/generate custom versions of the patches equivalent to each revision as needed.

-- Dan Burton


On Sun, May 12, 2019 at 7:22 PM Niklas Hambüchen <[hidden email]> wrote:
I noticed that if you use Hackage's online Cabal file editor, it changes all your \n line endings to \r\n.

This breaks all patches written against upstream .cabal files.

For example, if I write a patch against `cachix-0.2.0` git repo's .cabal file, I can no longer apply it to whatever revisions of it Hackage has, even if there's no real conflict.

Here's a reproduction (I changed only 1 line in the editor, and it shows the whole file as changed):

    diff -u <(curl https://hackage.haskell.org/package/AesonBson-0.2.0/revision/0.cabal) <(curl https://hackage.haskell.org/package/AesonBson-0.2.0/revision/1.cabal)

I found this especially problematic for nixpkgs, where patching packages is a common practice.
_______________________________________________
Haskell-Cafe mailing list
To (un)subscribe, modify options or view archives go to:
http://mail.haskell.org/cgi-bin/mailman/listinfo/haskell-cafe
Only members subscribed via the mailman list are allowed to post.

_______________________________________________
Haskell-Cafe mailing list
To (un)subscribe, modify options or view archives go to:
http://mail.haskell.org/cgi-bin/mailman/listinfo/haskell-cafe
Only members subscribed via the mailman list are allowed to post.
Reply | Threaded
Open this post in threaded view
|

Re: Cabal revision online form silently converts file to Windows line endings, breaking patches

Joachim Durchholz
Actually this is much, much worse than annoying: All attempts to
checksum or diff files modified in this way will have to take line-end
differences into account, which will be a neverending source of bugs,
and as every unexpected behaviour may make people miss a security issue,
it's also a source of exploits (likely to become more interesting as
Haskell gets into high-value niches).

Just my 2 cents from the sideline :-)

Am 13.05.19 um 16:29 schrieb Dan Burton:

> Two thoughts:
> 1. this is annoying and I wish it didn't do this
> 2. one workaround would be to always fetch v0 of the patch, and
> maintain/generate custom versions of the patches equivalent to each
> revision as needed.
>
> -- Dan Burton
>
>
> On Sun, May 12, 2019 at 7:22 PM Niklas Hambüchen <[hidden email]
> <mailto:[hidden email]>> wrote:
>
>     I noticed that if you use Hackage's online Cabal file editor, it
>     changes all your \n line endings to \r\n.
>
>     This breaks all patches written against upstream .cabal files.
>
>     For example, if I write a patch against `cachix-0.2.0` git repo's
>     .cabal file, I can no longer apply it to whatever revisions of it
>     Hackage has, even if there's no real conflict.
>
>     Here's a reproduction (I changed only 1 line in the editor, and it
>     shows the whole file as changed):
>
>          diff -u <(curl
>     https://hackage.haskell.org/package/AesonBson-0.2.0/revision/0.cabal) <(curl
>     https://hackage.haskell.org/package/AesonBson-0.2.0/revision/1.cabal)
>
>     I found this especially problematic for nixpkgs, where patching
>     packages is a common practice.
>     _______________________________________________
>     Haskell-Cafe mailing list
>     To (un)subscribe, modify options or view archives go to:
>     http://mail.haskell.org/cgi-bin/mailman/listinfo/haskell-cafe
>     Only members subscribed via the mailman list are allowed to post.
>
>
> _______________________________________________
> Haskell-Cafe mailing list
> To (un)subscribe, modify options or view archives go to:
> http://mail.haskell.org/cgi-bin/mailman/listinfo/haskell-cafe
> Only members subscribed via the mailman list are allowed to post.
>

_______________________________________________
Haskell-Cafe mailing list
To (un)subscribe, modify options or view archives go to:
http://mail.haskell.org/cgi-bin/mailman/listinfo/haskell-cafe
Only members subscribed via the mailman list are allowed to post.
Reply | Threaded
Open this post in threaded view
|

Re: Cabal revision online form silently converts file to Windows line endings, breaking patches

Boris-4
Hi everyone,

Not to contribute to the solution of the problem itself, but to help with mitigation of annoying symptoms.

The diff program has an option to --ignore-trailing-space (-Z for short) that will totally ignore changes of `\n` to `\n\r` and vice versa. And so you will see only 'actual' changes.

Another generally useful option is --ignore-space-change (-b for short) that will ignore indentation changes. Another option that you may use is --ignore-all-space.

The good thing, git diff also understand these options.

Having less clutter when viewing diff is nice, but sometimes it prevents clean merges/rebases. For such cases you can pass a strategy to the merge command. For example,

  git merge -Xignore-trailing-space pr/xyz
  git merge -Xignore-all-space pr/xyz

This allows you to merge a branch without the need to fight the line ending changes. Note that in some cases it doesn't work as good as you would expect it to work, but from my experience these options help a lot.

P. S. more information can be found in man diff, man git-diff and man git-merge.

Cheers,
[hidden email]

‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐
On Monday, May 13, 2019 7:21 PM, Joachim Durchholz <[hidden email]> wrote:

> Actually this is much, much worse than annoying: All attempts to
> checksum or diff files modified in this way will have to take line-end
> differences into account, which will be a neverending source of bugs,
> and as every unexpected behaviour may make people miss a security issue,
> it's also a source of exploits (likely to become more interesting as
> Haskell gets into high-value niches).
>
> Just my 2 cents from the sideline :-)
>
> Am 13.05.19 um 16:29 schrieb Dan Burton:
>
> > Two thoughts:
> >
> > 1.  this is annoying and I wish it didn't do this
> > 2.  one workaround would be to always fetch v0 of the patch, and
> >     maintain/generate custom versions of the patches equivalent to each
> >     revision as needed.
> >
> >
> > -- Dan Burton
> > On Sun, May 12, 2019 at 7:22 PM Niklas Hambüchen <[hidden email]
> > mailto:[hidden email]> wrote:
> >
> >     I noticed that if you use Hackage's online Cabal file editor, it
> >     changes all your \\n line endings to \\r\\n.
> >
> >     This breaks all patches written against upstream .cabal files.
> >
> >     For example, if I write a patch against `cachix-0.2.0` git repo's
> >     .cabal file, I can no longer apply it to whatever revisions of it
> >     Hackage has, even if there's no real conflict.
> >
> >     Here's a reproduction (I changed only 1 line in the editor, and it
> >     shows the whole file as changed):
> >
> >          diff -u <(curl
> >     https://hackage.haskell.org/package/AesonBson-0.2.0/revision/0.cabal) <(curl
> >     https://hackage.haskell.org/package/AesonBson-0.2.0/revision/1.cabal)
> >
> >     I found this especially problematic for nixpkgs, where patching
> >     packages is a common practice.
> >     _______________________________________________
> >     Haskell-Cafe mailing list
> >     To (un)subscribe, modify options or view archives go to:
> >     http://mail.haskell.org/cgi-bin/mailman/listinfo/haskell-cafe
> >     Only members subscribed via the mailman list are allowed to post.
> >
> >
> > Haskell-Cafe mailing list
> > To (un)subscribe, modify options or view archives go to:
> > http://mail.haskell.org/cgi-bin/mailman/listinfo/haskell-cafe
> > Only members subscribed via the mailman list are allowed to post.
>
> Haskell-Cafe mailing list
> To (un)subscribe, modify options or view archives go to:
> http://mail.haskell.org/cgi-bin/mailman/listinfo/haskell-cafe
> Only members subscribed via the mailman list are allowed to post.


_______________________________________________
Haskell-Cafe mailing list
To (un)subscribe, modify options or view archives go to:
http://mail.haskell.org/cgi-bin/mailman/listinfo/haskell-cafe
Only members subscribed via the mailman list are allowed to post.
Reply | Threaded
Open this post in threaded view
|

Re: Cabal revision online form silently converts file to Windows line endings, breaking patches

Joachim Durchholz
The options for ignoring spaces are a technical solution, but they do
not solve the social problem: That everybody dealing with the diffs
needs to remember to apply them.

They aren't a full solution either. --ignore-trailing-space will also
suppress trailing blanks.
This is exactly the kind of 99% solution that makes systems less stable:
have a dozen or so 99%-reliable points in the system, and the overall
system will fail constantly, every time for a different reason.

The options are still valuable as a stopgap solution, of course!

Am 14.05.19 um 08:19 schrieb Boris:

> Hi everyone,
>
> Not to contribute to the solution of the problem itself, but to help with mitigation of annoying symptoms.
>
> The diff program has an option to --ignore-trailing-space (-Z for short) that will totally ignore changes of `\n` to `\n\r` and vice versa. And so you will see only 'actual' changes.
>
> Another generally useful option is --ignore-space-change (-b for short) that will ignore indentation changes. Another option that you may use is --ignore-all-space.
>
> The good thing, git diff also understand these options.
>
> Having less clutter when viewing diff is nice, but sometimes it prevents clean merges/rebases. For such cases you can pass a strategy to the merge command. For example,
>
>    git merge -Xignore-trailing-space pr/xyz
>    git merge -Xignore-all-space pr/xyz
>
> This allows you to merge a branch without the need to fight the line ending changes. Note that in some cases it doesn't work as good as you would expect it to work, but from my experience these options help a lot.
>
> P. S. more information can be found in man diff, man git-diff and man git-merge.
>
> Cheers,
> [hidden email]
>
> ‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐
> On Monday, May 13, 2019 7:21 PM, Joachim Durchholz <[hidden email]> wrote:
>
>> Actually this is much, much worse than annoying: All attempts to
>> checksum or diff files modified in this way will have to take line-end
>> differences into account, which will be a neverending source of bugs,
>> and as every unexpected behaviour may make people miss a security issue,
>> it's also a source of exploits (likely to become more interesting as
>> Haskell gets into high-value niches).
>>
>> Just my 2 cents from the sideline :-)
>>
>> Am 13.05.19 um 16:29 schrieb Dan Burton:
>>
>>> Two thoughts:
>>>
>>> 1.  this is annoying and I wish it didn't do this
>>> 2.  one workaround would be to always fetch v0 of the patch, and
>>>      maintain/generate custom versions of the patches equivalent to each
>>>      revision as needed.
>>>
>>>
>>> -- Dan Burton
>>> On Sun, May 12, 2019 at 7:22 PM Niklas Hambüchen <[hidden email]
>>> mailto:[hidden email]> wrote:
>>>
>>>      I noticed that if you use Hackage's online Cabal file editor, it
>>>      changes all your \\n line endings to \\r\\n.
>>>
>>>      This breaks all patches written against upstream .cabal files.
>>>
>>>      For example, if I write a patch against `cachix-0.2.0` git repo's
>>>      .cabal file, I can no longer apply it to whatever revisions of it
>>>      Hackage has, even if there's no real conflict.
>>>
>>>      Here's a reproduction (I changed only 1 line in the editor, and it
>>>      shows the whole file as changed):
>>>
>>>           diff -u <(curl
>>>      https://hackage.haskell.org/package/AesonBson-0.2.0/revision/0.cabal) <(curl
>>>      https://hackage.haskell.org/package/AesonBson-0.2.0/revision/1.cabal)
>>>
>>>      I found this especially problematic for nixpkgs, where patching
>>>      packages is a common practice.
>>>      _______________________________________________
>>>      Haskell-Cafe mailing list
>>>      To (un)subscribe, modify options or view archives go to:
>>>      http://mail.haskell.org/cgi-bin/mailman/listinfo/haskell-cafe
>>>      Only members subscribed via the mailman list are allowed to post.
>>>
>>>
>>> Haskell-Cafe mailing list
>>> To (un)subscribe, modify options or view archives go to:
>>> http://mail.haskell.org/cgi-bin/mailman/listinfo/haskell-cafe
>>> Only members subscribed via the mailman list are allowed to post.
>>
>> Haskell-Cafe mailing list
>> To (un)subscribe, modify options or view archives go to:
>> http://mail.haskell.org/cgi-bin/mailman/listinfo/haskell-cafe
>> Only members subscribed via the mailman list are allowed to post.
>
>
>

_______________________________________________
Haskell-Cafe mailing list
To (un)subscribe, modify options or view archives go to:
http://mail.haskell.org/cgi-bin/mailman/listinfo/haskell-cafe
Only members subscribed via the mailman list are allowed to post.
Reply | Threaded
Open this post in threaded view
|

Re: Cabal revision online form silently converts file to Windows line endings, breaking patches

Peter Simons-2
In reply to this post by Boris-4
Hi guys,

 > The diff program has an option to --ignore-trailing-space
 > (-Z for short) that will totally ignore changes of `\n`
 > to `\n\r` and vice versa. And so you will see only
 > 'actual' changes.

more importantly, patch(1) has an option --ignore-whitespace
that helps applying patches to a revised Cabal file which
suddenly comes along with CRLF line endings. Using that flag
with diff(1) when creating the patch is a good idea, but it
won't make the patch resistant against failures when it's
applied to a modified Cabal file.

 > Another generally useful option is --ignore-space-change
 > (-b for short) that will ignore indentation changes.
 > Another option that you may use is --ignore-all-space.

This option is actually somewhat dangerous when applied in
cases where the underlying source relies on layout to convey
structure, because changes in indention will be lost with
that flag enabled.

Generally speaking, Hackage's policy of adding CRLFs to
edited files seems like a bad idea. The least intrusive
choice would be to follow whatever line-ending convention
the original Cabal file has and to adhere to that.

Anyway, I'll throw in a historical footnote for good
measure: [1]

Best regards,
Peter


[1] https://github.com/commercialhaskell/all-cabal-files/issues/11

_______________________________________________
Haskell-Cafe mailing list
To (un)subscribe, modify options or view archives go to:
http://mail.haskell.org/cgi-bin/mailman/listinfo/haskell-cafe
Only members subscribed via the mailman list are allowed to post.
Reply | Threaded
Open this post in threaded view
|

Re: Cabal revision online form silently converts file to Windows line endings, breaking patches

Brandon Allbery
The main problem being that if they're using basic web upload for the file, it's using network CRLF.

On Tue, May 14, 2019 at 7:05 AM Peter Simons <[hidden email]> wrote:
Hi guys,

 > The diff program has an option to --ignore-trailing-space
 > (-Z for short) that will totally ignore changes of `\n`
 > to `\n\r` and vice versa. And so you will see only
 > 'actual' changes.

more importantly, patch(1) has an option --ignore-whitespace
that helps applying patches to a revised Cabal file which
suddenly comes along with CRLF line endings. Using that flag
with diff(1) when creating the patch is a good idea, but it
won't make the patch resistant against failures when it's
applied to a modified Cabal file.

 > Another generally useful option is --ignore-space-change
 > (-b for short) that will ignore indentation changes.
 > Another option that you may use is --ignore-all-space.

This option is actually somewhat dangerous when applied in
cases where the underlying source relies on layout to convey
structure, because changes in indention will be lost with
that flag enabled.

Generally speaking, Hackage's policy of adding CRLFs to
edited files seems like a bad idea. The least intrusive
choice would be to follow whatever line-ending convention
the original Cabal file has and to adhere to that.

Anyway, I'll throw in a historical footnote for good
measure: [1]

Best regards,
Peter


[1] https://github.com/commercialhaskell/all-cabal-files/issues/11

_______________________________________________
Haskell-Cafe mailing list
To (un)subscribe, modify options or view archives go to:
http://mail.haskell.org/cgi-bin/mailman/listinfo/haskell-cafe
Only members subscribed via the mailman list are allowed to post.


--
brandon s allbery kf8nh

_______________________________________________
Haskell-Cafe mailing list
To (un)subscribe, modify options or view archives go to:
http://mail.haskell.org/cgi-bin/mailman/listinfo/haskell-cafe
Only members subscribed via the mailman list are allowed to post.