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

ci: .gitattributes - Ensure eol=lf for shell scripts #3755

Merged
merged 2 commits into from
Jan 7, 2024

Conversation

polarathene
Copy link
Member

@polarathene polarathene commented Jan 7, 2024

Description

  • These files should always use LF for line endings.
  • Dockerfile does not like building with HereDoc RUN scripts that expect LF.

Cloned on Windows, but ran Docker build from WSL2 which failed as Windows checkout used CRLF:

 => [stage-base 2/6] COPY target/bin/sedfile /usr/local/bin/sedfile                                                                                                                                               0.0s
 => ERROR [stage-base 3/6] RUN <<EOF (chmod +x /usr/local/bin/sedfile...)                                                                                                                                         0.4s
------
 > [stage-base 3/6] RUN <<EOF (chmod +x /usr/local/bin/sedfile...):
0.372 chmod: cannot access '/usr/local/bin/sedfile'$'\r': No such file or directory
------
Dockerfile:23
--------------------
  22 |     COPY target/bin/sedfile /usr/local/bin/sedfile
  23 | >>> RUN <<EOF
  24 | >>>   chmod +x /usr/local/bin/sedfile
  25 | >>>   adduser --quiet --system --group --disabled-password --home /var/lib/clamav --no-create-home --uid 200 clamav
  26 | >>> EOF
  27 |
--------------------
ERROR: failed to solve: process "/bin/bash -e -o pipefail -c   chmod +x /usr/local/bin/sedfile\r\n  adduser --quiet --system --group --disabled-password --home /var/lib/clamav --no-create-home --uid 200 clamav\r\n" did not complete successfully: exit code: 1
make: *** [Makefile:21: build] Error 1

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • Improvement (non-breaking change that does improve existing functionality)

Checklist:

  • New and existing unit tests pass locally with my changes
  • I have added information about changes made in this PR to CHANGELOG.md

- These files should always use LF for line endings.
- `Dockerfile` does not like building with HereDoc `RUN` scripts that expect LF.
@polarathene polarathene added area/ci kind/bug/fix A fix (PR) for a confirmed bug labels Jan 7, 2024
@polarathene polarathene added this to the v14.0.0 milestone Jan 7, 2024
@polarathene polarathene self-assigned this Jan 7, 2024
@casperklein
Copy link
Member

Shouldn't this take care of all file types, not explicitly configured?

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

@polarathene
Copy link
Member Author

polarathene commented Jan 7, 2024

Shouldn't this take care of all file types, not explicitly configured?

It does in the sense of the text attribute but that is regarding check-in.

eol=lf affects check-out. By default on git a clone on Windows will checkout as CRLF unless we explicitly configure it to be LF. Certain files expect to have LF line endings (notably scripts, python included apparently), or at least they do when running on Linux 😅

Until I can switch back to linux or use VMs again, this is my lazy way of addressing the issue when I hit failure due to line endings being incorrect.


Technically, we probably could do * text=auto eol=lf, assuming eol=lf only applies to files detected as text 🤷‍♂️

Not sure if there would be any files in the project that would actually need eol=crlf 🤔

@polarathene polarathene merged commit 6d66651 into master Jan 7, 2024
1 check passed
@polarathene polarathene deleted the ci/ensure-lf-for-shellscript-content branch January 7, 2024 20:34
@casperklein
Copy link
Member

TIL cloning a repo does not just copy all files 1:1 (without any conversion) 😆

You might give git config --global core.autocrlf false a try.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/ci kind/bug/fix A fix (PR) for a confirmed bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants