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

cmd/go: document Git setup to preserve go.mod line endings on Windows #31870

Closed
bruceadowns opened this issue May 6, 2019 · 33 comments
Closed

cmd/go: document Git setup to preserve go.mod line endings on Windows #31870

bruceadowns opened this issue May 6, 2019 · 33 comments

Comments

@bruceadowns
Copy link

@bruceadowns bruceadowns commented May 6, 2019

What version of Go are you using (go version)?

> go version
go version go1.12.4 windows/amd64

Does this issue reproduce with the latest release?

yes

What operating system and processor architecture are you using (go env)?

go env Output
> go env
set GOARCH=amd64
set GOBIN=
set GOCACHE=C:\Users\Bruce Downs\AppData\Local\go-build
set GOEXE=.exe
set GOFLAGS=
set GOHOSTARCH=amd64
set GOHOSTOS=windows
set GOOS=windows
set GOPATH=C:\Users\Bruce Downs\go
set GOPROXY=
set GORACE=
set GOROOT=c:\go
set GOTMPDIR=
set GOTOOLDIR=c:\go\pkg\tool\windows_amd64
set GCCGO=gccgo
set CC=gcc
set CXX=g++
set CGO_ENABLED=1
set GOMOD=C:\Users\Bruce Downs\dev\acmeperf\go.mod
set CGO_CFLAGS=-g -O2
set CGO_CPPFLAGS=
set CGO_CXXFLAGS=-g -O2
set CGO_FFLAGS=-g -O2
set CGO_LDFLAGS=-g -O2
set PKG_CONFIG=pkg-config
set GOGCCFLAGS=-m64 -mthreads -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=C:\Users\BRUCED~1\AppData\Local\Temp\go-build607182278=/tmp/go-build -gno-record-gcc-switches

What did you do?

  1. clone repo on windows
  2. run go build
  3. note go.mod CRLF changed to LF

What did you expect to see?

That go.mod line endings do not change.

What did you see instead?

That changing the line endings of go.mod creates an artificial difference.

@bradfitz

This comment has been minimized.

Copy link
Member

@bradfitz bradfitz commented May 6, 2019

gofmt does the same thing, which we've repeatedly agreed is fine (#16355, etc). I see no reason why we'd decide on a different policy for go.mod files.

@bruceadowns

This comment has been minimized.

Copy link
Author

@bruceadowns bruceadowns commented May 6, 2019

Ah ok, I see this is well-debated territory. My initial reaction if I were a windows end-user is to complain and disagree, but I will try and digest the referenced issue.

@bradfitz

This comment has been minimized.

Copy link
Member

@bradfitz bradfitz commented May 6, 2019

Actually, there is one difference from the gofmt issue: we never ran gofmt for you automatically, but now that we're in the business of automatically rewriting go.mod files all the time, people can see a change in their git status after doing a go build etc.

I'm not sure that changes my opinion, but it is a difference.

/cc @rsc @bcmills

@bradfitz bradfitz changed the title Line endings of go.mod are re-written from CRLF to LF on windows cmd/go: go.mod line endings re-written from CRLF to LF on windows May 6, 2019
@bradfitz bradfitz added this to the Go1.13 milestone May 6, 2019
@bruceadowns

This comment has been minimized.

Copy link
Author

@bruceadowns bruceadowns commented May 7, 2019

Here's a transcript of

  • pulling code via git.exe
  • running gofmt.exe
  • running goimports.exe
  • running go.exe test

and viewing the inconsequential diffs.

Windows PowerShell
Copyright (C) Microsoft Corporation. All rights reserved.

PS C:\Users\Bruce Downs\dev> git.exe clone https://github.com/bruceadowns/gomodeol
Cloning into 'gomodeol'...
remote: Enumerating objects: 12, done.
remote: Counting objects: 100% (12/12), done.
remote: Compressing objects: 100% (9/9), done.
remote: Total 12 (delta 2), reused 12 (delta 2), pack-reused 0
Unpacking objects: 100% (12/12), done.

PS C:\Users\Bruce Downs\dev> cd .\gomodeol\

PS C:\Users\Bruce Downs\dev\gomodeol> gofmt.exe -w .\hello.go
PS C:\Users\Bruce Downs\dev\gomodeol> goimports.exe -w .\hello_test.go
PS C:\Users\Bruce Downs\dev\gomodeol> go.exe test
PASS
ok      github.com/bruceadowns/gomodeol 0.281s

PS C:\Users\Bruce Downs\dev\gomodeol> git.exe status
On branch master
Your branch is up to date with 'origin/master'.

Changes not staged for commit:
  (use "git add <file>..." to update what will be committed)
  (use "git checkout -- <file>..." to discard changes in working directory)

        modified:   go.mod
        modified:   hello.go
        modified:   hello_test.go

no changes added to commit (use "git add" and/or "git commit -a")

PS C:\Users\Bruce Downs\dev\gomodeol> git.exe diff
warning: LF will be replaced by CRLF in go.mod.
The file will have its original line endings in your working directory.
warning: LF will be replaced by CRLF in hello.go.
The file will have its original line endings in your working directory.
warning: LF will be replaced by CRLF in hello_test.go.
The file will have its original line endings in your working directory.
@bruceadowns

This comment has been minimized.

Copy link
Author

@bruceadowns bruceadowns commented May 7, 2019

Also, if it weren't obvious enough, the degenerate side effect in vscode.

This is not a lesser evil isolated to notepad.exe.

image

@bcmills

This comment has been minimized.

Copy link
Member

@bcmills bcmills commented May 7, 2019

@bruceadowns, since the standard for Go tools is newline-only line endings, and even Notepad now supports that standard, it seems like the cleanest solution is to configure your Git client not to transform the line-endings to CRLF in the first place.

You can configure your Git client to avoid spurious diffs by setting core.autocrlf=false, then either setting the eol=lf attribute for the go.mod file, or setting core.eol=lf for the repository as a whole. That should transform line endings to \n on checkin, and avoid transforming line endings to \r\n on checkout.

@bcmills bcmills closed this May 7, 2019
@bruceadowns

This comment has been minimized.

Copy link
Author

@bruceadowns bruceadowns commented May 7, 2019

Ok, that seems like the most expedient solution for the go team.

Though, I experimented with the noted settings and got the noted spurious results.

Step 1 - Override git default eol settings on Windows

PS C:\Users\Bruce Downs\dev\gomodeol> git remote -v
origin  https://github.com/bruceadowns/gomodeol (fetch)
origin  https://github.com/bruceadowns/gomodeol (push)

PS C:\Users\Bruce Downs\dev\gomodeol> git config --get core.autocrlf
false

PS C:\Users\Bruce Downs\dev\gomodeol> git config --get core.eol
lf

Step 2 - Add new files using vscode

PS C:\Users\Bruce Downs\dev\gomodeol> git status
On branch master
Your branch is up to date with 'origin/master'.

Changes to be committed:
  (use "git reset HEAD <file>..." to unstage)

        new file:   glass.go
        new file:   glass_test.go

PS C:\Users\Bruce Downs\dev\gomodeol> git diff HEAD
diff --git a/glass.go b/glass.go
new file mode 100644
index 0000000..4b2639e
--- /dev/null
+++ b/glass.go
@@ -0,0 +1,8 @@
+package gomodeol^M
+^M
+import "rsc.io/quote"^M
+^M
+// Glass ...^M
+func Glass() string {^M
+       return quote.Glass()^M
+}^M
diff --git a/glass_test.go b/glass_test.go
new file mode 100644
index 0000000..c151d69
--- /dev/null
+++ b/glass_test.go
@@ -0,0 +1,10 @@
+package gomodeol^M
+^M
+import "testing"^M
+^M
+func TestGlass(t *testing.T) {^M
+       want := "I can eat glass and it doesn't hurt me."^M
+       if got := Glass(); got != want {^M
+               t.Errorf("Glass() = %q, want %q", got, want)^M
+       }^M
+}^M

