From d22a959851ee91859b83e68eb6ac41728475be65 Mon Sep 17 00:00:00 2001 From: Michael Matloob Date: Mon, 14 Aug 2023 12:44:12 -0400 Subject: [PATCH 1/3] go/tools/gopackagesdriver: pass Compiler and Arch in DriverResponse go/packages is being changed to expect Compiler and Arch in its DriverResponse (see golang.org/cl/516917) because we can't expect the type of the types.Sizes returned by types.Sizes to be types.StdSizes. (The default it uses when Compiler and Arch aren't set is the set of types for gc,amd64, so there's no change in behavior if those fields aren't set, but we should set them. I'd also like to see if we can correctly provide the arch assuming it's not amd64. I left a question in the code asking about that. --- go/tools/gopackagesdriver/main.go | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/go/tools/gopackagesdriver/main.go b/go/tools/gopackagesdriver/main.go index fea2d2c141..7c1d69e9a0 100644 --- a/go/tools/gopackagesdriver/main.go +++ b/go/tools/gopackagesdriver/main.go @@ -18,8 +18,8 @@ import ( "context" "encoding/json" "fmt" - "go/types" "os" + "runtime" "strings" ) @@ -31,8 +31,10 @@ type driverResponse struct { // lists of multiple drivers, go/packages will fall back to the next driver. NotHandled bool - // Sizes, if not nil, is the types.Sizes to use when type checking. - Sizes *types.StdSizes + // Compiler and Arch are the arguments pass of types.SizesFor + // to get a types.Sizes to use when type checking. + Compiler string + Arch string // Roots is the set of package IDs that make up the root packages. // We have to encode this separately because when we encode a single package @@ -63,9 +65,14 @@ var ( additionalKinds = strings.Fields(os.Getenv("GOPACKAGESDRIVER_BAZEL_KINDS")) emptyResponse = &driverResponse{ NotHandled: true, - Sizes: types.SizesFor("gc", "amd64").(*types.StdSizes), - Roots: []string{}, - Packages: []*FlatPackage{}, + // For the reviewer: Does bazel ever use gccgo? I assume bazel can build for other + // platforms. If that's the case, how do we find out the actual arch to use here? + // Is it reasonable to assume that the configuration that gopackagesdriver is built + // with matches the configuration + Compiler: "gc", + Arch: runtime.GOARCH, + Roots: []string{}, + Packages: []*FlatPackage{}, } ) From 824a5f2f45a3397178d620ead77d03b339ba1cd5 Mon Sep 17 00:00:00 2001 From: Michael Matloob Date: Mon, 14 Aug 2023 13:02:16 -0400 Subject: [PATCH 2/3] fix an additional place the driverResponse is instantiated --- go/tools/gopackagesdriver/json_packages_driver.go | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/go/tools/gopackagesdriver/json_packages_driver.go b/go/tools/gopackagesdriver/json_packages_driver.go index 9bbf34081a..35b675abf0 100644 --- a/go/tools/gopackagesdriver/json_packages_driver.go +++ b/go/tools/gopackagesdriver/json_packages_driver.go @@ -16,7 +16,7 @@ package main import ( "fmt" - "go/types" + "runtime" ) type JSONPackagesDriver struct { @@ -52,8 +52,10 @@ func (b *JSONPackagesDriver) GetResponse(labels []string) *driverResponse { return &driverResponse{ NotHandled: false, - Sizes: types.SizesFor("gc", "amd64").(*types.StdSizes), - Roots: rootPkgs, - Packages: packages, + // Reviewer: see comment in main.go about the Arch field in the driver response. + Compiler: "gc", + Arch: runtime.GOARCH, + Roots: rootPkgs, + Packages: packages, } } From 72409a7f04f83df905292e86d66ad912012e1565 Mon Sep 17 00:00:00 2001 From: Michael Matloob Date: Mon, 14 Aug 2023 13:49:17 -0400 Subject: [PATCH 3/3] remove question to reviewers --- go/tools/gopackagesdriver/json_packages_driver.go | 9 ++++----- go/tools/gopackagesdriver/main.go | 12 ++++-------- 2 files changed, 8 insertions(+), 13 deletions(-) diff --git a/go/tools/gopackagesdriver/json_packages_driver.go b/go/tools/gopackagesdriver/json_packages_driver.go index 35b675abf0..d91f2ed114 100644 --- a/go/tools/gopackagesdriver/json_packages_driver.go +++ b/go/tools/gopackagesdriver/json_packages_driver.go @@ -52,10 +52,9 @@ func (b *JSONPackagesDriver) GetResponse(labels []string) *driverResponse { return &driverResponse{ NotHandled: false, - // Reviewer: see comment in main.go about the Arch field in the driver response. - Compiler: "gc", - Arch: runtime.GOARCH, - Roots: rootPkgs, - Packages: packages, + Compiler: "gc", + Arch: runtime.GOARCH, + Roots: rootPkgs, + Packages: packages, } } diff --git a/go/tools/gopackagesdriver/main.go b/go/tools/gopackagesdriver/main.go index 7c1d69e9a0..8cc904d3ce 100644 --- a/go/tools/gopackagesdriver/main.go +++ b/go/tools/gopackagesdriver/main.go @@ -65,14 +65,10 @@ var ( additionalKinds = strings.Fields(os.Getenv("GOPACKAGESDRIVER_BAZEL_KINDS")) emptyResponse = &driverResponse{ NotHandled: true, - // For the reviewer: Does bazel ever use gccgo? I assume bazel can build for other - // platforms. If that's the case, how do we find out the actual arch to use here? - // Is it reasonable to assume that the configuration that gopackagesdriver is built - // with matches the configuration - Compiler: "gc", - Arch: runtime.GOARCH, - Roots: []string{}, - Packages: []*FlatPackage{}, + Compiler: "gc", + Arch: runtime.GOARCH, + Roots: []string{}, + Packages: []*FlatPackage{}, } )