Skip to content

Add .gitattributes to prevent CRLF issues on WSL (#508) #509

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from

Conversation

stoneHee99
Copy link
Contributor

This PR adds a .gitattributes file that enforces LF line endings for shell scripts.
This resolves an issue where WSL users may encounter:

/usr/bin/env: ‘bash\r’: No such file or directory

This PR addresses Issue #508

Summary

  • Ensures .sh scripts are checked out with LF even on Windows
  • Helps WSL users avoid line-ending related execution errors

@native-api
Copy link
Member

native-api commented May 11, 2025

Will this actually help? Other files read by shell scripts will also have bogus CRLFs in them. And the main executable script don't have extensions at all!

You need to rather checkout the repo with autocrlf=input or false (input is prerefable, it will prevent you from accidentally committing CRLF into the repo)

@stoneHee99
Copy link
Contributor Author

Thanks for the thoughtful feedback!

You're absolutely right — this .gitattributes change won't cover all CRLF-related issues, especially for scripts without extensions or other shell-read files.

My intention here was to address one of the most frequent problems reported by WSL users, where .sh files cloned with CRLF cause bash\r errors. While not comprehensive, this change helps prevent the most common trap for new contributors.

That said, I agree that documenting core.autocrlf=input in the README might be a better long-term solution. I'd be happy to update this PR or send a follow-up one to include a note in the README for WSL/Windows users.

Let me know what direction you'd prefer!

@native-api
Copy link
Member

Pyenv-Virtualenv has only one .sh file, install.sh, which does not even participate in regular use.
So the change as currently proposed will have no effect in any case.

@stoneHee99
Copy link
Contributor Author

Thanks for clarifying — you're right, I didn't realize that pyenv-virtualenv itself has no actively used .sh scripts. That makes this .gitattributes ineffective in practice.

Instead, I'd be happy to open a follow-up PR to update the README with a note for WSL/Windows users recommending core.autocrlf=input, since this was the root cause of my issue. I think that would be more helpful for others facing similar problems.

Let me know if that sounds good!

@native-api
Copy link
Member

native-api commented May 12, 2025

Documenting core.autocrlf use SGTM -- though we need to somehow ensure the users will actually see that. Probably an installation section for WSL?

How do they primarily install Pyenv in WSL?
If with the official installer then https://github.com/pyenv/pyenv-installer could use setting autocrlf for the repos.

@stoneHee99
Copy link
Contributor Author

Thanks for the clarification — that makes sense.

I installed pyenv and pyenv-virtualenv manually (via git clone), which I imagine is still fairly common, especially for WSL users following the README directly.

So I think having a brief WSL-specific note in the README could still be helpful for users not relying on pyenv-installer. That said, I agree that adding core.autocrlf=input in the installer would be a more robust long-term solution, and I’d be happy to open a separate PR there if that’s helpful.

Let me know if you'd prefer the README note as a standalone section or just a short warning under the Installation section — I can update this PR accordingly.

@native-api
Copy link
Member

native-api commented May 12, 2025

I reckon a "WSL note:" before the git clone commands in Pyenv-Virtualenv README and/or a WSL section / similar note in Pyenv README would be natural-looking locations.

@stoneHee99
Copy link
Contributor Author

Thanks for the thoughtful discussion and guidance!

You're absolutely right — since this change doesn't actually affect any files in this repository, and core.autocrlf is better addressed through documentation, I'll go ahead and close this PR.

I'll follow up shortly with a new PR that adds a WSL note in the README as suggested.

Appreciate your help in finding the right direction! 🙌

@stoneHee99 stoneHee99 closed this May 12, 2025
@stoneHee99 stoneHee99 deleted the fix/wsl-crlf-lineendings branch May 12, 2025 02:05
@stoneHee99
Copy link
Contributor Author

Confirmed that pyenv-installer already handles this more safely using -c core.autocrlf=false, so no further action needed there.

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