conda-forge / conda-smithy

The tool for managing conda-forge feedstocks.
https://conda-forge.org/
BSD 3-Clause "New" or "Revised" License
152 stars 183 forks source link

Don't use binary for gitattributes for patch files #1834

Open asmeurer opened 10 months ago

asmeurer commented 10 months ago

Solution to issue cannot be found in the documentation.

Issue

The .gitattributes file uses

*.patch binary
*.diff binary

This makes it so that git diff and git show do not show the contents of patch files.

This is quite annoying as you typically have to update patch files manually, so you actually need to use git to update them.

This is evidently done to handle different line endings, but there are other ways this could be done that are less annoying:

See the discussion at https://github.com/conda-forge/emacs-feedstock/pull/80#discussion_r1458160030

Installed packages

.

Environment info

.
mbargull commented 10 months ago

https://github.com/conda-forge/emacs-feedstock/pull/80#discussion_r1458945084 :

We might be able to do either

*.path -text
*.diff -text

or

*.path binary diff
*.diff binary diff

. IDK which one would work/be preferred.

Looking at https://github.com/git/git/blob/v2.43.0/attr.c#L662 it seems we have binary defined as

[attr]binary -diff -merge -text

. So,

*.path -merge -text
*.diff -merge -text

would probably suffice to address the reported issue while still retaining the desired behavior with Git not break source line ending depending on the users' configurations.

jakirkham commented 10 months ago

Maybe we can try one of these changes in a PR?

IIRC the line ending issue was tricky to get right depending on whether the user was on UNIX or Windows also depending on whether the patch/source code was written on UNIX or Windows. So we probably want to test all 4 cases to make sure that things behave as intended

mbargull commented 10 months ago

Maybe we can try one of these changes in a PR?

Yes, it would be nice if someone could try it out.

IIRC the line ending issue was tricky to get right depending on whether the user was on UNIX or Windows also depending on whether the patch/source code was written on UNIX or Windows.

Yes, depending on the users' Git settings and platforms they use things can break easily. A good example to test things out for would be python-feedstock since we do have patches for files with different line endings in there.

So we probably want to test all 4 cases to make sure that things behave as intended

No, we'd only need to test the

*.patch -merge -text
*.diff -merge -text

case. binary is just an alias for -diff -merge -text. So, the minimal change to get diff to display contents of *.patch/*.diff without risking further issues is just to remove the -diff part of binary. See https://git-scm.com/docs/gitattributes/2.43.0 for details (search for -diff/-text).