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

Windows - Line endings & conversion #135

Open
MSP-Greg opened this issue Jan 10, 2020 · 34 comments
Open

Windows - Line endings & conversion #135

MSP-Greg opened this issue Jan 10, 2020 · 34 comments
Labels
documentation Improvements or additions to documentation

Comments

@MSP-Greg
Copy link

Many repos, use a line ending of \n.

When using Windows, these seem to be converted. Often, this isn't an issue with actual code, but has messed up testing. The following seems to fix it, run before the checkout action:

git config --system core.autocrlf false
git config --system core.eol lf

If this could be an option with 'checkout', some might find it helpful...

@MSP-Greg MSP-Greg changed the title Line endings & conversion Windows - Line endings & conversion Jan 10, 2020
@ericsciple
Copy link
Contributor

would .gitattributes solve the problem?

@MSP-Greg
Copy link
Author

Yes. I use them in my repos.

But, it's a change for people migrating from AppVeyor (along with UTF8 console encoding).

Also, for repos that are predominately *nix software that support Windows, may have binary files, etc...

Close if you'd like, just thought I'd mention it.

@devsnek
Copy link

devsnek commented Jan 21, 2020

With the temp fix in the OP, I'm getting a permissions error about trying to write to /etc/gitconfig. Any suggestions?

@MSP-Greg
Copy link
Author

@devsnek

I apologize, I didn't state in the original post that I noticed the issue when working with ps1 or cmd shells. I've also had the issue locally. Also, locally, I almost never work in the Git bash (or MSYS2) shell.

If you're using a bash shell, maybe try:

git config --global ...

I don't know...

@lydell
Copy link

lydell commented Mar 22, 2020

This worked for me:

    steps:
      - name: Set git to use LF
        run: |
          git config --global core.autocrlf false
          git config --global core.eol lf

      - uses: actions/checkout@v2

@myitcv
Copy link

myitcv commented Apr 7, 2020

Related is https://travis-ci.community/t/files-in-checkout-have-eol-changed-from-lf-to-crlf/349.

Per my comment in https://travis-ci.community/t/files-in-checkout-have-eol-changed-from-lf-to-crlf/349/3, I don't think the default config of core.autocrlf=true on a runner is correct, because otherwise there’s a considerable amount of work required to undo this behaviour.

I would suggest making the default on all runners false and then allowing people to configure it to true as required (not that this should really ever be required).

@mvdan
Copy link

mvdan commented Apr 13, 2020

I raised the same issue on the forum last year, and while it did get an initial reply from @ethomson, it hasn't seen any response since.

@ethomson
Copy link
Contributor

If this could be an option with 'checkout', some might find it helpful...

Git already has a configuration option, it's the .gitattributes file, which has the added property of being universally supported by all systems, not just GitHub Actions.

Per my comment in https://travis-ci.community/t/files-in-checkout-have-eol-changed-from-lf-to-crlf/349/3, I don't think the default config of core.autocrlf=true on a runner is correct, because otherwise there’s a considerable amount of work required to undo this behaviour.

core.autocrlf's default is true because that's the default in Git for Windows.

Git for Windows installer options page

Most people use this unchanged, and so our runners are configured to match the most common setting. We see the majority of Windows users have configured this option. To change it to false would hurt most repositories that are missing a .gitattributes.

There's no "correct" choice here for users to make: it's purely a preference. And in the absence of correctness, we have to choose the option that supports the most users correctly.

But this is a guess. And we don't want to be in the business of guessing what to do about your line endings. We'd strongly prefer for us to tell us what you want to do. You can do that with a .gitattributes. This places the configuration about your repository where it belongs - in the repository itself.

To emulate the behavior of core.autocrlf=true, add a .gitattributes that is * text=auto. To emulate core.autocrlf=false, add a .gitattributes that is * -text. This will disable text translations (line ending changes).

More information is available in this blog post

@myitcv
Copy link

myitcv commented Apr 14, 2020

Thanks @ethomson. Doesn't setting * -text have the side effect that when files are added on Windows (for example) they will not have their line endings normalised to LF?

@ethomson
Copy link
Contributor

@myitcv That's correct - sorry, I did sort of gloss over the many combinations of settings that are possible. 😰

Thanks @ethomson. Doesn't setting * -text have the side effect that when files are added on Windows (for example) they will not have their line endings normalised to LF?

Yes - that's correct - * -text will prevent any line ending translation. This should map to core.autocrlf=false in the absence of a core.eol setting.

There is an interesting bit to the translation of core.autocrlf/core.eol and .gitattributes - core.autocrlf=false is required for core.eol to have any effect - on the other hand, the eol attribute implies the text attribute (not -text).

I don't think that I had really realized this seeming mismatch until you asked - it's not that core.autocrlf precisely means "do text translation" when true, you can set core.autocrlf=false and still see text translation, depending on the setting of core.eol.

