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

Jump to bottom

cmd/gofmt: format regression #25161

Closed
dvyukov opened this issue Apr 29, 2018 · 5 comments
Closed

cmd/gofmt: format regression #25161

dvyukov opened this issue Apr 29, 2018 · 5 comments
Labels
FrozenDueToAge Unfortunate
Milestone
Go1.11

Comments

@dvyukov
Copy link
Member

dvyukov commented Apr 29, 2018

Here is snippet of code:

package foo

var linuxOopses = []*oops{
	&oops{
		[]*regexp.Regexp{
			compile("INFO: lockdep is turned off"),
			compile("INFO: Stall ended before state dump start"),
			compile("INFO: NMI handler .* took too long to run"),
			compile("_INFO::"),                                       // Android can print this during boot.
			compile("INFO: sys_.* is not present in /proc/kallsyms"), // pkg/host output in debug mode
			compile("INFO: no syscalls can create resource"),         // pkg/host output in debug mode
		},
	},
}

With 1.10 formatting produces no diff, however with tip I am getting diff:

$ gofmt -d /tmp/format.go
diff -u /tmp/format.go.orig /tmp/format.go
--- /tmp/format.go.orig	2018-04-29 12:01:23.844573882 +0200
+++ /tmp/format.go	2018-04-29 12:01:23.844573882 +0200
@@ -6,7 +6,7 @@
 			compile("INFO: lockdep is turned off"),
 			compile("INFO: Stall ended before state dump start"),
 			compile("INFO: NMI handler .* took too long to run"),
-			compile("_INFO::"),                                       // Android can print this during boot.
+			compile("_INFO::"), // Android can print this during boot.
 			compile("INFO: sys_.* is not present in /proc/kallsyms"), // pkg/host output in debug mode
 			compile("INFO: no syscalls can create resource"),         // pkg/host output in debug mode
 		},

This breaks our CI.

go version devel +e0d37a33ab Sun Apr 29 09:38:32 2018 +0000 linux/amd64
@agnivade agnivade changed the title gofmt: format regression cmd/gofmt: format regression Apr 29, 2018
@agnivade agnivade added the NeedsFix The path to resolution is known, but the work has not been done. label Apr 29, 2018
@agnivade agnivade added this to the Go1.11 milestone Apr 29, 2018
@agnivade
Copy link
Contributor

/cc @griesemer

@dvyukov
Copy link
Member Author

dvyukov commented Apr 29, 2018

To clarify the problem: we have several Go versions on CI and users using a mix of Go versions. CI checks that all code is formatted.

@agnivade
Copy link
Contributor

bisected to 542ea5a

@jimmyfrasche
Copy link
Member

@dvyukov see also the discussion in #22695

dvyukov added a commit to google/syzkaller that referenced this issue Apr 29, 2018
@dvyukov
pkg/report: reformat code
c7f6891 
There is some regression in Go formatting on tip.
I am constantly getting diffs after formatting.
Filed: golang/go#25161
@griesemer
Copy link
Contributor

Specific to this example: The "_INFO::" entry is just one char below the threshold. If it were "__INFO::" (for instance), the comment would be aligned. I'm not saying you should change it; but there's a threshold and this code simply is below (or above) it. This is simply "unfortunate" in this case.

