all: audit which generate directives depend on version of vendored packages #43440

dmitshur · 2020-12-31T00:06:30Z

The Go standard library uses vendored copies of some external packages. These packages are updated as needed to fix bugs or bring in new features, and in bulk as part of release cycle maintenance (issue #36905).

There are some generate directives that depend on data in vendored packages, and so they need to be re-run whenever vendored versions change.

We are currently aware of:

net/http:
    //go:generate bundle -o=h2_bundle.go -prefix=http2 -tags=!nethttpomithttp2 
    //go:generate bundle -o socks_bundle.go -prefix socks golang.org/x/net/internal/socks

syscall:
    //go:generate go run ./mksyscall_windows.go -systemdll -output zsyscall_windows.go syscall_windows.go security_windows.go
internal/syscall/windows:
    //go:generate go run ../../../syscall/mksyscall_windows.go -output zsyscall_windows.go syscall_windows.go security_windows.go psapi_windows.go symlink_windows.go
internal/syscall/windows/registry:
    //go:generate go run ../../../../syscall/mksyscall_windows.go -output zsyscall_windows.go syscall.go

So at the very least:

go generate -run=bundle net/http needs to be re-run whenever golang.org/x/net version changes
go generate syscall internal/syscall/... whenever golang.org/x/sys version changes

This is the tracking issue to audit the rest and find out if there any more. We can incorporate findings here into tests that are being added for #36852 and #41409. Thanks to @bcmills for raising this question in CL 275446 (comment).

As of commit 20d0991, there are a total of 60 generate directives in GOROOT/src, not counting testdata and vendored packages:

Searching 4051 files for "^//go:generate " (regex)

cmd/compile/internal/gc/alg.go:
   18: //go:generate stringer -type AlgKind -trimprefix A

cmd/compile/internal/gc/go.go:
   61: //go:generate stringer -type=Class

cmd/compile/internal/gc/main.go:
    5: //go:generate go run mkbuiltin.go

cmd/compile/internal/gc/syntax.go:
  741: //go:generate stringer -type=Op -trimprefix=O

cmd/compile/internal/syntax/tokens.go:
    9: //go:generate stringer -type token -linecomment
  108: //go:generate stringer -type Operator -linecomment

cmd/compile/internal/types/type.go:
   18: //go:generate stringer -type EType -trimprefix T

cmd/go/internal/test/testflag.go:
   22: //go:generate go run ./genflags.go

cmd/go/main.go:
    5: //go:generate ./mkalldocs.sh

cmd/internal/goobj/builtin.go:
   36: //go:generate go run mkbuiltin.go

cmd/internal/obj/arm/a.out.go:
   35: //go:generate go run ../stringer.go -i $GOFILE -o anames.go -p arm

cmd/internal/obj/arm64/a.out.go:
  511: //go:generate go run ../stringer.go -i $GOFILE -o anames.go -p arm64

cmd/internal/obj/link.go:
  219: //go:generate stringer -type AddrType
  542: //go:generate stringer -type ABI

cmd/internal/obj/mips/a.out.go:
   36: //go:generate go run ../stringer.go -i $GOFILE -o anames.go -p mips

cmd/internal/obj/ppc64/a.out.go:
   34: //go:generate go run ../stringer.go -i $GOFILE -o anames.go -p ppc64

cmd/internal/obj/riscv/cpu.go:
   33: //go:generate go run ../stringer.go -i $GOFILE -o anames.go -p riscv

cmd/internal/obj/s390x/a.out.go:
   34: //go:generate go run ../stringer.go -i $GOFILE -o anames.go -p s390x

cmd/internal/obj/wasm/a.out.go:
    9: //go:generate go run ../stringer.go -i $GOFILE -o anames.go -p wasm

cmd/internal/obj/x86/aenum.go:
    7: //go:generate go run ../stringer.go -i $GOFILE -o anames.go -p x86

cmd/internal/objabi/reloctype.go:
   35: //go:generate stringer -type=RelocType

cmd/internal/objabi/symkind.go:
   40: //go:generate stringer -type=SymKind

cmd/link/internal/sym/symkind.go:
   39: //go:generate stringer -type=SymKind

crypto/md5/md5.go:
    5: //go:generate go run gen.go -output md5block.go

crypto/tls/common.go:
  367: //go:generate stringer -type=SignatureScheme,CurveID,ClientAuthType -output=common_string.go

crypto/x509/root.go:
    7: //go:generate go run root_ios_gen.go -version 55161.140.3

debug/dwarf/const.go:
    9: //go:generate stringer -type Attr -trimprefix=Attr
  204: //go:generate stringer -type Tag -trimprefix=Tag

debug/dwarf/entry.go:
  372: //go:generate stringer -type=Class

debug/macho/reloctype.go:
    7: //go:generate stringer -type=RelocTypeGeneric,RelocTypeX86_64,RelocTypeARM,RelocTypeARM64 -output reloctype_string.go

encoding/gob/decode.go:
    5: //go:generate go run decgen.go -output dec_helpers.go

encoding/gob/encode.go:
    5: //go:generate go run encgen.go -output enc_helpers.go

html/template/context.go:
   84: //go:generate stringer -type state
  168: //go:generate stringer -type delim
  186: //go:generate stringer -type urlPart
  207: //go:generate stringer -type jsCtx
  225: //go:generate stringer -type element
  241: //go:generate stringer -type attr

image/color/palette/generate.go:
    5: //go:generate go run gen.go -output palette.go

image/internal/imageutil/imageutil.go:
    5: //go:generate go run gen.go

index/suffixarray/sais.go:
  121: //go:generate go run gen.go

internal/syscall/windows/mksyscall.go:
    9: //go:generate go run ../../../syscall/mksyscall_windows.go -output zsyscall_windows.go syscall_windows.go security_windows.go psapi_windows.go symlink_windows.go

internal/syscall/windows/registry/mksyscall.go:
    9: //go:generate go run ../../../../syscall/mksyscall_windows.go -output zsyscall_windows.go syscall.go

math/big/float.go:
  144: //go:generate stringer -type=RoundingMode
  157: //go:generate stringer -type=Accuracy

math/bits/bits.go:
    5: //go:generate go run make_tables.go

net/http/http.go:
    5: //go:generate bundle -o=h2_bundle.go -prefix=http2 -tags=!nethttpomithttp2 golang.org/x/net/http2

net/http/socks_bundle.go:
    2: //go:generate bundle -o socks_bundle.go -prefix socks golang.org/x/net/internal/socks

regexp/syntax/regexp.go:
   29: //go:generate stringer -type Op -trimprefix Op

runtime/internal/sys/sys.go:
   15: //go:generate go run gengoos.go

runtime/preempt.go:
  291: //go:generate go run mkpreempt.go

runtime/runtime.go:
   12: //go:generate go run wincallback.go
   13: //go:generate go run mkduff.go
   14: //go:generate go run mkfastlog2table.go

runtime/sizeclasses.go:
    2: //go:generate go run mksizeclasses.go

sort/sort.go:
    5: //go:generate go run genzfunc.go

strconv/quote.go:
    5: //go:generate go run makeisprint.go -output isprint.go

syscall/syscall.go:
   29: //go:generate go run ./mksyscall_windows.go -systemdll -output zsyscall_windows.go syscall_windows.go security_windows.go

time/tzdata/tzdata.go:
    5: //go:generate go run generate_zipdata.go

time/zoneinfo.go:
   13: //go:generate env ZONEINFO=$GOROOT/lib/time/zoneinfo.zip go run genzabbrs.go -output zoneinfo_abbrs_windows.go

60 matches across 49 files

CC @golang/release.

The text was updated successfully, but these errors were encountered:

gopherbot · 2021-01-14T07:04:03Z

Change https://golang.org/cl/283643 mentions this issue: cmd/internal/moddeps: check content of all modules in GOROOT

Expand the scope of the TestAllDependenciesVendored test to check that all modules in GOROOT are tidy, that packages are vendored, the vendor content matches the upstream copy exactly, and that bundled packages are re-generated (using x/tools/cmd/bundle at the version selected in cmd module; this is deterministic and guaranteed to be updated over time). This is done in a conceptually simple way: 1. Make a temporary copy of the entire GOROOT tree (except .git), one that is safe to modify. 2. Run a list of high-level commands, the same commands we expect Go developers should be able to run in a normal complete GOROOT tree to make it clean and tidy. 3. Diff the end result with the original GOROOT tree being tested to catch any unexpected differences. The current set of commands that are run require the cmd/go command, and a functional compiler itself (because re-generating the syscall package involves a directive like //go:generate go run [...]). As a result, copying a large majority of the GOROOT tree is a requirement. Instead of looking for the few files or directories that can we can get away not copying (e.g., the testdata directories aren't strictly needed at this time), we opt not to optimize and just do the simple copy. This is motivated by these reasons: • We end up having a complete, normal GOROOT tree, one that happens to be located at another path. There's a very high likelihood that module management/code generation commands, both the ones we run today and any additional ones that we might want to add in the future, will result in correct results even as the Go project evolves over time. • Having a completely stand-alone copy of the GOROOT tree without symlinks minimizes the risk of some of the module management/code generation commands, either now or in the future, from modifying the user's original GOROOT tree, something that should not happen during test execution. Overlays achieved with symlinks work well when we can guarantee only new files are added, but that isn't the case here. • Copying the entire GOROOT (without .git), takes around 5 seconds on a fairly modern computer with an SSD. The most we can save is a couple of seconds. (We make some minor exceptions: the GOROOT/.git directory isn't copied, and GOROOT/{bin,pkg} are deemed safe to share and thus symlink instead of copying. If these optimizations cease to be viable to make, we'll need to remove them.) Since this functionality is fairly expensive to execute and requires network access, it runs only when the test is executed without -short flag. The previous behavior of the TestAllDependenciesVendored test is kept in -short test mode. all.bash runs package tests with -short flag, so its behavior is unchanged. The expectation is that the new test will run on some of the longtest builders to catch problems. Users can invoke the test manually 'go test cmd/internal/moddeps' (and it's run as part of 'go test cmd', again, only when -short flag isn't provided). On a 2017 MacBook Pro, a successful long test takes under 15 seconds, which should be within scope of all long tests that are selected by 'go test std cmd'. We may further adjust when and where the test runs by default based on our experience. Fixes #36852. Fixes #41409. Fixes #43687. Updates #43440. Change-Id: I9eb85205fec7ec62e3f867831a0a82e3c767f618 Reviewed-on: https://go-review.googlesource.com/c/go/+/283643 Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org> TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: Bryan C. Mills <bcmills@google.com> Trust: Dmitri Shuralyov <dmitshur@golang.org>

dmitshur added the NeedsFix The path to resolution is known, but the work has not been done. label Dec 31, 2020

dmitshur added this to the Backlog milestone Dec 31, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

all: audit which generate directives depend on version of vendored packages #43440

all: audit which generate directives depend on version of vendored packages #43440

dmitshur commented Dec 31, 2020

gopherbot commented Jan 14, 2021

all: audit which generate directives depend on version of vendored packages #43440

all: audit which generate directives depend on version of vendored packages #43440

Comments

dmitshur commented Dec 31, 2020

gopherbot commented Jan 14, 2021