fix: gitattributes enforcing line endings

[Benehiko]

fixes #5377

- What I did
Switch to an opt-in approach to forcing the lf line ending to certain files and directories.

- How I did it
Change the first line * text=auto eol=lf to not enforce line endings and instead only force the scripts/ directory to use lf line endings. Also add future support for .bat files in-case we add them and also enforce .sh files with lf line endings.

- How to verify it
Checkout the branch after setting git config core.autocrlf true on a windows machine. On linux we can also enforce the line endings with git config core.eol crlf and use that as a test. There shouldn't be any files marked by git as changed.

- Description for the changelog

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

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 59.73%. Comparing base (2b6d2d9) to head (4a6ab2b).
Report is 4 commits behind head on master.

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #5381   +/-   ##
=======================================
  Coverage   59.73%   59.73%           
=======================================
  Files         345      345           
  Lines       23394    23394           
=======================================
  Hits        13974    13974           
  Misses       8450     8450           
  Partials      970      970           

[Benehiko]

I've updated this PR to be opt-in to only certain files and the scripts/ directory. This should allow git to sort out binary files for us.

[thaJeztah]

LGTM

[mbhadale]

Changes LGTM. Can I have an ETA, please? We are blocked due to this issue. Thanks.

[mbhadale]

Hi, I don't see the changes in the master branch. https://github.com/docker/cli/blob/master/.gitattributes.

[mbhadale]

Could you please share ETA?

[mbhadale]

Still see cli/command/container/testdata/utf8.env file modified after git clone on Linux machine.

-bash-4.2$ git status
On branch master
Your branch is up-to-date with 'origin/master'.
Changes not staged for commit:
(use "git add ..." to update what will be committed)
(use "git checkout -- ..." to discard changes in working directory)

modified: cli/command/container/testdata/utf8.env
no changes added to commit (use "git add" and/or "git commit -a")

[Benehiko]

@mbhadale what is your git config values for git config get core.autocrlf and git config get core.eol and is this from a fork or from us? Do you see this only after cloning?

[Benehiko]

Seems like this file was maybe committed with CRLF line endings.

⋊> ~/G/cli on master ◦ file cli/command/container/testdata/utf8.env                                   09:30:18
cli/command/container/testdata/utf8.env: Unicode text, UTF-8 (with BOM) text, with CRLF line terminators
⋊> ~/G/cli on master ◦ git log -p -- file cli/command/container/testdata/utf8.env                     09:30:25
commit 1630fc40f8f9f665a7292838b755520b1fefe307
Merge: 130222870 e5b7b7e87
Author: Daniel Nephin <[email protected]>
Date:   Mon Apr 17 17:40:59 2017 -0400

    Import docker/docker/cli
    
    Signed-off-by: Daniel Nephin <[email protected]>

[thaJeztah]

Looks like those fixtures were added to test issues on Windows, so it's likely they were indeed CRLF on purpose; original commit that added them was in moby/moby; moby/moby@01d3a16

Return error if env file contains non-ascii or utf8 bytes (for windows)

This fix tries to address the issue raised in 26179 where an env file
with non-ascii or utf8 bytes will crash on windows platform.

The issue is two-fold:

• Windows will adds a BOM mark at the begining with Notepad as the editor
• Non-utf8 bytes can not be handled by env file parser.

This fix removes utf8 BOM marker if exists so that utf8 encoded env file
could be processed.
This fix also returns an error (instead of a runtime CreateProcess crash)
if env file contains non-utf8 bytes, thus giving users better experiences.

Additional test cases has been added in unit tests.

• Return error if env file contains non-ascii or utf8 bytes (for windows) moby/moby#26296

[mbhadale]

This is what I get.

-bash-4.2$ git config --list
push.default=simple
[email protected]:.insteadof=https://gitlab.eng.vmware.com/
core.autocrlf=input
core.repositoryformatversion=0
core.filemode=true
core.bare=false
core.logallrefupdates=true
[email protected]:core-build/mirrors_github_docker_cli.git
remote.origin.fetch=+refs/heads/*:refs/remotes/origin/*
branch.master.remote=origin
branch.master.merge=refs/heads/master

[Benehiko]

If you’re on a Linux or macOS system that uses LF line endings, then you don’t want Git to automatically convert them when you check out files; however, if a file with CRLF endings accidentally gets introduced, then you may want Git to fix it. You can tell Git to convert CRLF to LF on commit but not the other way around by setting core.autocrlf to input:

$ git config --global core.autocrlf input

https://git-scm.com/book/en/v2/Customizing-Git-Git-Configuration

This might be converting the cli/command/container/testdata/utf8.env file on checkout for you. This file should be CRLF and not LF. Try core.autocrlf=false?

[mbhadale]

Still seeing this issue with core.autocrlf=false.

-bash-4.2$ git config --list
[email protected]:.insteadof=https://gitlab.eng.vmware.com/
core.autocrlf=false
core.repositoryformatversion=0
core.filemode=true
core.bare=false
core.logallrefupdates=true
remote.origin.url=https://github.com/docker/cli.git
remote.origin.fetch=+refs/heads/:refs/remotes/origin/
branch.master.remote=origin
branch.master.merge=refs/heads/master

[Benehiko]

This is a bit difficult to debug since I cannot reproduce this on my Linux machine 🙈
Can you maybe test the clone on a different environment?

Also re-read this section of the Git book

You can tell Git to convert CRLF to LF on commit but not the other way around by setting core.autocrlf to input

Seems like this wouldn't do anything on checkout and only on commit.

[mbhadale]

I still see the issue, I tried setting git config core.eol crlf as mentioned in the testing section of the issue.

As per this #5381 (comment) file has CRLF ending.

I think we should remove * text=auto and specify files manually if they want git to handle line endings.
Or we can add an exception for this env file.

utf8.env -text

Let me know what you think.

[Benehiko]

I still see the issue, I tried setting git config core.eol crlf as mentioned in the testing section of the issue.

As per this #5381 (comment) file has CRLF ending.

I think we should remove * text=auto and specify files manually if they want git to handle line endings. Or we can add an exception for this env file.

utf8.env -text

Let me know what you think.

I'm not so sure. I still cannot reproduce what you are seeing, nor can any of my colleagues. Tested on Linux and Windows.

We don't enforce a particular line ending on checkout with * text=auto which is intentional.

When text is set to "auto", Git decides by itself whether the file is text or binary. If it is text and the file was not already in Git with CRLF endings, line endings are converted on checkin and checkout as described above. Otherwise, no conversion is done on checkin or checkout.

The only enforcement that is overriding your git config is for the scripts/ directory since they don't have a file extension causing Windows with certain git configs to add CRLF line endings on checkout, causing builds through Docker to fail.

The gitattributes config aren't enforcing a particular line-ending (except for scripts/). I don't see why adding an exclusionary clause for this particular file is necessary since we don't even know why you are seeing a diff on clone for this particular file.

In the issue you mention "This issue is observed as intermittent but the frequency of failure is high.". So it's not a consistent thing? It only happens now and again?

You also mention there is a mirror branch " We can not fix this in our mirror branch"? Maybe the original gitattributes file has messed with your mirror since it did enforce the lf line-ending and maybe the diff you are seeing is a correction of the file? Meaning, cli/command/container/testdata/utf8.env has CRLF line endings, but your mirror converted it to LF line endings. So the diff being produced is showing that the changed file has CRLF line endings?