I think that all the options for core.autocrlf plus core.eol are expressible with attributes.

Configuration: core.autocrlf=true
Attributes: * text=auto
Notes: core.eol cannot be used when core.autocrlf=true

Configuration: core.autocrlf=input
Attributes: * text eol=lf
Effect: Files on disk will are LF (all platforms), files in repository are LF. (This is equivalent to core.autocrlf=false with core.eol=lf.) core.eol cannot be used when core.autocrlf=true

Configuration: core.autocrlf=false
Attributes: * -text
Effect: Files on disk are whatever they were created in, checked in to the repository literally

Configuration: core.autocrlf=false core.eol=lf
Attributes: * text eol=lf
Effect: Files on disk are LF (all platforms), files in repository are LF

Configuration: core.autocrlf=false core.eol=crlf
Attributes: * text eol=crlf
Effect: Files on disk are CRLF (all platforms), files in repository are LF

Configuration: core.autocrlf=false core.eol=native
Attributes: * text=auto
Effect: Files on disk are CRLF (Windows), LF (everywhere else), files in repository are LF

@myitcv
Copy link

myitcv commented Apr 14, 2020

Thanks @ethomson

There's no "correct" choice here for users to make:

This statement is the key here, and to your earlier point it makes sense for the default behaviour to be that one which will be most useful to the majority.

@ericsciple ericsciple added the documentation Improvements or additions to documentation label Apr 15, 2020
@ericsciple
Copy link
Contributor

I'm going to add a troubleshooting doc. I'll add a section for this

owenthereal added a commit to jqlang/jq that referenced this issue Jun 1, 2023
* Try making some GitHub Actions

* GHA: Add OS X, Windows, and Linux w/ scan-build builds

* Add link to discord server

Add link to discord server

* Update Ubuntu to 22.04 for GitHub Actions

* Remove if condition for Ubuntu build

* Fix MacOS OS on GitHub Actions

* Fix python3 package on GitHub Actions

* Disalbe valgrind

Getting failed tests: https://github.com/jqlang/jq/actions/runs/5113866588/jobs/9193542140#step:9:341

* Format file

* Fix macos build

* Fix syntax errors of windows build

https://github.com/jqlang/jq/actions/runs/5114162556/workflow

* Install windows package with choco

* Pin oniguruma to 5a24a49d710a9e3bb8ff11d12e1eae5a9f9df40c

MacOS build [fails](https://github.com/jqlang/jq/actions/runs/5114162555/jobs/9194126677#step:6:3160) due to

```
posix.c:94:3: error: implicit declaration of function 'onig_end' is invalid in C99 [-Werror,-Wimplicit-function-declaration]
  onig_end();
```

The current `oniguruma` revision
6fa38f4084b448592888ed9ee43c6e90a46b5f5c (15 Mar 2017) lacks the following explicit declaration in src/onigposix.h:

```
ONIG_EXTERN int onig_end P_((void));
```

This was added to oniguruma in revision 5a24a49d710a9e3bb8ff11d12e1eae5a9f9df40c (8 Sep 2017).

Ref: #2381

* Revert windows build to use pacman

* Don't zip jq.exe

* Fixing windows build

* Update .gitattriutes for eol on Windows

* Skip workflow_dispatch for windows build

* Clean up builds

* Use LF line endings for all text

Ref: actions/checkout#135

* Fix test that has rounding error

#2596 (comment)

* Enable CI for all

---------

Co-authored-by: Nicolas Williams <nico@cryptonector.com>
kuoruan added a commit to kuoruan/vite-plugin-svg-combiner that referenced this issue Jul 31, 2023
mirabilos added a commit to MirBSD/int that referenced this issue Aug 12, 2023
cf. actions/checkout#135

(whyyyyyyyy‽ if I check out a repo I want it byte for byte!)
@ilovelinux
Copy link

Partially related to actions/runner#2071

LarrMarburger pushed a commit to LarrMarburger/privacy.sexy that referenced this issue Nov 16, 2023
It prevents Git from modifying files on checkout. By default, it
converts LF line-endings to CRLF on Windows which leads to inconsistency
and ESLint `linebreak-style` with `unix` (LF) value to fail.

It also solves failed builds in GitHub actions agents actions/checkout#135.
Michael-F-Bryan added a commit to Michael-F-Bryan/wit-lsp that referenced this issue Mar 25, 2024
@exFalso
Copy link

exFalso commented May 15, 2024

There's no "correct" choice here for users to make: it's purely a preference. And in the absence of correctness, we have to choose the option that supports the most users correctly.

I would have to disagree, it's very clearly wrong to set this to anything but false. It can cause strange reproducibility issues, merge issues, hash mismatches etc etc. This is even more so true for git actions which are commonly used to build release artifacts. I don't even understand why there is a setting for this in git, what does the line ending have to do with revision control?

Also, just to demonstrate, see the list of issues linking this issue. And these are just the public issues.

@MSP-Greg
Copy link
Author

Haven't looked at this for a while. Much of the discussion seems to assume changing checkout's defaults.

What I should have mentioned is something like adding an input option like win-force-lf. Don't change the defaults, but allow an easy fix that doesn't require knowledge of git config or access to a Windows system.

I have often helped repos that have no maintainers/owners that use Windows...

@exFalso
Copy link

exFalso commented May 15, 2024

It's not the default! The default is false (see here), and the windows installer (which is installing on a dev machine) picks a different default. Why is the installer used as a baseline for a headless github action?

@ethomson
Copy link
Contributor

You shouldn't be using core.autocrlf. Line ending configuration is per repository - the configuration affects not only how things are placed on disk but also in the repository, so to rely on core.autocrlf would require every user of the repository to set it to the same value on their local machine.

This is impractical, as this discussion shows. Check in your line ending configuration with gitattributes. This ensures that all developers have the same settings for each repository they use.

@exFalso
Copy link

exFalso commented May 15, 2024

We have zero developers using windows. The only reason we ended up at this issue is because we had a nasty reproducibility bug that affected customers using windows, as our build artifacts had these random carriage returns due to this setting. It's infuriating, because this setting literally doesn't make sense in a github action context. In what situation is it useful? Do people open windows code editors in the git checkout of the action run?

@ethomson
Copy link
Contributor

The only reason we ended up at this issue is because we had a nasty reproducibility bug that affected customers using windows, as our build artifacts had these random carriage returns due to this setting.

Then check in a gitattributes file so that that doesn't happen.

It's infuriating, because this setting literally doesn't make sense in a github action context. In what situation is it useful?

When you have 100% of developers using Windows using an actions workflow and they have not bothered to correctly configure gitattributes.

(Which is more likely? 100% of developers are writing code on Linux and are doing a build on Windows, or 100% of developers are writing code on Windows and then doing their build on Windows? It's the latter.)

There is no effective way to communicate what should be done to line endings except to use gitattributes.

core.autocrlf is an unmitigated disaster that should have never existed. git and Git for Windows give it different defaults and it's configured in the wrong place. gitattributes is the correct way to tell git how to behave in a cross platform environment.

@exFalso
Copy link

exFalso commented May 15, 2024

Agreed, the option's existence itself is bizarre.

But also, even if you have only windows developers, how does this setting help in a GitHub Action context? Those windows developers used the default installer setting during development, fine, but in what circumstance should the Action itself set it? The only possible scenario I can imagine is if the Action itself somehow modified files in the repository and pushed the changes.. am I missing something? Isn't the only reasonable behaviour for headless contexts to leave the characters as-is?

@MSP-Greg
Copy link
Author

I wasn't clear when I opened this issue. I'm not referring to commits. For example, I've seen instances where CI was testing the construction of a string, and it was compared to the contents of a test file 'fixture'. This may lead to failures due to EOL settings.

I've got no problem with the current defaults, maybe a better input option would be force-eol, with options of crlf or lf?

@ethomson
Copy link
Contributor

But also, even if you have only windows developers, how does this setting help in a GitHub Action context? Those windows developers used the default installer setting during development, fine, but in what circumstance should the Action itself set it? The only possible scenario I can imagine is if the Action itself somehow modified files in the repository and pushed the changes.. am I missing something? Isn't the only reasonable behaviour for headless contexts to leave the characters as-is?

core.autocrlf doesn't just control the data going into a repository (ie, adding to the index / committing), it also controls the data coming out. Assume that you have some developer who has installed Git for Windows, been (understandably) baffled by the multiple pages of dense configuration options you're prompted to deal with, and just (again, understandably) accepts the defaults. Now they have core.autocrlf = true set. Meaning that when they run git add, git will turn the CRLFs in that file into LFs.

Now, assume that you're a developer (or a CI system) that has core.autocrlf = false set (whether implicitly or explicitly). When you check that repository out, you'll get... LFs on disk. And, of course, for most applications this won't matter. In 2024, even notepad.exe can read files with CRLF. But cmd scripts still need to be \r\n, and I'm sure there are others. (Because meaningfully, the files aren't the same as what the original developer checked in.)

(Don't get me started about files with mixed line endings, that's just a whole separate problem.)

But anyway, core.autocrlf has to match the setting for every developer in a repository — whether they're creating or merely consuming content from it. This would be bad enough, except that this is usually set as a user-wide setting, which means that now every repository that a user uses also has to have the same settings.

(There's nothing good about this.)

The way around this is to put a gitattributes file in every repository you deal with — with the possible exception, I guess, maybe, of ones that will never, ever, never, absolutely never, you're really sure, be used by a Windows user, and even then, this problem is gross and thorny and terrible enough that I would do it anyway, just to make sure.

@exFalso
Copy link

exFalso commented May 15, 2024

cmd scripts. I have not considered that. My point was specifically that in a CI context there is no notepad.exe or vscode or similar being executed, but cmd scripts.. fair enough.

Re gitattributes: yes of course this works, but nobody's going to set this before encountering a related bug first.. I didn't even know about the existence of this option yesterday. There's just no good solution. Peace be upon all devs who encounter this mess.

Thank you for your patience

@kriegaex
Copy link

kriegaex commented May 16, 2024

I am baffled, watching you guys philosophise here after the 4+ years the issue has been open. I just checked, I commenterd myself in 2021 already: #135 (comment).

Just count the sheer number of related issues linking to or mentioning this one. By insisting that all projects world-wide should use .gitattributes instead of using a simple config option for this action, the problem does not go away. Reality is imperfect. Are you really expecting that millions of projects which did fine without .gitattributes so far will take the trouble to add such configuration just to avoid problems with a GitHub action? Sorry, that seems a wee bit unrealistic wishful thinking.

Look at the effort you expended debating this. Would it not have been better invested in just fulfilling this users' wish? I would think that the implementation would not be rocket science, if not trivial.

@ethomson
Copy link
Contributor

ethomson commented May 16, 2024

It's not philosophical, it's practical.

Just count the sheer number of related issues linking to or mentioning this one. By insisting that all projects world-wide should use .gitattributes instead of using a simple config option for this action, the problem does not go away.

Every project world-wide that is touched by Windows users should use a .gitattributes file, period. That's how you define how line endings work on Git for Windows. Not just for this action, for all the Windows machines.

And yes - the problem does go away if you use .gitattributes. You could argue that making people configure .gitattributes is impractical (and that's a reasonable argument) but it does solve the problem at hand of distributing line-ending configuration about a repository in-band.

Reality is imperfect.

I agree — reality is imperfect. And git is imperfect. Git has saddled you with their historical misunderstanding of Windows, and how line endings should work in a version control system. They've moved the goalposts to requiring you to have a .gitattributes file, and then not actually told you this, sticking their fingers in their ears and pretending that there's no problem. There is a problem.

But — in reality, the solution to this problem is .gitattributes. This isn't philosophical. It's practical.

Are you really expecting that millions of projects which did fine without .gitattributes so far will take the trouble to add such configuration just to avoid problems with a GitHub action? Sorry, that seems a wee bit unrealistic wishful thinking.

They didn't do fine — that's why this issue is open. As you mentioned upthread:

For example, I use "check out as-is, commit Unix-style line endings" whenever I install Git for Windows

Okay, but how is this action supposed to know that? How is your colleague who clones this repo supposed to know that? What happens when you use a different git installation that looks in a different place for the configuration (Portable Git). What happens when you're accidentally in an Administrator terminal?

Yes, the answer to "how is this actions supposed to know" could be "we'll configure it in the YAML". Or it could be "we'll configure it in .gitattributes for this action, and every other thing that touches this repository".

Look at the effort you expended debating this. Would it not have been better invested in just fulfilling this users' wish? I would think that the implementation would not be rocket science, if not trivial.

I'm not sure who you're talking to — me? I used to work at GItHub, but I don't anymore. Now I'm just a helpful bystander who works on cross-platform git tooling and wants you to be successful despite git's obvious shortcomings in this area.

If you want to have an option for this in the action, it's obvious that GitHub isn't going to do anything here on their own, seeing as this issue is 4.5 years old. Someone will need to take it into their own hands to send a PR.

But my suggestion here is: don't. By making a change to this action, you're just putting your finger in the levee. You're going to keep running to every place that checks out a file, whether that's the other people that you write code with, or the build and deploy tools that interact with git. And you're going to keep telling them "SET core.autocrlf THIS WAY! NOTHING WORKS UNLESS YOU SET core.autocrlf JUST LIKE I DO!"

Or you can just check in a .gitattributes file and get on with your life, the way git wants you to (even if it has done a poor job of telling you that).

Viir added a commit to Arcitectus/Sanderling that referenced this issue May 26, 2024
Following <pine-vm/pine@08bf4cd>
Avoid git modifying files on checkout in Azure Pipelines Windows Environment: Apply fix described at <actions/checkout#135 (comment)>
Viir added a commit to Viir/bots that referenced this issue May 26, 2024
Following <pine-vm/pine@08bf4cd>
Avoid git modifying files on checkout in Azure Pipelines Windows Environment: Apply fix described at <actions/checkout#135 (comment)>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

No branches or pull requests