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/gofmt: honor build tags, especially release tags #21337

Closed
dvyukov opened this issue Aug 8, 2017 · 12 comments
Closed

cmd/gofmt: honor build tags, especially release tags #21337

dvyukov opened this issue Aug 8, 2017 · 12 comments
Assignees
Milestone

Comments

@dvyukov
Copy link
Member

@dvyukov dvyukov commented Aug 8, 2017

go version go1.8.3 linux/amd64

I've pulled latest versions of all deps, including golang.org/x/net/context. Now 1.8 go fmt ./... fails with:

vendor/golang.org/x/net/context/go19.go:15:14: expected type, found '='
vendor/golang.org/x/net/context/go19.go:20:17: expected type, found '='

https://travis-ci.org/google/syzkaller/builds/262119828

Not sure what exactly part is guilty. But I guess it will affect a bunch of users.
Go1.9 is not released yet, and I am not sure when it will appear on travis-ci.org.
go fmt ignores build tags, which is reasonable.
1.8 go does not ignore vendor dirs.
I cannot not update packages because our cloud APIs continue to break public interfaces.

Any way we can resolve this?

odeke-em added a commit to orijtech/syzkaller that referenced this issue Aug 8, 2017
For golang/go#21337.

Since the introduction of aliases is in Go1.9 but Go1.9 hasn't
yet been officially released, let's use go1.9rc1 which is supported
on Travis CI by their Go version getter gimme
https://github.com/travis-ci/gimme
and also test against tip to ensure we aren't left behind.
@odeke-em
Copy link
Member

@odeke-em odeke-em commented Aug 8, 2017

@dvyukov for now, I've added for you a PR at google/syzkaller#329, please test it out on Travis CI and it should be able to run the tests against Go1.9rc1 as well as tip, instead of against 1.8.1 on which aliases are gonna trip out, and when Go1.9 is released then we can update from 1.9rc1 to 1.9.

odeke-em added a commit to orijtech/syzkaller that referenced this issue Aug 8, 2017
For golang/go#21337.

Since the introduction of aliases is in Go1.9 but Go1.9 hasn't
yet been officially released, let's use go1.9rc1 which is supported
on Travis CI by their Go version getter gimme
https://github.com/travis-ci/gimme
instead of against go1.8.1. This solves the problem on which
our vendored code is updated using Go1.9* syntax but is running
against Go1.8* in Travis CI tests.
dvyukov added a commit to google/syzkaller that referenced this issue Aug 8, 2017
For golang/go#21337.

Since the introduction of aliases is in Go1.9 but Go1.9 hasn't
yet been officially released, let's use go1.9rc1 which is supported
on Travis CI by their Go version getter gimme
https://github.com/travis-ci/gimme
instead of against go1.8.1. This solves the problem on which
our vendored code is updated using Go1.9* syntax but is running
against Go1.8* in Travis CI tests.
@bradfitz
Copy link
Contributor

@bradfitz bradfitz commented Aug 8, 2017

That's unfortunate. Even if we did a Go 1.8.x release for this, user also wouldn't get that.

So really the answer is for people to move to Go 1.9 if they use deps using Go 1.9 (even +build conditional Go 1.9) features I guess.

This is something to keep in mind for rolling out any future language changes.

/cc @ianlancetaylor @griesemer @robpike @broady @rsc

@dvyukov
Copy link
Member Author

@dvyukov dvyukov commented Aug 8, 2017

It seems that x/net/context added type aliases too early. I mean it's a widely used package and 1.9 is not even released.

@bradfitz
Copy link
Contributor

@bradfitz bradfitz commented Aug 8, 2017

I don't think so.

The whole point of +build tags is so we can do just that.

I think the bug here is that gofmt should respect +build go1.N tags at least, or at least least respect them enough to ignore them if the parse fails.

That is, if I'm running go1.14 and it's formatting a file with +build go1.16, it should try its best to format that file, unless it gets a parse error, in which case it should silently do nothing and not fail.

@ianlancetaylor ianlancetaylor added this to the Go1.10 milestone Aug 8, 2017
@griesemer
Copy link
Contributor

@griesemer griesemer commented Aug 9, 2017

