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

eol=crlf in gitattributes produces unwanted outcomes on windows/linux mixed environments #493

Open
nils-a opened this issue Oct 25, 2020 · 7 comments

Comments

@nils-a
Copy link

nils-a commented Oct 25, 2020

I'm not sure whether this is really a bug in the generator or rather a problem in the windows defaults of git, but:
The generator generates .gitattributes with content:

###############################################################################
# Set default behavior to automatically normalize line endings.
###############################################################################
* text=auto

*.cs text diff=csharp

*.ps1      text eol=crlf
*.bat      text eol=crlf
*.sln      text eol=crlf
*.csproj   text eol=crlf
tasks.json text eol=crlf

*.sh       text eol=lf

*.md       text whitespace=-trailing-space

# Exclude files from exporting

.gitattributes export-ignore
.gitignore     export-ignore

This leads to csproj being converted to crlf during checkout on non-windows systems (actually on windows-systems, too but here that's the default...)
At the same time, during checkin on windows (or rather on systems with core.autocrlf=true) those line-endings will be converted to lf,

Now, the problem starts when development is done on windows and linux (or on windows and having dependabot create PRs): While windows-systems will always checkin lf line-endings, the non-windows-systems will checkin crlf line-endings.

To demonstrate I have created a simple repository and let dependabot make PRs:

While I do see that this is also (maybe more so) the fault of the windows-default of setting core.autocrlf=true, may I ask: Is there a reason for setting eol=crlf? It does nothing on windows-systems and while it converts the files during checkout on non-windows-systems the files shuold have been created correctly in the first place. Or am I missing something?

@nils-a
Copy link
Author

nils-a commented Oct 25, 2020

reading up a lot on this topic, it seems that setting core.autocrlf overrides settings in .gitattributes.

So maybe all the generator should do is print out a warning when core.autocrlf is set to true.

@augustoproiete
Copy link

AFAIK (from this list) only .sln and .bat files are expected to have CRLF on any platform.

.csproj, .ps1, and .json (I believe) should have line-endings normalized by Git during checkout - thanks to * text=auto in the .gitattributes (provided they don't have a core.autocrlf override on the machine as you noted) - to work well on any environment.

* text=auto

*.cs text diff=csharp

- *.ps1      text eol=crlf
+ *.ps1      text
*.bat      text eol=crlf
*.sln      text eol=crlf
- *.csproj   text eol=crlf
+ *.csproj   text
- tasks.json text eol=crlf
+ tasks.json text

*.sh       text eol=lf

*.md       text whitespace=-trailing-space

# Exclude files from exporting

.gitattributes export-ignore
.gitignore     export-ignore

It might make sense to follow what the .NET team uses on their own repositories. Sampling a few repos, seems to confirm that only .sln requires CRLF on any OS:

@AdmiringWorm
Copy link
Member

@augustoproiete @nils-a

There is a reason why those files are forced as crlf:

  • For powershell scripts, on windows in needs to be crlf to work correctly (PS core can handle both), as such forcing it to use crlf makes sense due to the ability of setting core.autocrlf to input (which is the default on some build systems, appveyor do this for instance).
  • For csproj files, it was mainly added because I got tired of fighting with visual studio to change it (so yes, it was for my benefit since I typically have autocrlf set to input)
  • I found that visual studio code couldn't read tasks.json in some cases (both on Linux and windows) when unix line ending was read, so this was changed.

reading up a lot on this topic, it seems that setting core.autocrlf overrides settings in .gitattributes.

That isn't entirely correct.
If the gitattributes specifies the eol, that line ending will be used, autocrlf is taken into account when eol is set to auto.

@nils-a
Copy link
Author

nils-a commented Jan 5, 2021

If the gitattributes specifies the eol, that line ending will be used, autocrlf is taken into account when eol is set to auto.

Really? I was under the impression that eol (regardless of core.eol or defined per-path in .gitattributes) is overridden by core.autocrlf, if core.autocrlf is set to true or input.

The reason I created this issue was that for all projects created from this template I always have "problems" when dependabot updates a csproj.

  • for the a dependabot update, always the full file ist updated (not the one line containing the change)
  • after a pull on my system the file is marked modified immediately - without having touched it.

all that went away, when I removed core.autocrlf from my system.

@AdmiringWorm
Copy link
Member

The reason I created this issue was that for all projects created from this template I always have "problems" when dependabot updates a csproj.

for the a dependabot update, always the full file ist updated (not the one line containing the change)
after a pull on my system the file is marked modified immediately - without having touched it.

hmm, the only time I have seen that happen is when the files have had a commit before adding a gitattributes file, and thus not being normalized before they are changed again.

it is also possible that dependabot just respects the text=auto instead of the specific eol handling of the file extension.

@nils-a
Copy link
Author

nils-a commented Jan 5, 2021

It is also possible that dependabot just respects the text=auto instead of the specific eol handling of the file extension.

Not sure about that. As I said, the problem went away after I set core.autocrlf to false globally on my end.

But there's really no need to dwell on this. I though "everyone" had the same problem I had (until I set core.autocrlf to false) since you're using input and have no problems, seems there is not really a (global) problem. We could move ahead and close this issue.

@AdmiringWorm
Copy link
Member

Let us keep the issue open, it is something I would like to come back to in the future to investigate.

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

No branches or pull requests

3 participants