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

Line endings in Windows make the project can't be compiled #180

Closed
fooock opened this issue Apr 17, 2018 · 17 comments
Closed

Line endings in Windows make the project can't be compiled #180

fooock opened this issue Apr 17, 2018 · 17 comments
Labels
area/build-tools Development tooling. I.e. pretty much everything in the `build` directory. kind/bug These are bugs.
Milestone

Comments

@fooock
Copy link
Contributor

fooock commented Apr 17, 2018

I executed the make test task using Windows 10 WLS. using the master branch. The expected result is a clean build, but I get:

Successfully built 899d2fed6f79
Successfully tagged agones-build:1fd9e47722
make[1]: Leaving directory '/d/Projects/go/src/agones.dev/agones/build'
docker run --rm -v /d/Projects/go/src/agones.dev/agones/build//.config/gcloud:/root/.config/gcloud -v ~/.kube:/root/.kube -v /d/Projects/go/src/agones.dev/agones:/go/src/agones.dev/agones -w /go/src/agones.dev/agones  agones-build:1fd9e47722 bash -c \
        "/root/gen-lint-exclude.sh && gometalinter --config .exclude.gometalinter.json --deadline 100s -t --skip vendor ./..."
/usr/bin/env: 'bash\r': No such file or directory
Makefile:136: recipe for target 'lint' failed
make: *** [lint] Error 127

This is because Windows uses \r\n instead of \n. If I change the line endings to LF, the error is not shown.

How can we change this behavior to make it OS agnostic? Maybe change the .gitattributes file?

References

@markmandel markmandel added kind/bug These are bugs. area/build-tools Development tooling. I.e. pretty much everything in the `build` directory. labels Apr 17, 2018
@markmandel
Copy link
Member

@enocom you worked on #92 - shouldn't this have prevented this from happening in the first place? (if I understand gitattributes correctly?

@fooock How would/did you change .gitattributes to solve this issue?

Do we need a

Makefile text

attribute added to the .gitiattributes file to force \n line endings on Makefiles?

@fooock
Copy link
Contributor Author

fooock commented Apr 17, 2018

Currently, the content of the .gitattributes is

* text=auto

Maybe we can add the next line to format using LF only in Makefiles

Makefile text eol=lf

@markmandel
Copy link
Member

Potentially silly question, if we make the change for Makefile in the .gitattributes file - does that solve the problem? 😄

@enocom
Copy link
Contributor

enocom commented Apr 17, 2018

I was hoping per this advice that Git would do the reasonable thing across all OS's.

If we use Makefile text eol=lf, then according to the link above:

Git will always convert line endings to LF on checkout. You should use this for files that must keep LF endings, even on Windows.

Does Make on Windows require \n? If so, then I think adding a separate directive for Makefiles in .gitattributes is the reasonable thing to do.

@markmandel
Copy link
Member

On Windows we have users use WSL, so it should require \n. The weird thing is, it's WSL, and we all use Linux/Mac - and so if it works for us, I don't actually understand why it doesn't work for @fooock .

Actually if I test for \r\n on the Makefile, I don't find at crlf endings. I am wondering if this is something on your end @fooock ?

@markmandel
Copy link
Member

markmandel commented Apr 17, 2018

A thought - when you wrote:

If I change the line endings to LF, the error is not shown.

  • How where you changing the line endings? And on what files?
  • And when you made the change, did that result in a git difference vs the master branch?

Just wondering if that will shed some light on things.

@fooock
Copy link
Contributor Author

fooock commented Apr 17, 2018

When I checkout the project using the Windows git executable, git automatically use the \r\n line endings. I tested now what happens when I checkout the master branch using the bash git, and it works like in your envs (with \n)

@markmandel I use Intellij + go plugin. I changed the line endings of all project files, and when done, the error disappeared

When I made this change, git detect all project files as modified (the line endings).

@markmandel
Copy link
Member

@fooock what happens if you run:
git config core.autocrlf; git config --global core.autocrlf - see if that is false? (on Windows I mean) - it should be overwritten by the .gitattributes, but still?

Admittedly, when I was using WSL, I only ever use WSL (so I git in there as well). I wonder why windows is adding the crlf line ending on checkout? That is super weird.

@fooock
Copy link
Contributor Author

fooock commented Apr 18, 2018

Using Windows console:

Javi@FOOOCK D:\Projects\go\src\agones.dev\agones (feature/e2e)
 git config core.autocrlf
true

Javi@FOOOCK D:\Projects\go\src\agones.dev\agones (feature/e2e)
 git config --global core.autocrlf

Global flag don't show nothing (to clarify).

@markmandel
Copy link
Member

🤷‍♂️ I've no idea at this point. Somehow Git on windows adds crlf endings on checkout.

Solution - don't use it? 😄

Also curious - when you changed the line endings within IntelliJ - which files were marked as changed in Git? All of them, or a specific set?

@fooock
Copy link
Contributor Author

fooock commented Apr 18, 2018

I used the option from the File menu to change line endings in all files, and all were marked as changed by git

@markmandel
Copy link
Member

Okay. I'm officially out of ideas. 😭

@enocom
Copy link
Contributor

enocom commented Apr 18, 2018

In theory, git should overlook the conversion from CRLF to LF.

@fooock Would you be available for a video chat in the next few days? I'm thinking it would be good to see if we could solve this together, as Windows users are a big part of this project.

@enocom
Copy link
Contributor

enocom commented Apr 18, 2018

There's is also this which makes me think the configuration is correct as is.

As long as we are running all tasks within containers, the linux line ending should be fine. I suppose the tricky part is to configure one's IDE to refrain from changing the line endings to the Windows format.

Does that make sense?

Edit Should we just configure line endings to be CLRF by default since we're using containers everywhere?

@enocom
Copy link
Contributor

enocom commented Apr 18, 2018

Looking at this some more, I see the problem.

Presently, .gitattributes configures Git on Windows to convert all LF to CRLF when checking out code. When committing, those CRLF bytes are restored to LF.

So when a Windows user checks out Agones, Git will convert all files to have a CRLF line ending with any committed code converted back to LF line endings. That's fine for development.

However, since we're building containers and using local files (like build/build-image/gen-lint-exclude.sh), Git on Windows will have converted those local files to use CRLF line endings. But -- and here's the problem -- our current build pulls those local files with the CRLF line ending into a Linux container and then tries to run then. Hence, the problem we have above where the lint script is failing to run in a Linux context on account of its converted line endings.

So, @fooock if you don't mind being a test user, I think the pragmatic and working solution (at least for now) is to add exclusions to .gitattributes as they appear. The first would be this:

build/build-image/gen-lint-exclude.sh text eol=lf

@markmandel WDYT?

@fooock
Copy link
Contributor Author

fooock commented Apr 18, 2018

Related PR #182

@fooock
Copy link
Contributor Author

fooock commented Apr 20, 2018

Closed in #182

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/build-tools Development tooling. I.e. pretty much everything in the `build` directory. kind/bug These are bugs.
Projects
None yet
Development

No branches or pull requests

3 participants