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

Use git config core.autocrlf false on windows machines #1184

Closed
joyeecheung opened this issue Mar 14, 2018 · 6 comments
Closed

Use git config core.autocrlf false on windows machines #1184

joyeecheung opened this issue Mar 14, 2018 · 6 comments

Comments

@joyeecheung
Copy link
Member

joyeecheung commented Mar 14, 2018

Refs: nodejs/node#19221

I think the current git setting on those machines converts CRLF to LF on checkout, which should not be what normal windows devs are using because the JS linter would complain about all the CRLF in the code base, and this causes the Windows CI of nodejs/node#19221 to fail because it is matching output mixed with code from local files.

Can somebody help me configure this on the CI? git config core.autocrlf false should stop git from converting the line feeds. I don't think I have access to the windows jobs. Thanks!

@bzoz
Copy link

bzoz commented Mar 14, 2018

vcbuild lint-js works fine with both core.autocrlf set to true and false. The GitHub documentation suggests setting it to true, so I would guess most Windows devs would have it this way (at least I do).

@joyeecheung
Copy link
Member Author

@bzoz Hmm, in that case we should set git config core.autocrlf true on the CI? I am not sure about the current settings (input?), but it is converting LF to CRLF on check out so the tests in nodejs/node#19221 fails on the CI.

@joyeecheung
Copy link
Member Author

@MSLaguana suggested in nodejs/node#19221 (comment) to update .gitattributes instead, I'll try that route for the time being

@joaocgreis
Copy link
Member

There has been plenty of discussion around this in the past. In Windows, LF should work as much as possible but CRLF is the right line ending. Thus, we should always make sure that Node compiles and works with CRLF. Git is installed with with core.autocrlf=true, which is the default setting so I believe it's how most developers have it.

I've once made my own attempt at .gitattributes, nodejs/node#7019 (actually started much simpler than the final version there now), but there was no consensus so nodejs/node#8785 landed instead.

autocrlf should stay true in CI machines. @joyeecheung I'll try to take a better look at your PR in node.

@maclover7
Copy link
Contributor

@joyeecheung @BridgeAR is this still needed?

@joyeecheung
Copy link
Member Author

@maclover7 The particular issue should be fixed by nodejs/node#20754 I think it's fine to keep core.autocrlf=true if we want to test the default git config on Windows as @joaocgreis suggested.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants