-
Notifications
You must be signed in to change notification settings - Fork 810
Force LF line endings on text files #9353
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
Conversation
Depending on how git is configured on Windows, git may attempt to normalize text file line endings to CRLF (\r\n), which results in broken Docker containers being built
WalkthroughA Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes This is a straightforward configuration file addition with no logic or code changes required. Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.gitattributes (1)
2-2: Consider adding explicit binary file markers for extra safety.While
text=autoshould adequately identify binary files, you might consider adding explicit patterns (e.g.,*.png binary,*.exe binary,*.jpg binary) for certain high-risk binary formats. This is a common best practice to prevent accidental normalization of non-text files.For example:
# Auto detect text files and normalize end of lines to LF (\n) * text=auto eol=lf # Explicitly mark common binary files *.png binary *.jpg binary *.jpeg binary *.gif binary *.exe binaryThat said, many projects successfully use
* text=auto eol=lfwithout explicit markers, so this is optional.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.gitattributes(1 hunks)
🔇 Additional comments (1)
.gitattributes (1)
1-2: Correct approach to enforce LF line endings across platforms.The
.gitattributesrule correctly leverages Git's auto-detection and line-ending normalization to ensure text files consistently use LF, which solves the stated problem of Docker containers failing on Windows due to CRLF line endings.
|
Sorry for the doubling of PRs, completely screwed up the commit history on the last one (#9349) 😅 |
|
Hi @Guiorgy, which ticket is associated with this PR? |
|
@khushboovashi BTW, this is unrelated, just an FYI, in commit 57f88cf you added I was about to revert that patch as our build machines were failing due to that change. |
|
Technically, if the whole team is using Linux, Unix. OSX, or Windows with differently configured git, this isn't an issue to you all, but this would help those that want to contribute, especially on the Docker side, that are using Windows with git configured to "fix" line endings. Building an image without this patch on my Windows machine: > git status
On branch master
Your branch is up to date with 'origin/master'.
nothing to commit, working tree clean
> docker build --tag "pgadmin4:crlf" .
...
=> exporting to image 10.0s
=> => exporting layers 10.0s
=> => writing image sha256:5ba4ccc73836adf9922d29d9d2a73e2d21eaa451c57dc5478697e3c311e0e6f4 0.0s
=> => naming to docker.io/library/pgadmin4:crlf 0.0s
> docker run --rm -p 8080:80 -e [email protected] -e PGADMIN_DEFAULT_PASSWORD=admin pgadmin4:crlf
': No such file or directoryThe image fails to run, and with a very non-descriptive error at that. |
Depending on how git is configured on Windows (for example mine), git may attempt to normalize text file line endings to CRLF (\r\n), which results in broken Docker containers being built, as, for example, the entry point shell script fails to execute if it has CRLF line endings.
This PR adds the
.gitattributesfile instructing git to force LF (\n) line endings on text files.Demonstration of the
entrypoint.shscript having Windows CRLF line endings:Demonstration of the
entrypoint.shscript having Unix LF line endings after applying this PR (and resetting files by executinggit rm --cached -r . && git reset --hard HEADto force git to apply the rules, since they are applied lazily when a file is modified):Demonstration of the Docker build and run on a Windows machine:
The page
localhost:8080responded:So the image builds and runs successfully on Windows.
Summary by CodeRabbit