Regarding the layout algorithm: It could be better, and perhaps in the future we will have a better approach (which is also why we don't want to freeze gofmt). We could trivially change the threshold but that simply moves the problem from this example to another example.

More generally: We do not want to freeze gofmt. See #22695 (comment) .

I'm going to close this as "unfortunate".

@griesemer griesemer added Unfortunate and removed NeedsFix The path to resolution is known, but the work has not been done. labels Apr 30, 2018
This was referenced Jun 30, 2018
Closed
Closed
@mvdan mvdan mentioned this issue Jul 5, 2018
Closed
This was referenced Jul 5, 2018
Merged
Merged
This was referenced Aug 15, 2018
Closed
Merged
@asalkeld asalkeld mentioned this issue Nov 6, 2018
Closed
amyangfei added a commit to pingcap/dm that referenced this issue Jan 16, 2019
@amyangfei
test: fix comment align to satisfy gofmt (#402)
0e8882e 
gofmt regression strategy changes in different go release version. In
this case, go version under 1.11.x will re-format one line code while
go version 1.11.x does nothing. Similar issue occurs in
golang/go#25161. So we change some comment position
amyangfei added a commit to pingcap/dm that referenced this issue Jan 16, 2019
@amyangfei
test: fix comment align to satisfy gofmt (#402)
8209bc7 
gofmt regression strategy changes in different go release version. In
this case, go version under 1.11.x will re-format one line code while
go version 1.11.x does nothing. Similar issue occurs in
golang/go#25161. So we change some comment position
amyangfei added a commit to pingcap/dm that referenced this issue Jan 16, 2019
@amyangfei @IANTHEREAL
test: fix comment align to satisfy gofmt (#402)
755b376 
gofmt regression strategy changes in different go release version. In
this case, go version under 1.11.x will re-format one line code while
go version 1.11.x does nothing. Similar issue occurs in
golang/go#25161. So we change some comment position
amyangfei added a commit to pingcap/dm that referenced this issue Jan 16, 2019
@amyangfei
test: fix comment align to satisfy gofmt (#402)
891d6ff 
gofmt regression strategy changes in different go release version. In
this case, go version under 1.11.x will re-format one line code while
go version 1.11.x does nothing. Similar issue occurs in
golang/go#25161. So we change some comment position
amyangfei added a commit to pingcap/dm that referenced this issue Jan 16, 2019
@amyangfei
test: fix comment align to satisfy gofmt (#402)
f283d23 
gofmt regression strategy changes in different go release version. In
this case, go version under 1.11.x will re-format one line code while
go version 1.11.x does nothing. Similar issue occurs in
golang/go#25161. So we change some comment position
amyangfei added a commit to pingcap/dm that referenced this issue Jan 16, 2019
@amyangfei
test: fix comment align to satisfy gofmt (#402)
7e58ed8 
gofmt regression strategy changes in different go release version. In
this case, go version under 1.11.x will re-format one line code while
go version 1.11.x does nothing. Similar issue occurs in
golang/go#25161. So we change some comment position
amyangfei added a commit to pingcap/dm that referenced this issue Jan 16, 2019
@amyangfei
test: fix comment align to satisfy gofmt (#402)
72ea652 
gofmt regression strategy changes in different go release version. In
this case, go version under 1.11.x will re-format one line code while
go version 1.11.x does nothing. Similar issue occurs in
golang/go#25161. So we change some comment position
amyangfei added a commit to pingcap/dm that referenced this issue Jan 16, 2019
@amyangfei
test: fix comment align to satisfy gofmt (#402)
010ebfc 
gofmt regression strategy changes in different go release version. In
this case, go version under 1.11.x will re-format one line code while
go version 1.11.x does nothing. Similar issue occurs in
golang/go#25161. So we change some comment position
amyangfei added a commit to pingcap/dm that referenced this issue Jan 16, 2019
@amyangfei
test: fix comment align to satisfy gofmt (#402)
0840f6b 
gofmt regression strategy changes in different go release version. In
this case, go version under 1.11.x will re-format one line code while
go version 1.11.x does nothing. Similar issue occurs in
golang/go#25161. So we change some comment position
@dvyukov dvyukov mentioned this issue Feb 11, 2019
Merged
@dvyukov dvyukov mentioned this issue Mar 14, 2019
Merged
dvyukov added a commit to dvyukov/syzkaller that referenced this issue Mar 14, 2019
@dvyukov
all: switch to Go 1.12
eb2251e 
Differences in code formatting between Go versions cause constant
problems for us (golang/go#25161).
Currently we support 1.9 and 1.10. Switch to newer 1.11 and 1.12.

Fixes google#1013
dvyukov added a commit to google/syzkaller that referenced this issue Mar 14, 2019
@dvyukov
all: switch to Go 1.12
d23e90a 
Differences in code formatting between Go versions cause constant
problems for us (golang/go#25161).
Currently we support 1.9 and 1.10. Switch to newer 1.11 and 1.12.

Fixes #1013
@golang golang locked and limited conversation to collaborators Apr 30, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge Unfortunate
Projects
None yet
Milestone
Go1.11
Development

No branches or pull requests
5 participants
@jimmyfrasche @dvyukov @agnivade @griesemer @gopherbot