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

Project doesn't build on Windows due to crlf defaults #228

Closed
digulla opened this issue Feb 4, 2019 · 9 comments
Closed

Project doesn't build on Windows due to crlf defaults #228

digulla opened this issue Feb 4, 2019 · 9 comments

Comments

@digulla
Copy link

digulla commented Feb 4, 2019

This is to discuss the need of a .gitattributes file.

git config --list --show-origin shows that core.autocrlf is not defined. I think the default is true.

How about dropping .editorconfig and configuring Git in such a way that it will work for everyone out of the box, no matter how autocrlf is configured? I have about 50 Git projects from all kinds of sources checked out right now. Your project is the only one which doesn't build after checkout because of EOL issues.

@ppalaga
Copy link
Contributor

ppalaga commented Feb 4, 2019

For the record: we started discussing this in #215 (comment)

git config --list --show-origin shows that core.autocrlf is not defined.

Strange.

I think the default is true.

No, it is not. https://stackoverflow.com/a/48230871 says the following:

Checking the git source code, core.autocrlf is set to false by default. (And has been since the property's original introduction on Feb 13, 2007, though it has since been converted from a static value to a constant.)

The Windows installer does require you to pick a value for this property which is explicitly set in the git system config.

Apparently, it works out of the box on our Windows CI.

Could please some other windows user (e.g. @gevegab ) confirm or deny that he/she gets all files checked out with CRLF end of lines on Windows? Sorry if this is a well known common behavior, but I never heart of it.

@ppalaga
Copy link
Contributor

ppalaga commented Feb 4, 2019

@digulla could you please confirm, that git config --local core.autocrlf false in the current project directory is at least a valid workaround?

@gevegab
Copy link
Contributor

gevegab commented Feb 5, 2019

The default configuration in my machine is :

PS C:\WINDOWS\system32> git config -l --show-origin
file:"C:\\ProgramData/Git/config"       core.symlinks=false
file:"C:\\ProgramData/Git/config"       core.autocrlf=true
file:"C:\\ProgramData/Git/config"       core.fscache=true
file:"C:\\ProgramData/Git/config"       color.diff=auto
file:"C:\\ProgramData/Git/config"       color.status=auto
file:"C:\\ProgramData/Git/config"       color.branch=auto
file:"C:\\ProgramData/Git/config"       color.interactive=true
file:"C:\\ProgramData/Git/config"       help.format=html
file:"C:\\ProgramData/Git/config"       rebase.autosquash=true
file:C:/Program Files/Git/mingw64/etc/gitconfig http.sslbackend=schannel
file:C:/Program Files/Git/mingw64/etc/gitconfig diff.astextplain.textconv=astextplain
file:C:/Program Files/Git/mingw64/etc/gitconfig filter.lfs.clean=git-lfs clean -- %f
file:C:/Program Files/Git/mingw64/etc/gitconfig filter.lfs.smudge=git-lfs smudge -- %f
file:C:/Program Files/Git/mingw64/etc/gitconfig filter.lfs.process=git-lfs filter-process
file:C:/Program Files/Git/mingw64/etc/gitconfig filter.lfs.required=true
file:C:/Program Files/Git/mingw64/etc/gitconfig credential.helper=manager
file:C:/Users/vega/.gitconfig   filter.lfs.clean=git-lfs clean -- %f
file:C:/Users/vega/.gitconfig   filter.lfs.smudge=git-lfs smudge -- %f
file:C:/Users/vega/.gitconfig   filter.lfs.process=git-lfs filter-process
file:C:/Users/vega/.gitconfig   filter.lfs.required=true
file:C:/Users/vega/.gitconfig   user.name=German Vega
file:C:/Users/vega/.gitconfig   user.email=gevegab@yahoo.com
file:C:/Users/vega/.gitconfig   core.sshcommand='C:\Windows\System32\OpenSSH\ssh.exe'
PS C:\WINDOWS\system32>

You can notice that core.autocrlf is set to true in C:\ProgramData/Git/config

The Git for Windows documentation states the following :

If not set explicitly with --file, there are four files where git config will search for configuration options:

$PROGRAMDATA/Git/config

(Windows-only) System-wide configuration file shared with other Git implementations.

Typically $PROGRAMDATA points to C:\ProgramData.

$(prefix)/etc/gitconfig

System-wide configuration file. (Windows-only) This file contains only the settings which
are specific for this installation of Git for Windows and which should not be shared with
other Git implementations like JGit, libgit2. --system will select this file.

$XDG_CONFIG_HOME/git/config

Second user-specific configuration file. If $XDG_CONFIG_HOME is not set or empty
$HOME/.config/git/config will be used. Any single-valued variable set in this file will
be overwritten by whatever is in ~/.gitconfig. It is a good idea not to create this file
if you sometimes use older versions of Git, as support for this file was added fairly
recently.

~/.gitconfig

User-specific configuration file. Also called "global" configuration file.

$GIT_DIR/config

Repository specific configuration file.

$GIT_DIR/config.worktree

This is optional and is only searched when extensions.worktreeConfig is present in $GIT_DIR/config.

So apparently $PROGRAMDATA/Git/config is a location that can be modified by other Git implementations. I do remember some installer asked me at some point to choose a value for autocrlf and the default choice was true, but I cannot remember if it was Git for Windows or Github Desktop or something else. @ppalaga what is the configuration in the windows CI server?

Anyway, with this configuration the build fails because of the .editorconfig validation.

I tried to manually change the local configuration to workaround the problem :

PS C:\projects\license-maven-plugin> git config --local core.autocrlf false
PS C:\projects\license-maven-plugin> git config -l --show-origin
file:"C:\\ProgramData/Git/config"       core.symlinks=false
file:"C:\\ProgramData/Git/config"       core.autocrlf=true
file:"C:\\ProgramData/Git/config"       core.fscache=true
file:"C:\\ProgramData/Git/config"       color.diff=auto
file:"C:\\ProgramData/Git/config"       color.status=auto
file:"C:\\ProgramData/Git/config"       color.branch=auto
file:"C:\\ProgramData/Git/config"       color.interactive=true
file:"C:\\ProgramData/Git/config"       help.format=html
file:"C:\\ProgramData/Git/config"       rebase.autosquash=true
file:C:/Program Files/Git/mingw64/etc/gitconfig http.sslbackend=schannel
file:C:/Program Files/Git/mingw64/etc/gitconfig diff.astextplain.textconv=astextplain
file:C:/Program Files/Git/mingw64/etc/gitconfig filter.lfs.clean=git-lfs clean -- %f
file:C:/Program Files/Git/mingw64/etc/gitconfig filter.lfs.smudge=git-lfs smudge -- %f
file:C:/Program Files/Git/mingw64/etc/gitconfig filter.lfs.process=git-lfs filter-process
file:C:/Program Files/Git/mingw64/etc/gitconfig filter.lfs.required=true
file:C:/Program Files/Git/mingw64/etc/gitconfig credential.helper=manager
file:C:/Users/vega/.gitconfig   filter.lfs.clean=git-lfs clean -- %f
file:C:/Users/vega/.gitconfig   filter.lfs.smudge=git-lfs smudge -- %f
file:C:/Users/vega/.gitconfig   filter.lfs.process=git-lfs filter-process
file:C:/Users/vega/.gitconfig   filter.lfs.required=true
file:C:/Users/vega/.gitconfig   user.name=German Vega
file:C:/Users/vega/.gitconfig   user.email=gevegab@yahoo.com
file:C:/Users/vega/.gitconfig   core.sshcommand='C:\Windows\System32\OpenSSH\ssh.exe'
file:.git/config        core.repositoryformatversion=0
file:.git/config        core.filemode=false
file:.git/config        core.bare=false
file:.git/config        core.logallrefupdates=true
file:.git/config        core.symlinks=false
file:.git/config        core.ignorecase=true
file:.git/config        core.autocrlf=false
file:.git/config        submodule.active=.
file:.git/config        remote.origin.url=https://github.com/mojohaus/license-maven-plugin.git
file:.git/config        remote.origin.fetch=+refs/heads/*:refs/remotes/origin/*
file:.git/config        branch.master.remote=origin
file:.git/config        branch.master.merge=refs/heads/master
PS C:\projects\license-maven-plugin>

But then I had to remove all the files in the cloned repository (keeping only the .git directory) and then restore them back with the good line endings using git reset --hard (found this recipe at github dealing-with-line-endings).

After that, the project builds without any additional step using the provided mvnw.cmd. Although one integration test is failing (but I guess that's another issue) :

[INFO] run script postbuild.groovy
[INFO]           aggregate-third-party-report\pom.xml ............. FAILED (6.9 s)
[INFO]   The post-build script did not succeed. assert content.contains('<a href="#Overview">Back to top</a>')
       |       |
       |       false

The first time I started working on this project effectively the build failed, but I found quickly it was because of the line endings and I simply set the autocrlf=false in the global configuration and cloned the project again, Of course I also had to configure my IDE (Eclipse in my case) to use unix line endings and UTF-8 encoding, otherwise the editorconfig fails after any editing.

I do not know whether adding a .gitattributes is a good idea or not, but I have to say that at least things are clear and explicit using the editorconfig . In any case, if you decide to add a .gitattributes, you have to especially consider the integration tests that have different test cases for windows and linux.

For instance, in test update-file-header-test-mojo I added a test file for windows test.crlf.sh (with CR-LF endings) and the corresponding test file for linux test.sh (with LF endings). Of course I also added the appropriate .editorconfig

@ppalaga
Copy link
Contributor

ppalaga commented Feb 13, 2019

@ppalaga what is the configuration in the windows CI server?

I have not touched it and I never cared so I do not know. Feel free to add git config -l --show-origin to .appveyor.yml` and send a PR to see.

From my PoV, this looks like a git misconfiguration on the user's side. It is defintely good that the editorconfig plugin is there to tell the user that he has a problem. .editorconfig is there not only to verify. It's huge advantage is that it is an IDE/editor independent code style definition format that many editors and IDEs understood and honor out of the box, so for those IDEs and editors the user does not have to do anything to get the right settings in his IDE/editor.

Now, you guys seem to indicate that this issue may tend to be very common on Windows because some Windows tools default to core.autocrlf = true. I admit having .gitattributes would increase the comfort for affected Windows user but I am still hesitant to re-implement .editorconfig there. Esp. the original proposal https://github.com/digulla/license-maven-plugin/blob/4e6a692955e28492480918c06e2b6a5332920ce2/.gitattributes of @digulla seems to be very fragile. Would it not be at least possible to set lf for all (*) and crlf for the few exceptions that we currently have?

@nfalco79
Copy link
Contributor

The same issue here. The core.autocrlf is valued to true as suggested by the git guide when there are developer that works with different operative system (https://git-scm.com/book/en/v2/Customizing-Git-Git-Configuration chapter "Formatting and Whitespace").
When set to true and clone a repository all files in workdir are created with CRLF but on commit/push are converted to LF. This settings ensure on git repository files ends all with LF because by default Max OS X and Linux use LF.

To fix this issue you could move the editorconfig plugin to a profile and use that profile just only on CI system (activated by when a environment variable is present for example). Or create a new profile that based on the OS value the editorconfig property file path to a the specific one.

@ppalaga
Copy link
Contributor

ppalaga commented Feb 13, 2019

Let's put it very clearly: core.autocrlf having true for this repo is a not possible because several tests assume specific line endings on some specific files. Those tests will inevitably fail for anybody having autocrlf having true.

The two valid solutions I see ATM are

(a) recommend users setting core.autocrlf to false which @gevegab confirmed as working

or

(b) find a maintainable setup in .gitattributes

@nfalco79
Copy link
Contributor

If so than you could close the issue

@ppalaga ppalaga added this to the Backlog milestone Feb 20, 2019
@ppalaga
Copy link
Contributor

ppalaga commented Jun 22, 2019

Windows users, I am still open to accept a .gitattributes patch that would be better maintainable than the original proposal. I'd like to see lf set for all (*) and crlf for the few exceptions that we currently have. I am not sure .gitattributes options allow for something like that. Some windows user must please figure out and try to find a working solution. This would mean some duplication against .editorconfig but be it for the comfort of Windows users.

@ppalaga ppalaga closed this as completed in c0da6d8 Jul 3, 2019
ppalaga added a commit that referenced this issue Jul 3, 2019
@ppalaga
Copy link
Contributor

ppalaga commented Jul 3, 2019

For the record: @kingle says that users that cloned the repo before #344 was merged, may need to "re-clone or git reset --hard".

In case somebody thinks this is worth documenting (I do not) in the README, please feel free to send a PR.

@ppalaga ppalaga changed the title Project doesn't build out of the box on Windows Project doesn't build on Windows due to crlf defaults Jul 3, 2019
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