Skip to content

go: "go" directive in modules does not prevent using new stdlib code #73603

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

Closed
Groxx opened this issue May 5, 2025 · 5 comments
Closed

go: "go" directive in modules does not prevent using new stdlib code #73603

Groxx opened this issue May 5, 2025 · 5 comments
Labels
BugReport Issues describing a possible bug in the Go implementation.

Comments

@Groxx
Copy link

Groxx commented May 5, 2025

Go version

go version go1.24.2 darwin/arm64

Output of go env in your module/workspace:

❯ go env
AR='ar'
CC='cc'
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_ENABLED='1'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
CXX='c++'
GCCGO='gccgo'
GO111MODULE=''
GOARCH='arm64'
GOARM64='v8.0'
GOAUTH='netrc'
GOBIN=''
GOCACHE='/Users/stevenl/Library/Caches/go-build'
GOCACHEPROG=''
GODEBUG=''
GOENV='/Users/stevenl/Library/Application Support/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFIPS140='off'
GOFLAGS=''
GOGCCFLAGS='-fPIC -arch arm64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -ffile-prefix-map=/var/folders/zp/sqm92pj94sq7whfh1rl3yjd00000gn/T/go-build3995360884=/tmp/go-build -gno-record-gcc-switches -fno-common'
GOHOSTARCH='arm64'
GOHOSTOS='darwin'
GOINSECURE=''
GOMOD='/Users/stevenl/go/src/go.uber.org/cadence/go.mod'
GOMODCACHE='/Users/stevenl/go/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='darwin'
GOPATH='/Users/stevenl/go'
GOPRIVATE=''
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/opt/homebrew/Cellar/go/1.24.2/libexec'
GOSUMDB='sum.golang.org'
GOTELEMETRY='local'
GOTELEMETRYDIR='/Users/stevenl/Library/Application Support/go/telemetry'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/opt/homebrew/Cellar/go/1.24.2/libexec/pkg/tool/darwin_arm64'
GOVCS=''
GOVERSION='go1.24.2'
GOWORK=''
PKG_CONFIG='pkg-config'

What did you do?

go build ./... in a project using 1.24-specific stdlib packages/methods, but a 1.21-specific go.mod file.

What did you see happen?

Build (and run) succeeded.

Demo in the playground: https://go.dev/play/p/qqJqFz5G8et

What did you expect to see?

Given the go 1.21 version in go.mod, I expected the build to fail as if it was built using go 1.21, e.g. GOTOOLCHAIN=go1.21.1 go build ./... which produces errors like

prog.go:5:2: package weak is not in std (/usr/local/go-faketime/src/weak)

But this does not happen.

This too-old-version build failure is currently reproducible in the playground with the previous go version, e.g. https://go.dev/play/p/qqJqFz5G8et?v=goprev which uses go 1.23, but I'm not sure how to target specific versions to make a stable link.

The go module reference seems to imply the go version should protect against new Go things:

The go directive affects use of new language features:

  • For packages within the module, the compiler rejects use of language features introduced after the version specified by the go directive. For example, if a module has the directive go 1.12, its packages may not use numeric literals like 1_000_000, which were introduced in Go 1.13.

But perhaps this is only for "language features" like generics, not "language features" like the standard library? TBH I'm not sure that distinction makes much sense, the two are inseparable for essentially all users.

go vet also does not complain about this, nor does go mod tidy try to upgrade the target version. Currently this seems to mean that we need to set up a manual test matrix to build with multiple Go versions, just to check something that can be statically determined.
This seems... subpar?

@seankhliao
Copy link
Member

As the play snippet shows, it's a vet check, not a build error.
See previously #46136

@seankhliao seankhliao closed this as not planned Won't fix, can't repro, duplicate, stale May 5, 2025
@gabyhelp gabyhelp added the BugReport Issues describing a possible bug in the Go implementation. label May 5, 2025
@Groxx
Copy link
Author

Groxx commented May 5, 2025

The snippet doesn't seem to show it being a vet check? It just outputs

demo of https://github.com/golang/go/issues/73603

Program exited.

Also running this locally and then go vet . does not produce any output at all:

❯ bat *
───────┬─────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
       │ File: go.mod
───────┼─────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
   1   │ module example.com
   2   │
   3   │ go 1.21
───────┴─────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
───────┬─────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
       │ File: main.go
───────┼─────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
   1   │ package main
   2   │
   3   │ import (
   4   │     "fmt"
   5   │     "testing"
   6   │     "weak"
   7   │ )
   8   │
   9   │ func main() {
  10   │     fmt.Println("demo of https://github.com/golang/go/issues/73603")
  11   │ }
  12   │
  13   │ func ignored() {
  14   │     (&testing.T{}).Context()      // new method introduced in 1.24: https://pkg.go.dev/testing@master#T.Context
  15   │     _ = weak.Make((*string)(nil)) // new package in 1.24
  16   │ }
───────┴─────────────────────────────────────────────────────────────────────────────────────────────────────────────────────

❯ go vet .

❯ go vet -stdversion .

❯ go vet -stdversion

❯ go vet -stdversion ./...

❯ go version
go version go1.24.2 darwin/arm64

@Groxx
Copy link
Author

Groxx commented May 5, 2025

@seankhliao ^ I believe this might deserve re-opening. Maybe as a vet bug report.

Groxx added a commit to cadence-workflow/cadence-go-client that referenced this issue May 5, 2025
Mockery is failing when run with go 1.24 due to vektra/mockery#914 , so let's upgrade it.

Basic steps to upgrade are:
```
# add go toolchain to go.mod to enforce a higher minimum
cd internal/tools
go get golang.org/x/tools@latest github.com/vektra/mockery/v2@latest
cd ../..
make all
```
and some careful reading to verify the results.

And then some changes to address our CI needs, e.g. since runtime and tool-time versions have diverged (currently) we now automatically pull and build with the old version.
Most of these version values can be found by doing a grep like
```
❯ rg 'go(lang)?.?1.?2\d'
go.mod
3:go 1.21                # intentionally different

docker/buildkite/Dockerfile
1:FROM golang:1.24

Makefile
36:EXPECTED_GO_VERSION := go1.24

CHANGELOG.md
28:- Bump x/tools for tools, to support go 1.22 (#1336)
143:- Fix TestActivityWorkerStop: it times out with go 1.20 by @dkrotx in #1223

internal/tools/go.mod
3:go 1.23.0
5:toolchain go1.24.2
```

I had _expected_ `go 1.21` in go.mod to take care of this kind of check, but apparently not: golang/go#73603
so it's now done by hand.

---

There are *some* changes which I think stand a very small chance of causing problems:
- Our RPC client's mocks now check for `.Get(0).(func(args) (any, error)` signatures where before they did not
  - I believe anyone using this would've hit a `.Error` further down after these changes, so hopefully this just fixes buggy behavior and does not cause any existing tests to fail
- Many funcs gained a `if len(ret) == 0 { panic(..) }` check
  - Since there's a `ret.Get(0)` immediately after which also panics, I suspect this just gives better error messages.
- Our mocks now have a `DoAndReturn` method
  - Since there are no interfaces for all this, this should be fine, ignoring reflection (which we must ignore or else we will go mad).

Otherwise seems like nothing exciting, and this does not affect any user-facing libraries or Go versions.
Since this needs a newer CI Go version to build, I've updated those references too.

---

Separately, this was the source of some rather disturbing TILs:
```bash
❯ bash -ec '
[[[ "$a" = "foo"]] && echo first
echo second
'
bash: line 2: [[[: command not found
second

❯ echo $?
0
```

Apparently syntax errors are (sometimes) not script failures, even with `set -e`.

And `sh` does not support `[[`, so it was _printing an error_ and continuing without erroring.  CI uses `sh`, locally we generally get bash or zsh, so a locally-working `[[` may be a remote-silently-failing command.

Madness.  Absolute madness.  This is going to haunt me for a while, unless I can find a way to prevent it (without shellcheck).
@seankhliao
Copy link
Member

It does work on tip (e.g. playground dev branch), so if there was a bug, it was fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BugReport Issues describing a possible bug in the Go implementation.
Projects
None yet
Development

No branches or pull requests

3 participants