Skip to content
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

fix: Ensure committed files are normalized to LF #361

Merged

Conversation

polarathene
Copy link
Contributor

This avoids accidentally committing files with CRLF line endings which rustfmt will happily normalize to LF creating a very noisy diff from these invisibles.


Additional context

As I'm presently developing from Windows with VSCode and I haven't adjusted defaults yet, when I create new files they default to CRLF for line endings.

I was only aware of this after running rustfmt and noticing every single line had been changed in the diff, even though there was no visual difference the invisibles change from CRLF to LF was the cause (_since I had already committed this file quite a while ago with CRLF before running rustfmt.

.gitattributes is a useful file to prevent that sort of mistake from occurring as the project gains more contributors. It also helps ensure better compatibility when certain files should be checked out with LF or CRLF specifically, which I've added a contextual comment for the maintainers benefit.


Dockerfile can be one of the cases where you want to ensure LF is used as checkout, since docker build can fail RUN directives under some contexts (as I have experienced in the past):

FROM alpine

# This will work:
RUN chmod +x /bin/sh

# However the HereDoc feature retains the
# trailing CRLF, triggering a build error:
# (Fails with sh/bash/ash)
RUN <<EOF
  chmod +x /bin/sh
EOF
$ docker build --no-cache --tag crlf-example -f Dockerfile .

# ...                                                                                                                                                      
 => CACHED [1/2] FROM docker.io/library/alpine:latest
 => [2/3] RUN chmod +x /bin/sh
 => ERROR [3/3] RUN <<EOF (chmod +x /bin/sh)
------
 > [3/3] RUN <<EOF (chmod +x /bin/sh):
: No such file or directory
------
Dockerfile:9
--------------------
   8 |     # (Fails with sh/bash/ash)
   9 | >>> RUN <<EOF
  10 | >>>   chmod +x /bin/sh
  11 | >>> EOF
  12 |
--------------------
ERROR: failed to solve: process "/bin/sh -c   chmod +x /bin/sh\r\n" did not complete successfully: exit code: 1

Likewise for any scripts like .sh relying on the shebang to be executed that are added to the image via COPY, since the shebang line would have CRLF in environments that default to that otherwise without eol=lf. Similar for .bat which should use eol=crlf.


Dockerfile-cuda-all was renamed to a more conventional Dockerfile.cuda-all.

Most editors will better recognize Dockerfile.* or *.Dockerfile (which Dockers official docs suggest as convention). I went with Dockerfile. as a prefix for better co-location, similar to what the repo is already doing.

Copy link

Code Metrics Report
  ===============================================================================
 Language            Files        Lines         Code     Comments       Blanks
===============================================================================
 Dockerfile              1           34           25            0            9
 Happy                   1          442          369            0           73
 JSON                    9           21           21            0            0
 Python                 21          741          622           21           98
 TOML                   15          393          355            1           37
-------------------------------------------------------------------------------
 Jupyter Notebooks       1            0            0            0            0
 |- Markdown             1           60           30           22            8
 |- Python               1           96           87            1            8
 (Total)                            156          117           23           16
-------------------------------------------------------------------------------
 Markdown               16         1054            0          781          273
 |- BASH                 6          203          190            0           13
 |- Python               6          121          110            0           11
 |- Rust                 3          185          172            9            4
 (Total)                           1563          472          790          301
-------------------------------------------------------------------------------
 Rust                   86        28448        26034          379         2035
 |- Markdown            42          439            0          427           12
 (Total)                          28887        26034          806         2047
===============================================================================
 Total                 151        31133        27426         1182         2525
===============================================================================
  

This avoids accidentally committing files with CRLF line endings which `rustfmt` will happily normalize to LF creating a very noisy diff from these invisibles.
Copy link
Owner

@EricLBuehler EricLBuehler left a comment

Choose a reason for hiding this comment

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

Thanks!

@EricLBuehler EricLBuehler merged commit d5c5a09 into EricLBuehler:master May 30, 2024
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants