Skip to content

Conversation

@ndeybach
Copy link
Contributor

@ndeybach ndeybach commented Dec 11, 2025

The line ending of some scripts had CRLF line endings.

Which was posing problem under linux when running the source scripts/activate.sh script without manual conversion.

To avoid this in the future, this PR adds a .giattributes file to normalize .sh to LF line ending.

@ndeybach
Copy link
Contributor Author

I also did a normalisation locally on my computer with :

find . -type f -name '*.sh' -exec dos2unix {} +

I do not know if the .gitattributes will force line ending for all future git clones ...

Don't hesitate to tell me if another action is needed on my side. It is my first time doing such a change on an existing public repo.

@ndeybach ndeybach closed this Dec 13, 2025
@ndeybach ndeybach deleted the add-git-attributes branch December 13, 2025 19:15
@ndeybach ndeybach restored the add-git-attributes branch December 13, 2025 19:17
@ndeybach ndeybach reopened this Dec 13, 2025
@ndeybach
Copy link
Contributor Author

ndeybach commented Dec 13, 2025

@WyattBlue

You seem to be reviewer / commiter on pyAV if I am not wrong ?

I have updated to latest commit this branch.

Could you take a look and if all looks ok to you merge it ?

@WyattBlue
Copy link
Member

Why would this causes issues on Linux, but not on MacOS? I can run scripts on my local machine just fine.

This violates the principal of least surprise because I don't expect git to be changing the format of end lines.

@ndeybach
Copy link
Contributor Author

ndeybach commented Dec 14, 2025

On the Linux vs macOS observation: one common reason this shows up is the execution path. If a CRLF script is run as ./script.sh, the \r on the shebang line can make the interpreter path invalid (e.g. /bin/bash\r), which fails immediately. If it's run as bash script.sh, that specific failure mode doesn’t apply, so it can look like it "runs fine" on one machine and not the other. Not having a mac though, I cannot confirm directly on why it works on your. You also could have set an auto crlf system wide (or git by default)...

On "least surprise": I agree it's surprising if we force LF across many file types. The intent here is to avoid platform-dependent behavior caused by per-user Git/editor settings. To keep the repo behavior predictable without being heavy-handed, I can reduce the .gitattributes scope to only the files where CRLF is a known execution risk (shell scripts + Makefiles), and leave the rest to * text=auto. What is your opinion on this direction ?

@ndeybach
Copy link
Contributor Author

ndeybach commented Dec 14, 2025

Just to clarify what I'm trying to achieve with .gitattributes: it's a way to make line endings predictable across contributors. Without it, the same file can be checked out as LF or CRLF depending on each person’s Git config/editor, and that’s where the "works here, breaks there" case you cite could come from imo...

Also, with the reduced scope, this won't force a repo-wide "LF everywhere" rule. It would only pin LF for the few files where CRLF is known to cause real problems (shell scripts / Makefiles). Everything else stays whatever Git would normally do (* text=auto) and can still be platform-specific when it makes sense.

@ndeybach
Copy link
Contributor Author

@WyattBlue

I have updated the gitattributes to extreme minimalism on the matter. Only .sh scripts and makefile.

Does that look good to you?

To minimize the area of influence of line ending enforcement, I added only bash scripts and makefiles to forced LF line ending.
@WyattBlue WyattBlue merged commit f10e3ae into PyAV-Org:main Dec 16, 2025
6 checks passed
@ndeybach ndeybach deleted the add-git-attributes branch December 16, 2025 12:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants