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: dependency module in workspace has go version requirement loaded before local replacement #66516

Open
JetSetIlly opened this issue Mar 25, 2024 · 9 comments
Labels
GoCommand cmd/go NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.

Comments

@JetSetIlly
Copy link

Go version

1.21.7

Output of go env in your module/workspace:

GO111MODULE=''
GOARCH='amd64'
GOBIN=''
GOCACHE='/home/steve/.cache/go-build'
GOENV='/home/steve/.config/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFLAGS=''
GOHOSTARCH='amd64'
GOHOSTOS='linux'
GOINSECURE=''
GOMODCACHE='/home/steve/Go/go/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='linux'
GOPATH='/home/steve/Go/go'
GOPRIVATE=''
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/home/steve/Go/go1.21.7/go'
GOSUMDB='sum.golang.org'
GOTMPDIR=''
GOTOOLCHAIN='local'
GOTOOLDIR='/home/steve/Go/go1.21.7/go/pkg/tool/linux_amd64'
GOVCS=''
GOVERSION='go1.21.7'
GCCGO='gccgo'
GOAMD64='v1'
AR='ar'
CC='gcc'
CXX='g++'
CGO_ENABLED='1'
GOMOD='/home/steve/Source/andy/f1gopher/go.mod'
GOWORK=''
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
PKG_CONFIG='pkg-config'
GOGCCFLAGS='-fPIC -m64 -pthread -Wl,--no-gc-sections -fmessage-length=0 -ffile-prefix-map=/tmp/go-build3910143555=/tmp/go-build -gno-record-gcc-switches'

What did you do?

I want to build a project with Go version 1.21.7

The go.mod file says version 1.22.0 is required. However, I know this isn't strictly true and will compile with <1.22.0 except for the go.mod file

The project has a dependency which also requires version 1.22.0 according to its go.mod file, but which uses no 1.22.0 features and will compile with the lower version of the Go compiler (except for the go.mod file)

I have local copies of both the project and the dependency cloned from github

I change the version requirement in the go.mod of both projects and begin a go.work file with "go work init"

I add both projects to the go.work file with "go work use "

What did you see happen?

The project will not build with the lower Go version despite the changes to the version requirements in the local copies of the go.mod files

The error message shown as below:

go: module github.com/project/project@v0.9.1-0.20240221103349-46ee8a99748c requires go >= 1.22 (running go 1.21.7; GOTOOLCHAIN=local)

This project (name changed because it is not a public repository) is the dependency that was cloned and added to the go.work file after changing the Go version requirement in the go.mod file

What did you expect to see?

I expected the project to be compiled because there are no 1.22.0 features used in either project and because I changed the Go version requirements in the local copy of both the project and the dependency

It seems like Go is checking the version requirements of the remote version of the dependency and not the local version pointed to by the go.work file

NOTE

I understand that if I set GOTOOLCHAIN to auto then this would work. But I believe that a GOTOOLCHAIN set to local should still allow me to change the Go version requirements in local copies of projects in the way described

NOTE 2

I have checked that my Go workspace is working correctly by compiling the project with a local version of 1.22.0. In that case the version check passes and local copy of the dependency is used

@seankhliao seankhliao changed the title GOTOOLCHAIN='local' and Go Workspaces cmd/go: GOTOOLCHAIN='local' and Go Workspaces Mar 25, 2024
@seankhliao seankhliao added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. GoCommand cmd/go labels Mar 25, 2024
@seankhliao
Copy link
Member

can you provide a reproducer for the issue?
I couldn't replicate your results

@seankhliao seankhliao added the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label Apr 10, 2024
@JetSetIlly
Copy link
Author

I tried reproducing with a simple reproducer and I'm having difficulty myself. The basic mechanism works as expected.

I do have a reproducer but for a complex case which I'll include at the bottom of the post.

I think the problem is being caused by the main project using a dependency that is not a versioned release and only pointing to a git commit.

If I checkout the correct commit in the cloned repository, change the required go version in the go.mod file, then I would want that to work. However, it does not.

Go doesn't seem to be matching the dependency with the cloned repository, even when it has been checked out at the correct commit.

This is the main project: https://github.com/f1gopher/f1gopher

And the reproducer:

> git clone git@github.com:f1gopher/f1gopher.git
Cloning into 'f1gopher'...
remote: Enumerating objects: 796, done.
remote: Counting objects: 100% (11/11), done.
remote: Compressing objects: 100% (11/11), done.
remote: Total 796 (delta 0), reused 8 (delta 0), pack-reused 785
Receiving objects: 100% (796/796), 9.54 MiB | 6.17 MiB/s, done.
Resolving deltas: 100% (593/593), done.

> cd f1gopher

> go run .
go: go.mod requires go >= 1.22 (running go 1.21.7; GOTOOLCHAIN=local)

> go mod edit -go 1.21.0

> go run .
go: github.com/f1gopher/f1gopherlib@v0.9.1-0.20240221103349-46ee8a99748c requires go >= 1.22 (running go 1.21.7; GOTOOLCHAIN=local)

> git clone git@github.com:f1gopher/f1gopherlib.git
Cloning into 'f1gopherlib'...
remote: Enumerating objects: 412, done.
remote: Counting objects: 100% (23/23), done.
remote: Compressing objects: 100% (18/18), done.
remote: Total 412 (delta 10), reused 16 (delta 5), pack-reused 389
Receiving objects: 100% (412/412), 183.52 KiB | 806.00 KiB/s, done.
Resolving deltas: 100% (277/277), done.

> cd f1gopherlib

> git checkout 46ee8a99748c
HEAD is now at 46ee8a9 * Update packages

> go mod edit -go 1.21.0

> cd ..

> go work init

> go work use .

> go work use f1gopherlib

> go run .
go: module github.com/f1gopher/f1gopherlib@v0.9.1-0.20240221103349-46ee8a99748c requires go >= 1.22 (running go 1.21.7; GOTOOLCHAIN=local)

@seankhliao seankhliao removed the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label Apr 10, 2024
@seankhliao seankhliao changed the title cmd/go: GOTOOLCHAIN='local' and Go Workspaces cmd/go: dependency module in workspace has go version requirement loaded before local replacement Apr 10, 2024
@seankhliao
Copy link
Member

it does work using replace directives, but not workspaces
cc @matloob @samthanawalla

@matloob
Copy link
Contributor

matloob commented Apr 16, 2024

This is the intended behavior although I do understand the inconvenience of it.

What's happening is that although f1gopherlib is now a main module, and the selected version of it is the directory you checked out at 46ee8a99748c, the dependency on v0.9.1-0.20240221103349-46ee8a99748c from f1gopherlib's go.mod file is still there. Adding a main module to a workspace does not replace all versions of that module in the Minimal Version Selection graph-- it only sets the roots the algorithm is run from. So when MVS is run the go 1.22 dependency of f1gopherlib@v0.9.1-0.20240221103349-46ee8a99748c is still there which will enforce the go 1.22 requirement.

You'd need to remove the dependencies on that version of f1gopherlib@v0.9.1-0.20240221103349-46ee8a99748c from f1gopher's go.mod file to not pull in the 1.21 requirement.

Locally I created a workspace with f1gopher and f1gopherlib in the same directory and otherwise followed your instructions. Then in the f1gopher directory I ran go1.21.4 get github.com/f1gopher/f1gopherlib@4b91e49 to change the requirement on f1gopher to the version before the 1.22 requirement was added. Module loading proceeded fine but of course I ran into cgo errors on my mac because I don't have pkgconfig/cairo installed.

@JetSetIlly
Copy link
Author

This is the intended behavior although I do understand the inconvenience of it.

What's happening is that although f1gopherlib is now a main module, and the selected version of it is the directory you checked out at 46ee8a99748c, the dependency on v0.9.1-0.20240221103349-46ee8a99748c from f1gopherlib's go.mod file is still there. Adding a main module to a workspace does not replace all versions of that module in the Minimal Version Selection graph-- it only sets the roots the algorithm is run from. So when MVS is run the go 1.22 dependency of f1gopherlib@v0.9.1-0.20240221103349-46ee8a99748c is still there which will enforce the go 1.22 requirement.

You'd need to remove the dependencies on that version of f1gopherlib@v0.9.1-0.20240221103349-46ee8a99748c from f1gopher's go.mod file to not pull in the 1.21 requirement.

Locally I created a workspace with f1gopher and f1gopherlib in the same directory and otherwise followed your instructions. Then in the f1gopher directory I ran go1.21.4 get github.com/f1gopher/f1gopherlib@4b91e49 to change the requirement on f1gopher to the version before the 1.22 requirement was added. Module loading proceeded fine but of course I ran into cgo errors on my mac because I don't have pkgconfig/cairo installed.

Understood. As @seankhliao says the replace directive works so it's still possible to do what I wanted in this instance, just not through workspaces. I'm happy with that 👍🏾 and I think the issue can be closed

However, I do think there's work that can be done here with error messages. I think error messages from the Go tool are generally quite poor (messages from the compiler itself are fine) and I think the error message in this case is a great example of that.

To be clear, the error message is entirely accurate but it's not helpful.

Thank you for looking into this for me.

@matloob
Copy link
Contributor

matloob commented Apr 16, 2024

Hi, we appreciate the feedback.

What do you think would be a more helpful error message in this case?

@JetSetIlly
Copy link
Author

Hi, we appreciate the feedback.

What do you think would be a more helpful error message in this case?

Great question. In this case it wasn't clear why the error message was talking about a module that I thought I had "replaced" with the workspace entry. The error message is the same with ./f1gopherlib in the workspace as it when there is no go.work at all. That's confusing.

So I think the message could be improved by acknowledging the existence of the local version of the module and going on to explain that it is the module as stated in the go.mod that does not pass the MVS check.

Maybe my issue is that the workings of MVS is opaque. Maybe there should be a way of listing the decisions that MVS makes. That might have helped me diagnose and to understand what caused the problem.

@JetSetIlly
Copy link
Author

The error message is the same with ./f1gopherlib in the workspace as it when there is no go.work at all.

Actually, there is a slight difference:

Without a go.work file:

go: github.com/f1gopher/f1gopherlib@v0.9.1-0.20240221103349-46ee8a99748c requires go >= 1.22 (running go 1.21.7; GOTOOLCHAIN=local)

With a go.work file:

go: module github.com/f1gopher/f1gopherlib@v0.9.1-0.20240221103349-46ee8a99748c requires go >= 1.22 (running go 1.21.7; GOTOOLCHAIN=local)

It's not clear why the second message includes module near the beginning of the string.

@matloob
Copy link
Contributor

matloob commented Apr 16, 2024

Got it- I think it could be very useful to make MVS decisions more visible, but that would be a pretty complex undertaking. We need to think about how we might implement that, how we would expose such a feature, and what the messages would look like

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
GoCommand cmd/go NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

No branches or pull requests

3 participants