gofmt doesn't touch a file if it contains syntax errors, but I'm not convinced that it shouldn't complain. A script could trivially discard any error output. Or, perhaps for manual use, maybe there could be a -silent flag.

@dvyukov
Copy link
Member Author

@dvyukov dvyukov commented Aug 10, 2017

@griesemer you mean always running go fmt as: go fmt ./... 2>/dev/null || true?

@griesemer
Copy link
Contributor

@griesemer griesemer commented Aug 10, 2017

@dvyukov Not always, but perhaps in situations where we don't care about errors.

dvyukov added a commit to dvyukov/syzkaller that referenced this issue Aug 10, 2017
dvyukov added a commit to google/syzkaller that referenced this issue Aug 10, 2017
@sh0umik
Copy link

@sh0umik sh0umik commented Nov 14, 2017

Still exists in

go version go1.8 linux/amd64

Found this bug while generating swagger doc for my api

2017/11/14 12:20:03 Generate swagger docs....
2017/11/14 12:20:03 Generate general API Info
2017/11/14 12:20:03 ParseFile panic:vendor/golang.org/x/net/context/go19.go:15:14: expected type, found '=' (and 1 more errors)
panic: ParseFile panic:vendor/golang.org/x/net/context/go19.go:15:14: expected type, found '=' (and 1 more errors)

goroutine 1 [running]:
log.Panicf(0x7d359e, 0x13, 0xc421104fb8, 0x1, 0x1)
        /usr/local/go/src/log/log.go:329 +0xda
github.com/swaggo/swag.(*Parser).getAllGoFileInfo.func1(0xc4262c5b30, 0x27, 0x962040, 0xc4262bd6c0, 0x0, 0x0, 0x0, 0x0)
        /home/fahim/go/src/github.com/swaggo/swag/parser.go:228 +0x1a2
path/filepath.walk(0xc4262c5b30, 0x27, 0x962040, 0xc4262bd6c0, 0xc42025c220, 0x0, 0x0)
        /usr/local/go/src/path/filepath/path.go:351 +0x81
path/filepath.walk(0xc42625dea0, 0x1f, 0x962040, 0xc421c22680, 0xc42025c220, 0x0, 0x0)
        /usr/local/go/src/path/filepath/path.go:376 +0x414
path/filepath.walk(0xc42614d680, 0x17, 0x962040, 0xc421a86ea0, 0xc42025c220, 0x0, 0x0)
        /usr/local/go/src/path/filepath/path.go:376 +0x414
path/filepath.walk(0xc424a09c00, 0x13, 0x962040, 0xc4227cc820, 0xc42025c220, 0x0, 0x0)
        /usr/local/go/src/path/filepath/path.go:376 +0x414
path/filepath.walk(0xc424a09b60, 0x11, 0x962040, 0xc4227cc750, 0xc42025c220, 0x0, 0x0)
        /usr/local/go/src/path/filepath/path.go:376 +0x414
path/filepath.walk(0xc42017452a, 0x6, 0x962040, 0xc4202a7860, 0xc42025c220, 0x0, 0x0)
        /usr/local/go/src/path/filepath/path.go:376 +0x414
path/filepath.walk(0x7ce199, 0x2, 0x962040, 0xc4201204e0, 0xc42025c220, 0x0, 0x10)
        /usr/local/go/src/path/filepath/path.go:376 +0x414
path/filepath.Walk(0x7ce199, 0x2, 0xc42025c220, 0x1a, 0x0)
        /usr/local/go/src/path/filepath/path.go:398 +0x14c
github.com/swaggo/swag.(*Parser).getAllGoFileInfo(0xc42020c500, 0x7ce199, 0x2)
        /home/fahim/go/src/github.com/swaggo/swag/parser.go:234 +0x7b
github.com/swaggo/swag.(*Parser).ParseApi(0xc42020c500, 0x7ce199, 0x2, 0x7cfbfe, 0x9)
        /home/fahim/go/src/github.com/swaggo/swag/parser.go:59 +0xd2
github.com/swaggo/swag/gen.(*Gen).Build(0xc4201b1950, 0x7ce199, 0x2, 0x7cfbfe, 0x9, 0x0, 0x0)
        /home/fahim/go/src/github.com/swaggo/swag/gen/gen.go:27 +0x42a
