docker / cli

The Docker CLI
Apache License 2.0
4.75k stars 1.88k forks source link

feat: force lf line endings by default #5216

Closed Benehiko closed 6 days ago

Benehiko commented 1 week ago

- What I did

On Windows machines users' might install git using the default "Checkout Windows-style, commit Unix-style line endings". image

Or

git config --global core.autocrlf true

This will prevent the user from building the CLI due to bash scripts inside the scripts/* directory to automatically be perceived with the CRLF line ending. The error is ambiguous as to suggest a file is missing - however, it is due to bash scripts having incorrect line endings.

$ docker buildx bake

=> ERROR [build 2/2] RUN --mount=type=bind,target=.,ro     --mount=type=cache,target=/root/.cache     --mount=type=tmp  1.8s ------                                                                                                                         

> [build 2/2] RUN --mount=type=bind,target=.,ro     --mount=type=cache,target=/root/.cache     --mount=type=tmpfs,target=cli/winresources     xx-go --wrap &&     TARGET=/out ./scripts/build/binary &&     
xx-verify $([ "static" = "static" ] && echo "--static") /out/docker:                                                                                                        
': No such file or directory                                                                                                 

 ------                                                                                                                        
Dockerfile:64                                                                                                                 
--------------------                                                                                                            
63 |     COPY --link --from=goversioninfo /out/goversioninfo /usr/bin/goversioninfo                                           
64 | >>> RUN --mount=type=bind,target=.,ro \                                                                                  
65 | >>>     --mount=type=cache,target=/root/.cache \                                                                         
66 | >>>     --mount=type=tmpfs,target=cli/winresources \                                                                     
67 | >>>     # override the default behavior of go with xx-go                                                                 
68 | >>>     xx-go --wrap && \                                                                                                
69 | >>>     # export GOCACHE=$(go env GOCACHE)/$(xx-info)$([ -f /etc/alpine-release ] && echo "alpine") && \                 
70 | >>>     TARGET=/out ./scripts/build/binary && \                                                                          
71 | >>>     xx-verify $([ "$GO_LINKMODE" = "static" ] && echo "--static") /out/docker                                        
72 |                                                                                                                        --------------------                                                                                                          ERROR: failed to solve: process "/bin/sh -c xx-go --wrap &&     TARGET=/out ./scripts/build/binary &&     
xx-verify $([ \"$GO_LINKMODE\" = \"static\" ] && echo \"--static\") /out/docker" 
did not complete successfully: exit code: 127

- How I did it

Added * text=auto eol=lf to .gitattributes to force git to by default treat files in the repository with the lf line ending.

- How to verify it

Checkout the repository on Windows and with git config --global core.autocrlf true then open any of the script files in an editor, the editor should tell you the line endings are LF. You should also be able to see inside WSL, running file scripts/build/binary should print ... ASCII text executable.

If there's something about it containing CRLF then it means it's incorrect.

- Description for the changelog

- A picture of a cute animal (not mandatory but encouraged)

codecov-commenter commented 1 week ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 61.42%. Comparing base (0340921) to head (c56f4a1). Report is 2 commits behind head on master.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #5216 +/- ## ======================================= Coverage 61.42% 61.42% ======================================= Files 298 298 Lines 20798 20798 ======================================= Hits 12776 12776 Misses 7109 7109 Partials 913 913 ```
thaJeztah commented 1 week ago

I think with this change, we may be able to get rid of this part in our GHA workflow as well; https://github.com/docker/cli/blob/e2361a5ca888509f1560e60a4810597a31750266/.github/workflows/test.yml#L52-L57

If that's correct, you can remove it as part of this PR as well (can be a separate commit) /cc @crazy-max