PS C:\Users\Bruce Downs\dev\gomodeol> git push
Counting objects: 4, done.
Delta compression using up to 2 threads.
Compressing objects: 100% (4/4), done.
Writing objects: 100% (4/4), 563 bytes | 281.00 KiB/s, done.
Total 4 (delta 1), reused 0 (delta 0)
remote: Resolving deltas: 100% (1/1), completed with 1 local object.
To https://github.com/bruceadowns/gomodeol
   f0b9eab..ba89b60  master -> master

Step 3 - View files on Linux

downsb@ubuntudev:~/dev$ git clone https://github.com/bruceadowns/gomodeol.git
Cloning into 'gomodeol'...
cd goremote: Enumerating objects: 13, done.
remote: Counting objects: 100% (13/13), done.
remote: Compressing objects: 100% (11/11), done.
remote: Total 13 (delta 1), reused 13 (delta 1), pack-reused 0
Unpacking objects: 100% (13/13), done.

downsb@ubuntudev:~/dev$ file gomodeol/*
gomodeol/glass.go:      ASCII text, with CRLF line terminators
gomodeol/glass_test.go: ASCII text, with CRLF line terminators
gomodeol/go.mod:        ASCII text
gomodeol/go.sum:        ASCII text
gomodeol/hello.go:      ASCII text
gomodeol/hello_test.go: ASCII text
gomodeol/Makefile:      makefile script, ASCII text
gomodeol/README.md:     ASCII text
@bruceadowns

This comment has been minimized.

Copy link
Author

@bruceadowns bruceadowns commented May 7, 2019

Thanks for the info @bcmills!

I tried the git client settings you mentioned (core.autocrlf and core.eol), but do not get positive results.

@bcmills

This comment has been minimized.

Copy link
Member

@bcmills bcmills commented May 7, 2019

@bruceadowns, I generally assume that folks have some sort of format on save hook configured.

Beyond that, I'm not sure why git isn't respecting core.eol for you. Perhaps it only affects transformation of existing contents, not conversion of new files?

@bruceadowns

This comment has been minimized.

Copy link
Author

@bruceadowns bruceadowns commented May 7, 2019

I'm not sure why git isn't respecting core.eol for you. Perhaps it only affects transformation of existing contents, not conversion of new files?

I suspect some client setting is out of alignment, and that will generally be the case for this overarching issue.

With that said, I suspect it will not be difficult finding degenerate workflows for Windows, and subsequent ham-handed solutions to fix. This is why countless projects (scm and otherwise) have historically supported native line endings to cater to the enterprise Windows customer.

@alexbrainman

This comment has been minimized.

Copy link
Member

@alexbrainman alexbrainman commented Jun 3, 2019

it seems like the cleanest solution is to configure your Git client not to transform the line-endings to CRLF in the first place.

It is cleanest solution for us Go tool developers, because we don't need to do anything. But it puts burden on every Go user that encounters this problem. They would have to deal with the problem one way or the other.

Imagine if we decided to use CRLF, not LF as line delimiters in go.mod. We would have plenty of people complaining about the decision. And looking for solutions.

I think, this at least needs to be documented somewhere. Maybe in go help modules? And not just Git setup. What if people use other tools to maintain their code?

Also should your suggested Git settings be local to repo or global for the whole computer? What if another development tool supplier recommends the opposite of what you recommend?

Maybe we could just leave go.mod file as is (keep whatever line delimiter file has)?

Alex

@alexbrainman

This comment has been minimized.

Copy link
Member

@alexbrainman alexbrainman commented Jun 15, 2019

@bcmills can you, please, answer my questions here #31870 (comment)

I will reopen this issue, so we don't forget. Perhaps something can be done about this before go1.13 is released.

Thank you.

Alex

@alexbrainman alexbrainman reopened this Jun 15, 2019
@iWdGo

This comment has been minimized.

Copy link
Contributor

@iWdGo iWdGo commented Jun 16, 2019

Similar issue using Goland (Idea) on Windows 10.

>ver

Microsoft Windows [Version 10.0.17134.829]

>git version
git version 2.20.1.windows.1

Using core.autocrlf=true with core.eol unset, everything is fine on the Linux cloud server,

(repo)/src $ git config -l  | grep crlf
(repo)/src $ file go.mod 
go.mod: ASCII text

After another round of reading, nothing new.
The issue is unrelated to Goland (Idea) which issues specific git commands with unwanted effects on some file formats. There is a short discussion on golang-nuts.

The only issue that I can't solve is the reading of EOF. I suspect that the last CR triggers io.EOF but the last LF is also returned. This is confusing go version go1.12.6 windows/amd64.

@wuyumin

This comment has been minimized.

Copy link

@wuyumin wuyumin commented Jun 20, 2019

add .gitattributes file in your project root directory like this:

go.mod text eol=lf

@bruceadowns

@iWdGo

This comment has been minimized.

Copy link
Contributor

@iWdGo iWdGo commented Jun 26, 2019

Issue cannot be reproduce including EOF with

>git version
git version 2.22.0.windows.1
@bcmills bcmills changed the title cmd/go: go.mod line endings re-written from CRLF to LF on windows cmd/go: document Git setup to preserve go.mod line endings on Windows Jun 28, 2019
@bcmills bcmills added NeedsFix and removed NeedsDecision labels Jun 28, 2019
@andybons andybons modified the milestones: Go1.13, Go1.14 Jul 8, 2019
@tpaschalis

This comment has been minimized.

Copy link

@tpaschalis tpaschalis commented Sep 26, 2019

I'm looking for a first-time issue, so this seems safe to start with.
Where would this be best documented?

I think it could be a FAQ on https://github.com/golang/go/wiki/Modules

@alexbrainman

This comment has been minimized.

Copy link
Member

@alexbrainman alexbrainman commented Sep 26, 2019

Where would this be best documented?

I would still try and just fix the issue instead. Things should just work. And proposed resolution of making Go users adjust their Git settings will not scale (see https://devblogs.microsoft.com/oldnewthing/20081211-00/?p=19873 for why this proposal is wrong).

I would try and do what I suggested in #31870 (comment)

Maybe we could just leave go.mod file as is (keep whatever line delimiter file has)?

Alex

@tpaschalis

This comment has been minimized.

Copy link

@tpaschalis tpaschalis commented Sep 30, 2019

Sorry for the late response. I understand your point of view, and I'm by no means qualified to take a decision, as I've seen you've debated in other similar issues as well.

I agree that making Go users adjust their Git settings might not be the best solution, but my 2c in regards to why standardization to LF could make sense :
a) go.mod would not be manually edited, rather than rewritten automatically right?
b) any changes in go.mod would run through gofmt as well
c) the change is going to be reported in Git only once, after the initial transformation to LF, most editors would recognize the file having LF and keep working with it this way.

The only editor currently without LF support seems to be Notepad, and I don't know if this should drive the decision.

@alexbrainman

This comment has been minimized.

Copy link
Member

@alexbrainman alexbrainman commented Oct 1, 2019

I agree that making Go users adjust their Git settings might not be the best solution, but my 2c in regards to why standardization to LF could make sense :

I have nothing against making LF standard line ending. But how do you propose to enforce this rule? Do you expect users, who struggle with this issue, somehow magically find correspondent help text at https://github.com/golang/go/wiki/Modules and adjust their Git settings ? What if their company (or open source project) rules recommend different Git settings - settings that contradict yours?

a) go.mod would not be manually edited, rather than rewritten automatically right?

Normally go.mod will be updated by Go tools. But it can also be updated by Git or editor program.

b) any changes in go.mod would run through gofmt as well

I don't think gofmt touches go.mod file.

c) the change is going to be reported in Git only once, after the initial transformation to LF, most editors would recognize the file having LF and keep working with it this way.

I am not worried about editors. I agree that most editors will handle files with whatever line endings just fine.

I am worried about Git complaining that go.mod has changed after go build command, while nothing is changed.

c:\Users\alex\src>git clone C:\a
Cloning into 'a'...
Receiving objects: 100% (6/6), done.
Resolving deltas: 100% (1/1), done.

c:\Users\alex\src>cd a

c:\Users\alex\src\a>git status
On branch master
Your branch is up to date with 'origin/master'.

nothing to commit, working tree clean

c:\Users\alex\src\a>go build -v -o nul

c:\Users\alex\src\a>git status
On branch master
Your branch is up to date with 'origin/master'.

Changes not staged for commit:
  (use "git add <file>..." to update what will be committed)
  (use "git checkout -- <file>..." to discard changes in working directory)

        modified: go.mod

no changes added to commit (use "git add" and/or "git commit -a")

c:\Users\alex\src\a>git diff
warning: LF will be replaced by CRLF in go.mod.
The file will have its original line endings in your working directory.

c:\Users\alex\src\a>

This is how most common Git installation behaves on Windows (I selected Checkout Windows-style, commit Unix-style line ending when installing Git - https://www.edwardthomson.com/blog/git_for_windows_line_endings.html )

c:\Users\alex\src\a>git config --get core.autocrlf
true

I propose we use LF in go.mod, but avoid rewriting CRLF into LF unless there are other non-trivial changes. This will stop triggering confusing Git warnings when building Go code.

The only editor currently without LF support seems to be Notepad, and I don't know if this should drive the decision.

I agree. We should not worry about what editors support.

Alex

@bradfitz

This comment has been minimized.

Copy link
Member

@bradfitz bradfitz commented Oct 1, 2019

I propose we use LF in go.mod, but avoid rewriting CRLF into LF unless there are other non-trivial changes. This will stop triggering confusing Git warnings when building Go code.

That sounds reasonable.

@tpaschalis

This comment has been minimized.

Copy link

@tpaschalis tpaschalis commented Oct 1, 2019

Sounds good to me, too.

I'm not very familiar with the codebase, so it would take me a few days, but with some help I could try to implement a first approach.

@bcmills

This comment has been minimized.

Copy link
Member

@bcmills bcmills commented Oct 1, 2019

avoid rewriting CRLF into LF unless there are other non-trivial changes.

To be clear: we should avoid rewriting the go.mod file if there are only trivial changes in general, not just to preserve line endings.

@bruceadowns

This comment has been minimized.

Copy link
Author

@bruceadowns bruceadowns commented Oct 1, 2019

avoid rewriting CRLF into LF unless there are other non-trivial changes.

How do you categorize a trivial vs non-trivial go.mod change? (i.e. what is the criteria)

@bradfitz

This comment has been minimized.

Copy link
Member

@bradfitz bradfitz commented Oct 1, 2019

I interpreted "non-trivial" to mean semantically meaningful. Not whitespace or comments or line order.

@bcmills

This comment has been minimized.

Copy link
Member

@bcmills bcmills commented Oct 1, 2019

Comments are arguably non-trivial. Whitespace and line order are certainly trivial.

@alexbrainman

This comment has been minimized.

Copy link
Member

@alexbrainman alexbrainman commented Oct 2, 2019

I'm not very familiar with the codebase, so it would take me a few days, but with some help I could try to implement a first approach.

Please, have a go. Feel free to ask for help if you get stuck.

Comments are arguably non-trivial. Whitespace and line order are certainly trivial.

I agree with that. Solution does not need to be perfect. Something quick and simple should be good enough.

Alex

@bruceadowns

This comment has been minimized.

Copy link
Author

@bruceadowns bruceadowns commented Oct 6, 2019

Warning... unpopular opinion alert...

The Go standard tooling should support OS native line endings when rewriting source files. This includes, but is not limited to, gofmt.exe, goimports.exe, and go.exe modules. All tools should come with CR, LF, CRLF, and AUTO end-of-line modes, thus eliminating the need for developers to contort their environment settings.

I understand that this is an unfashionably banal debate, but consider that all major version control systems including git, svn, bazaar, and perforce support the ability to transmogrify line endings. And I presume these vendors chose to support it for good reason, regardless of intent.

Though windows is no longer my primary target, the support for native line endings made my past life much easier. Everything just worked. Compare that positivity with the ham-handed solutions noted in previous comments. This hodgepodge of environmental workarounds will continue to frustrate until fully addressed.

One simple proposal:

  1. add GOEOL environmental variable for go.exe (go.mod, go.sum)
  2. add -eol flag to gofmt.exe and goimports.exe

These flags may be set to [CR, LF, CRLF, AUTO] as necessary and would default to AUTO.

If my argument persuades, and the proposal seems worthy, I am willing to submit PRs against https://go.googlesource.com/go and https://go.googlesource.com/tools for consideration. If not, the argument is left for posterity.

@alexbrainman

This comment has been minimized.

Copy link
Member

@alexbrainman alexbrainman commented Oct 7, 2019

The Go standard tooling should support OS native line endings when rewriting source files.

This goes beyond what this issue is about.

If you want to pursue this further, I suggest you open new issue / proposal. See https://github.com/golang/proposal for details.

Thank you.

Alex

@bruceadowns

This comment has been minimized.

Copy link
Author

@bruceadowns bruceadowns commented Oct 7, 2019

The Go standard tooling should support OS native line endings when rewriting source files.

This goes beyond what this issue is about.

If you want to pursue this further, I suggest you open new issue / proposal. See https://github.com/golang/proposal for details.

Understood. I will consider a new proposal iff there are significant upvotes.

@gopherbot

This comment has been minimized.

Copy link

@gopherbot gopherbot commented Nov 4, 2019

Change https://golang.org/cl/204878 mentions this issue: cmd/go: make commands other than 'tidy' prune go.mod less agressively

gopherbot pushed a commit that referenced this issue Nov 6, 2019
Updates #31870
Updates #33326
Fixes #34822

Change-Id: I1337f171133c20800eacc6b0955ede5a394ea7eb
Reviewed-on: https://go-review.googlesource.com/c/go/+/204878
Run-TryBot: Bryan C. Mills <bcmills@google.com>
Reviewed-by: Jay Conrod <jayconrod@google.com>
@bcmills

This comment has been minimized.

Copy link
Member

@bcmills bcmills commented Nov 6, 2019

I propose we use LF in go.mod, but avoid rewriting CRLF into LF unless there are other non-trivial changes. This will stop triggering confusing Git warnings when building Go code.

This is now implemented at head. Is there anything remaining to be done for this issue?

@bradfitz

This comment has been minimized.

Copy link
Member

@bradfitz bradfitz commented Nov 6, 2019

That's probably sufficient.

@bcmills bcmills closed this Nov 6, 2019
@alexbrainman

This comment has been minimized.

Copy link
Member

@alexbrainman alexbrainman commented Nov 7, 2019

This is now implemented at head. Is there anything remaining to be done for this issue?

No. All looks good here. Thank you very much.

Alex

havoc-io added a commit to mutagen-io/mutagen that referenced this issue Dec 2, 2019
It seems that the Go toolchain wants to normalize go.mod's line endings
even if there are no changes to the file (golang/go#31870). This
normalization is disallowed by the -mod=readonly build flag, causing
builds to fail. This is likely fixed by golang/go@cf3be9b, but that
won't land until Go 1.14, and in any event the line ending normalization
will still occur if there are changes to go.mod/go.sum generated on
Windows.

Signed-off-by: Jacob Howard <jacob@havoc.io>
thrasher- added a commit to thrasher-corp/gocryptotrader that referenced this issue Dec 3, 2019
Saves having Windows users experience pain and suffering as documented
here:

golang/go#31870
thrasher- added a commit to thrasher-corp/gocryptotrader that referenced this issue Dec 3, 2019
Saves having Windows users experience pain and suffering as documented
here:

golang/go#31870
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
You can’t perform that action at this time.