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

x/tools/cmd/stringer: does no longer work in package with uninstalled dependencies #31216

Closed
eliaperantoni opened this issue Apr 2, 2019 · 21 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Tools This label describes issues relating to any tools in the x/tools repository.
Milestone

Comments

@eliaperantoni
Copy link

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

$ go version
go version go1.11.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\Elia\AppData\Local\go-build
set GOEXE=.exe
set GOFLAGS=
set GOHOSTARCH=amd64
set GOHOSTOS=windows
set GOOS=windows
set GOPATH=C:\Users\Elia\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=
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 -fmessage-length=0 -fdebug-prefix-map=C:\Users\Elia\AppData\Local\Temp\go-build407096830=/tmp/go-build -gno-record-gcc-switches

What did you do?

Ran stringer -type MyType in the package where struct MyType is declared.

What did you expect to see?

stringer running correctly and producing a generated code file like it always did so far

What did you see instead?

This error

PS C:\Users\Elia\go\src\gitlab.com\eliaperantoni\helloworld> stringer -type MyType
stringer: go [list -e -json -compiled=true -test=false -export=false -deps=true -find=false -tags= -- .]: exit status 1: go build github.com/sirupsen/logrus: no Go files in
go build github.com/goburrow/modbus: no Go files in

It seems like stringer is no longer able to operate when searching for a type declaration in a package that has uninstalled dependencies like it did before.

Installing dependencies before running stringer makes it work correctly.

Why would anyone need this?

I'm building docker images for my go application that doesn't use any dependency managment tool yet. The build process:

  1. Installs some essential dependencies like stringer itself
  2. Runs go generate
  3. Then installs all dependencies using go get -d ./.... Because of this there's no need to update the Dockerfile after adding or removing dependencies.

Running go generate before go get -d ./...is necessary because otherwise some directories would end up having no go files, causing go get to fail.

When was it working?

I've manually installed a version of golang.org/x/tools that goes back to the first week of march 2019 and that one works without any problems. It seems like golang.org/x/tools/go/packages has been updated ever since (while golang.org/x/tools/cmd/stringer hasn't) but I can't pinpoint the line or commit that is causing the issue.

Maybe this is even some intentional behaviour?

@gopherbot gopherbot added this to the Unreleased milestone Apr 2, 2019
@andybons
Copy link
Member

andybons commented Apr 3, 2019

@matloob @ianthehat

@andybons andybons added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Apr 3, 2019
@matloob
Copy link
Contributor

matloob commented Apr 4, 2019

I'm sorry, I don't completely understand: what do you mean by uninstalled dependencies? Do you mean that the dependencies of the package itself are missing?

@matloob
Copy link
Contributor

matloob commented Apr 4, 2019

@jba since he helped merge in @suzmue's work on stringer.

@andybons andybons changed the title x/tools/cmd/stringer does no longer work in package with uninstalled dependencies x/tools/cmd/stringer: does no longer work in package with uninstalled dependencies Apr 4, 2019
@eliaperantoni
Copy link
Author

@matloob Yes, by that I mean packages that I import in my project but are actually not installed on the system.

This happens i.e. when building a Docker image for a Go project that uses stringer.

...
COPY . .
RUN go generate
RUN go get -d ./...
...

Using RUN go get -d ./... is easier to use than managing dependencies inside Dockerfile but it must be run after go generate (which includes some stringer steps) otherwise some packages will have no Go file and the build process will exit

This might be a edge case but I thought that pointing out that it used to work and now it doesn't was the right thing to do.

@eliaperantoni
Copy link
Author

To reproduce:

git clone https://github.com/eliaperantoni/stringerIssue31216.git
cd stringerIssue31216
go mod download
go run golang.org/x/tools/cmd/stringer -type MyType

It seems like running stringer the first time makes it crash but running it again works correctly.
To make it crash again you have to edit root.go and change the imported package name to any other string.
I have no clue why it only works the second time but I think it always crashed in my real world scenario, no matter how many times I ran it.
I hope this is still useful.

@eliaperantoni
Copy link
Author

It looks like the bug first appears in golang/tools@dbeab5a

@matloob

@matloob
Copy link
Contributor

matloob commented Apr 4, 2019

Hm, this is interesting:

git clone https://github.com/eliaperantoni/stringerIssue31216.git
cd stringerIssue31216
go list -e -compiled .; echo $? #  exit status 1
go list -e -compiled .; echo $? # exit status 0

I don't think go list should have that behavior. Though we'll need to put in a temporary fix for the mean time

edit: added "-e", though the behavior is present both with and without -e

@matloob
Copy link
Contributor

matloob commented Apr 4, 2019

I want to make sure I understand the problem: You're trying to run stringer on a package that has a dependency that doesn't exist right? I'm going to do some research to figure out how stringer handles packages it can't type-check.

@matloob
Copy link
Contributor

matloob commented Apr 4, 2019

@bcmills @jayconrod for the go list behavior in #31216 (comment). Is that expected?

@eliaperantoni
Copy link
Author

@matloob It's correct. In my example I've used a meaningless string for the package name (you can also use completely random strings that don't make any sense for the import path actually and the issue remains) while in my real world scenario I was dealing with real libraries (such as logrus) that were imported but were not installed on the system yet

@bcmills
Copy link
Contributor

bcmills commented Apr 4, 2019

@matloob, go list -e -compiled failing is #26755.

If that snippet is accurate, the 0 exit status from the second call may be another bug; we'll need to investigate one the first one is fixed.

@matloob
Copy link
Contributor

matloob commented Apr 4, 2019

Are you using a meaningless string so you can replicate after the first time you stringer?

That is, were you also seeing the issue when using a package that does exist? I think the go list behavior in the comment above explains the behavior with a package that doesn't exist, but if it's happening with a package that does exist, there's another, bigger problem.

Could you try running stringer with a empty GOCACHE and GOPATH as follows?:

GOCACHE=$(mktemp -d) GOPATH=$(mktemp -d) go run golang.org/x/tools/cmd/stringer -type MyType

I'm interested if you still see the error in that case

@matloob
Copy link
Contributor

matloob commented Apr 4, 2019

@bcmills, so go list -e should return a zero exit status even if an imported package doesn't exist, right?

I'll have go/packages suppress that particular error

@eliaperantoni
Copy link
Author

eliaperantoni commented Apr 4, 2019

@matloob So I originally discovered the issue while using packages that exist and can be fetched (such as github.com/goburrow/modbus and github.com/sirupsen/logrus) but the issue persists with import paths that could technically exist but actually don't (the github.com/eliaperantoni/stringerIssue31216/subPackage is potentially a valid import path that the go tool could be able to resolve but actually is not a real package) and also completely random strings.

By the way yes I used random strings to find the commit that first caused the bug.

The fact that stringer sometimes starts working from the second time on could be related to the choice of the import path (existing but not installed, existing but potentially valid or completely random).

I'll investigate on that.

Also I'll try what you suggested

EDIT: I've never observed stringer failing when all imported packages were installed. I've seen it fail when using an import string that satisfies one of these:

  1. It's a package that could be fetched (eg from git) but is not present on the host filesystem yet
  2. It's a made up package that doesn't exist anywhere but could be a valid import string if only it existed
  3. It's an entirely random string

@matloob
Copy link
Contributor

matloob commented Apr 4, 2019

Okay, thanks! Looking forward to the results

@eliaperantoni
Copy link
Author

elia@elia-mint:/tmp/stringerIssue31216$ GOCACHE=$(mktemp -d) GOPATH=$(mktemp -d) go run golang.org/x/tools/cmd/stringer -type MyType
go: finding golang.org/x/tools v0.0.0-20190404132500-923d25813098
go: finding golang.org/x/net v0.0.0-20190311183353-d8887717615a
go: finding golang.org/x/text v0.3.0
go: finding golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2
go: finding golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a
go: downloading golang.org/x/tools v0.0.0-20190404132500-923d25813098
go: extracting golang.org/x/tools v0.0.0-20190404132500-923d25813098
stringer: go [list -e -json -compiled=true -test=false -export=false -deps=true -find=false -tags= -- .]: exit status 1: go: finding github.com/eliaperantoni/stringerIssue31216/subPackage latest
go build github.com/eliaperantoni/stringerIssue31216/subPackage: no Go files in 
exit status 1

I think the first lines should be ignore as go mod is trying to fetch the dependency but the installed stringer version is still the one from golang/tools@dbeab5a

The issue doesn't resolve itself by running the command twice.

@matloob
Copy link
Contributor

matloob commented Apr 4, 2019

Okay, that matches what I saw on my own machine.

I'm wondering if you can get a reproduction with a package that does exist?

@eliaperantoni
Copy link
Author

Okay so reproducing the error with existing packages is much more difficult than I thought. I might have oversimplified some of my examples and I'm very sorry for that :(
I've updated my repo to reproduce a scenario much more similar to what I've encountered initially.

Basically I am not using go modules and I have a subpackage that has no go files until code generation is run. This package (once generated) will depend on an external package (github.com/sirupsen/logrus) this is very important, otherwise stringer will just work based on my experiments.

Running go generate will run code generation for the subpackage (a trival file renaming) and also stringer for the root package. This is the (rather weird maybe but no that much) scenario where stringer fails.

You can see below that (in this scenario) stringer only fails when using dbeab5a or later

$ echo "using stringer from commit dbeab5af4b8d3204d444b78cafaba18a9a062a50"
$ cd
$ folder=$GOPATH/src/github.com/eliaperantoni
$ mkdir -p $folder
$ cd $folder
$ git clone https://github.com/eliaperantoni/stringerIssue31216.git
$ cd stringerIssue31216/
$ export GOCACHE=$(mktemp -d)
$ go generate

stringer: go [list -e -json -compiled=true -test=false -export=false -deps=true -find=false -tags= -- .]: exit status 1: go build github.com/sirupsen/logrus: no Go files in 
root.go:4: running "stringer": exit status 1

$ echo "using stringer from commit dbeab5af4b8d3204d444b78cafaba18a9a062a50"
$ cd
$ rm -rf $folder
$ sudo rm -rf $GOPATH/src/golang.org/x/tools
$ mkdir -p $GOPATH/src/golang.org/x
$ cd $GOPATH/src/golang.org/x
$ git clone https://github.com/golang/tools.git
$ cd tools
$ git checkout 45dd101d87843da2a383dd0ce49a8c8519ce766b
$ go install golang.org/x/tools/cmd/stringer
$ cd
$ folder=$GOPATH/src/github.com/eliaperantoni
$ mkdir -p $folder
$ cd $folder
$ git clone https://github.com/eliaperantoni/stringerIssue31216.git
$ cd stringerIssue31216/
$ export GOCACHE=$(mktemp -d)
$ go generate
$ echo "no errors"

@gopherbot gopherbot added the Tools This label describes issues relating to any tools in the x/tools repository. label Sep 12, 2019
@matloob
Copy link
Contributor

matloob commented Oct 30, 2019

Hi I'm really sorry this fell through the cracks. I tried reproducing it (using head) and I got now errors for both attempts (at head and at 45dd101). Is this still an issue?

@eliaperantoni
Copy link
Author

No worries. Actually I'm not having issues anymore on the latest version with my current project setup

@matloob
Copy link
Contributor

matloob commented Oct 30, 2019

Ok, I'll mark this as closed then. Please reopen if this shows up again.

@matloob matloob closed this as completed Oct 30, 2019
@golang golang locked and limited conversation to collaborators Oct 29, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Tools This label describes issues relating to any tools in the x/tools repository.
Projects
None yet
Development

No branches or pull requests

5 participants