Repository navigation
doc: document changes in behavior of gofmt in 1.11 #26228
Copy link
Copy link
Closed
Labels
DocumentationIssues describing a change to documentation.Issues describing a change to documentation.FrozenDueToAgeNeedsFixThe path to resolution is known, but the work has not been done.The path to resolution is known, but the work has not been done.
Milestone
Description
Please answer these questions before submitting your issue. Thanks!
What version of Go are you using (go version)?
go version go1.11beta1 linux/amd64
Does this issue reproduce with the latest release?
yes, latest beta
What operating system and processor architecture are you using (go env)?
GOARCH="amd64"
GOBIN=""
GOCACHE="/root/.cache/go-build"
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/go"
GOPROXY=""
GORACE=""
GOROOT="/usr/local/go"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/linux_amd64"
GCCGO="gccgo"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build926389312=/tmp/go-build -gno-record-gcc-switches"
VGOMODROOT=""
What did you do?
Running gofmt in Go 1.11beta1 causes a diff 😄
To reproduce;
- Create a file
example.go, with the following content;
package main
func main() {
_ = map[string]string{
"a": "",
"123456789012345678": "",
}
_ = map[string]string{
"a": "",
"1234567890123456789": "",
}
}gofmtthe file;
gofmt -s -d ./example.go What did you expect to see?
On Go 1.10.3, only the first map is re-formatted (the threshold for aligning values lies at 19 characters)
--- example.go.orig 2018-07-05 10:23:56.358365143 +0000
+++ example.go 2018-07-05 10:23:56.358365143 +0000
@@ -2,7 +2,7 @@
func main() {
_ = map[string]string{
- "a": "",
+ "a": "",
"123456789012345678": "",
}
_ = map[string]string{What did you see instead?
On Go 1.11beta1, both maps are re-formatted (the threshold for aligning values lies at 18 characters):
--- example.go.orig 2018-07-05 10:24:32.709844185 +0000
+++ example.go 2018-07-05 10:24:32.709844185 +0000
@@ -2,11 +2,11 @@
func main() {
_ = map[string]string{
- "a": "",
+ "a": "",
"123456789012345678": "",
}
_ = map[string]string{
- "a": "",
+ "a": "",
"1234567890123456789": "",
}
}This change complicates projects that test against multiple versions of Go (see related issues moby/moby#37358, containerd/containerd#2436)
If this was a deliberate change, please add a mention of this in the changelog 🤗
Reactions are currently unavailable
Metadata
Metadata
Assignees
Labels
DocumentationIssues describing a change to documentation.Issues describing a change to documentation.FrozenDueToAgeNeedsFixThe path to resolution is known, but the work has not been done.The path to resolution is known, but the work has not been done.
Activity
mvdan commentedon Jul 5, 2018
This change was done on purpose in 542ea5a.
As to why gofmt changes every release - see @griesemer's comment in #25161 (comment).
However, I agree that this change should be documented in the release notes. I can't see it in https://tip.golang.org/doc/go1.11.
[-]gofmt: threshold for aligning values changed in Go 1.11beta1[/-][+]doc: document changes in behavior of gofmt in 1.11[/+]thaJeztah commentedon Jul 5, 2018
Oh! actually found that commit, but see I forgot to include it in my description (thanks!)
hm, alright, so I understand Go should not stop progression; just thinking what the best approach is for projects that test against, and support multiple go versions;
gofmtcheck on version Xmvdan commentedon Jul 5, 2018
Yes - if a project wants to enforce
gofmtvia CI, it should stick to exactly one version ofgofmt. That tends to be the latest stable version, such as 1.10 now.The other option is to not enforce it via CI, and instead enforce it via pre-commit checks and by humans, like Go itself does.
thaJeztah commentedon Jul 5, 2018
True; just that, pre-commit checks won't resolve accepting contributions from users running different versions of go. In an ideal world, every project would switch to (and be compatible with) latest stable Go the moment it's released. Although upgrading Go versions has become less painful over the years, but we're not there yet, so unfortunately this isn't always possible (hence, contributors pinning to different versions of Go).
Thanks for your suggestions; it's appreciated 👍
mvdan commentedon Jul 5, 2018
I agree that enforcing this server-side instead of client-side is better. Perhaps it should be made easier for developers to download specific versions of
gofmt, so that they could usegofmt1.10for work projects even if they were using 1.9 or 1.11beta1 (master) on their laptop.gofmtcould even include a version flag, so that scripts could check if the right version is being used. Although it might be a bad idea for thegofmtthat ships with Go itself to report a version, equal but separate fromgo version.#22695 didn't seem to reach any conclusion that would help with this. I might open a proposal issue along the lines of what I described above, once this documentation issue is fixed.
thaJeztah commentedon Jul 5, 2018
Yes, while writing my previous comment, I wanted to suggest a "version" option, so that a different
gofmtcould be used to ease a project's transition between Go versions. I decided not to, because doing so may defeat the whole purpose ofgofmt(end all discussions around code-style/formatting).Having said the above, perhaps there's still merit in that option; introduce an option to select a "slow" version of
gofmt, limited to versions of Go that are actively maintained;For example, currently, Go 1.11 (next stable), 1.10.x (current stable), and 1.9.x (previous stable) are actively maintained; I think it's ok to assume (most) projects that support multiple Go versions won't Go beyond those versions
Could look something like this; (
gofmt [--version=<option>]);gofmtof the installed versionstablego tipor a betaprevious-stableedit: removed
--version=currentoption from the table, as it didn't make much sensemvdan commentedon Jul 5, 2018
When I mentioned a version flag, I meant just a flag to print its version :) Then, scripts could do something like:
I'll send a proposal about this in a few days. @thaJeztah if you want, I can email you a draft before publishing.
thaJeztah commentedon Jul 5, 2018
Looks like that would indeed help 👍 You can /cc me at
sebastiaan <swirly-thingy> docker.comgopherbot commentedon Jul 5, 2018
Change https://golang.org/cl/122295 mentions this issue:
doc: explain minor change to gofmt in go1.11ci: upgrade ci's golang to 1.11 to prevent from lint error
kortschak commentedon Sep 9, 2018
@mvdan
Yes, Russ has made this comment too in the past. Pre-commit checks don't work well in practice unless there is either a requirement that contributors run a Makefile to install the hooks or there is a bespoke codereview tool as exists for Go contributions. These are not necessarily feasible for small open source projects (a good example is Gonum where we already have difficulty managing new and drive-by contributions).
I'll just go and eat my brioche.
mvdan commentedon Sep 9, 2018
@kortschak please note that this issue is closed, and that I filed a proposal precisely to make enforcing gofmt easier - #26397.
Explicitly run go fmt on csi/csi_test.go
Explicitly run go fmt on csi/node_test.go
Explicitly run go fmt on csi/node_test.go