main.main.func1(0xc420088840, 0x0, 0xc420088840)
        /home/fahim/go/src/github.com/swaggo/swag/cmd/swag/main.go:23 +0x55
github.com/urfave/cli.HandleAction(0x7557e0, 0x7e1f60, 0xc420088840, 0xc420270100, 0x0)
        /home/fahim/go/src/github.com/urfave/cli/app.go:502 +0xd4
github.com/urfave/cli.Command.Run(0x7ce7c4, 0x4, 0x0, 0x0, 0xc42025c100, 0x1, 0x1, 0x7d1bd5, 0xe, 0x0, ...)
        /home/fahim/go/src/github.com/urfave/cli/command.go:210 +0xb87
github.com/urfave/cli.(*App).Run(0xc420388000, 0xc42000c5c0, 0x2, 0x2, 0x0, 0x0)
        /home/fahim/go/src/github.com/urfave/cli/app.go:259 +0x7b7
main.main()
        /home/fahim/go/src/github.com/swaggo/swag/cmd/swag/main.go:29 +0x196

Also Duting go fmt

gateway/vendor/golang.org/x/net/context/go19.go:15:14: expected type, found '='
gateway/vendor/golang.org/x/net/context/go19.go:20:17: expected type, found '='
exit status 2
gateway/vendor/golang.org/x/net/ipv4/batch.go:59:14: expected type, found '='
gateway/vendor/golang.org/x/net/ipv4/batch.go:78:2: expected declaration, found 'if'
exit status 2
gateway/vendor/golang.org/x/net/ipv6/batch.go:56:14: expected type, found '='
gateway/vendor/golang.org/x/net/ipv6/batch.go:69:2: expected declaration, found 'if'
exit status 2

here what go19.go file looks like

// Copyright 2017 The Go Authors. All rights reserved.
// Use of this source code is governed by a BSD-style
// license that can be found in the LICENSE file.

// +build go1.9

package context

import "context" // standard library's context, as of Go 1.7

// A Context carries a deadline, a cancelation signal, and other values across
// API boundaries.
//
// Context's methods may be called by multiple goroutines simultaneously.
type Context = context.Context

// A CancelFunc tells an operation to abandon its work.
// A CancelFunc does not wait for the work to stop.
// After the first call, subsequent calls to a CancelFunc do nothing.
type CancelFunc = context.CancelFunc

@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Nov 14, 2017

@sh0umik Yes, that specific problem with gofmt is fixed in the 1.9 release.

This issue is about whether gofmt should somehow honor build tags to avoid this kind of crash in the future.

@ianlancetaylor ianlancetaylor modified the milestones: Go1.10, Go1.11 Nov 14, 2017
@ianlancetaylor ianlancetaylor changed the title cmd/gofmt: fails on type aliases cmd/gofmt: honor build tags, especially release tags Nov 14, 2017
@bradfitz bradfitz modified the milestones: Go1.11, Go1.12 Jun 20, 2018
@rsc
Copy link
Contributor

@rsc rsc commented Oct 3, 2018

I don't think we should try to make gofmt or "go fmt" detect "files from the future" by parsing build tags. Typically you run gofmt on your own code.

@rsc rsc closed this Oct 3, 2018
@noaos
Copy link

@noaos noaos commented Nov 4, 2018

I got this error today during make generate. My code is rebased on 8bd6bd63 ('prog: allow escaping paths but don't generate them').
`vendor/golang.org/x/net/context/go19.go:15:14: expected type, found '='
vendor/golang.org/x/net/context/go19.go:20:17: expected type, found '='
exit status 2
vendor/golang.org/x/net/http2/transport.go

vendor/google.golang.org/grpc/status/status.go
vm/qemu/qemu.go
make[1]: *** [format_go] Error 1
`

Any suggestion how to move on?
Thanks.

@dvyukov
Copy link
Member Author

@dvyukov dvyukov commented Nov 4, 2018

Hi Noa,

We require Go 1.9+ because of this, please upgrade. Also if you are going to do any changes, then 1.9 or 1.10, but not 1.11 because of breaking changes in gofmt output.

@golang golang locked and limited conversation to collaborators Nov 4, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
9 participants
You can’t perform that action at this time.