Skip to content

Ensure that shell scripts and Dockerfiles use Unix line-endings #5949

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

cowwoc
Copy link

@cowwoc cowwoc commented Apr 26, 2025

No description provided.

Copy link
Member

@tonistiigi tonistiigi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Based on that link isn't it easier to just add core.autocrlf = input to your gitconfig?

.gitattributes Outdated
# Auto detect text files and perform LF normalization
* text=auto

*.sh eol=lf
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why only these files? We only use unix line-endings.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tonistiigi Because most other text files, like source-code, are expected to have CRLF line endings on Windows.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with @tonistiigi — enforcing LF consistently is important.

I usually work in teams with a mix of Windows and Linux devs, and setting the following helps avoid cross-platform line-ending issues:

In .gitattributes:

* text = lf

And in .editorconfig:

root = true

[*]
end_of_line = lf


This makes sure both Git and editors enforce LF, which avoids diffs from accidental CRLF on Windows. This ensures editors (like VS Code, IntelliJ, etc.) also respect LF line endings, regardless of local defaults.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From what I read, not all Windows editors support LF endings but you're right that all the major coding editors certainly do.

I'm fine with defaulting to LF endings if you guys prefer. Let me know if you want me to update the PR accordingly.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's change it to lf for all text files if we want to get this in.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. Try now.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@alexgwolff PTAL

@cowwoc Please squash commits as well.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I usually work in teams with a mix of Windows and Linux devs, and setting the following helps avoid cross-platform line-ending issues

text attribute only affects check-in normalization, which while important does not prevent files being normalized to CRLF on checkout for Windows by default. You will need to set eol=lf for check-out. Usually there are only a few files that explicitly require this so it's better to be explicit IMO.

The editorconfig is complimentary, but last I recall wasn't used by default by VSCode without a plugin/extension active 🤔 (perhaps that has changed since, or you just haven't had to think about it since it's set and forget), I vaguely mention it here:

Another alternative is leveraging our .editorconfig with an editor that supports it (such as VSCode with the official extension installed).

This only applies the EOL conversion upon save however, thus not foolproof of working-tree gotchas.

On Windows a new blank file would default to CRLF last I recall, that's partially helpful should you run any tests before committing changes you make.

  • However, like text=lf / text=auto in .gitattributes it wouldn't switch to LF from CRLF when you checkout a commit on Windows, you'd have to open the file and save it (which won't be visible to git as a difference since it's only concerned with line-endings when the changes are staged).
  • If you do get LF at checkout and haven't adjusted git config on Windows then whatever you're using to perform the checkout would be doing that for you.

Because most other text files, like source-code, are expected to have CRLF line endings on Windows.

From what I read, not all Windows editors support LF endings but you're right that all the major coding editors certainly do.

It shouldn't be an issue these days. Only when a file should be CRLF (.bat) or LF (.sh / Dockerfile with Linux base image) does it really matter and these are affected by eol attribute.

You use the text=lf / text=auto attribute for git check-in normalization since by default an editor may default to CRLF (VSCode can do this when creating a new file), and some editors may not normalize the file to a specific line-ending IIRC, this can result in mixed line-endings which is more problematic, normalizing line-endings at check-in resolves that (unless files are already committed with mixed line-endings).


In my review comment I provided additional links to back up what I'm saying, since all involved in this review discussion thus far don't seem to understand .gitattributes well enough.

This:

# Only set `text=lf` if file appears to be text content:
* text=auto

# Check-out files with the `.sh` extension as LF, even on Windows (default CRLF):
*.sh eol=lf

is not equivalent to:

# Commit every file with line-endings bytes normalized to LF:
* text=lf

# Unset `text=lf` via `binary` attribute (aka `-text -diff`):
*.gif binary

The change suggested/applied is not an improvement but a regression to the PR.

Signed-off-by: Gili Tzabari <cowwoc2020@gmail.com>
@cowwoc cowwoc force-pushed the added-gitattributes branch from e5bce49 to 4c48984 Compare May 4, 2025 01:16
@alexgwolff
Copy link

missing .editorconfig to enforce configs in all major IDEs

alexgwolff

This comment was marked as duplicate.

Signed-off-by: Gili Tzabari <cowwoc2020@gmail.com>
@cowwoc
Copy link
Author

cowwoc commented May 21, 2025

@alexgwolff Done

Copy link
Contributor

@polarathene polarathene left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TL;DR: You might want to consider eol=lf too, but I'm not sure if that's ok as * text=auto eol=lf if it could affect binary files 🤔


For additional reference if anyone is interested about .gitattributes, I documented it rather thoroughly (related PR for more info).

You'll notice that while * text=auto does the equivalent of * text=lf with binary detection (thus the .gif line is probably not necessary if you trust git to distinguish between binary/text content), that I still have files that need to be lf are assigned an explicit eol=lf attribute.

That is because text=lf / text=auto is only adjusting to LF on check-in. That will not ensure files are checked out via git with LF on Windows, you'd still get CRLF by default (Dockerfile using RUN instructions with HereDoc strings fails without LF for example, another is interactions with formatting tools). See my earlier linked reference notes for more context.

If CRLF has been committed in the past, you may need to perform a normalization to convert those to LF. It is possible some files may even have mixed line-endings (as I encountered before, link provides full details and solution).

@@ -0,0 +1,4 @@
* text=lf
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* text=lf
# Normalize line endings of all non-binary files to LF upon check-in (`git add` / `git commit`):
* text=auto

NOTE:

  • This should technically make the *.gif binary line redundant.
  • text=auto / text=lf only affects check-in. Should anyone check-out on Windows it'll likely normalize to CRLF (only matters if they try to run/process files with CRLF that should otherwise be LF).

Comment on lines +7 to +8
[*]
end_of_line = lf
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
[*]
end_of_line = lf
[*]
charset = utf-8
end_of_line = lf
insert_final_newline = true

@@ -0,0 +1,8 @@
# https://editorconfig.org

# top-most EditorConfig file
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
# top-most EditorConfig file
# Treat this config file as the top-level (root) EditorConfig file:

@cowwoc
Copy link
Author

cowwoc commented May 26, 2025

I'm fine with the proposed changes if the rest of you agree. While we're on the topic, what (else) is preventing this PR from finally getting merged?

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.

5 participants