Fix gopackagesdriver for Go 1.18 by replicating stdlib and add test for stdliblist.go #3157

xytan0056 · 2022-05-18T07:43:02Z

What type of PR is this?

This PR addresses comments for #3083

What does this PR do? Why is it needed?
It fixes go list invocations from bazel for go1.18, where std library uses go:embed
see #3083

Which issues(s) does this PR fix?

Fixes #3080

Other notes for review
updated the patch in our repo with this patch

verified that the targets that failed without the patch can build successfully
verified a rebuild of all our repo behaved as expected

The stdliblist operation that the gopackagesdriver relies on fails on Go 1.18rc1 with the following error: ``` external/go_sdk/src/crypto/elliptic/p256_asm.go:24:12: pattern p256_asm_table.bin: cannot embed irregular file p256_asm_table.bin ``` We see this failure because Bazel builds a symlink tree of the GOROOT run `go list` with. However, since [CL 380475][1], the Go standard library uses `go:embed` to embed a file in `crypto/elliptic`, but `go:embed` does not support symlinks. [1]: https://go.dev/cl/380475 Fix this by having stdliblist copy the relevant portions of the GOROOT to run `go list` with. This matches [what the stdlib action does][2]. [2]: https://github.com/bazelbuild/rules_go/blob/e9a7054ff11a520e3b8aceb76a3ba44bb8da4c94/go/tools/builders/stdlib.go#L54-L57 Resolves bazel-contrib#3080

Add a dependency on `GoStdLib._list_json` to the stdlib test. This ensures that we can successfully build the JSON data needed by the gopackagesdriver.

The prior version of this fix was incomplete because it generated incorrect relative paths. For example, before a path would be: __BAZEL_OUTPUT_BASE__/external/go_sdk/src/archive/tar/common.go But with the prior version of this change. __BAZEL_OUTPUT_BASE__/src/archive/tar/common.go It would drop the external/go_sdk from the path because the flattening logic in stdliblist makes these relative to the execRoot. We cannot overwrite external/go_sdk in execRoot because that's a path controlled by Bazel. Instead, create a parallel external/go_sdk under a new directory "root", and flatten paths relative to that.

recover tests/core/stdlib/stdlib_files.bzl

go/tools/builders/stdliblist.go

go/tools/builders/stdliblist_test.go

linzhp · 2022-05-18T19:16:20Z

go/tools/builders/env.go

@@ -66,7 +66,7 @@ type env struct {
 // configured with those flags.
 func envFlags(flags *flag.FlagSet) *env {
 	env := &env{}
-	flags.StringVar(&env.sdk, "sdk", "", "Path to the Go SDK.")
+	flags.StringVar(&env.sdk, "sdk", "", "Relative path to the Go SDK.")


This set of flags are reused in other places, which may not require it to be relative path. Let's instead check it in stdliblist to verify it's a relative path

done

if filepath.Abs(goenv.sdk) { return error }

xytan0056 · 2022-05-18T23:48:02Z

weirdly this CI build failed for windows
https://buildkite.com/bazel/rules-go-golang/builds/3922
but this one passed
https://buildkite.com/bazel/rules-go-golang/builds/3923
but the difference between the 2 commits are trivial
https://github.com/bazelbuild/rules_go/compare/6918fd7..d61ec66

go/tools/builders/stdliblist.go

[![Mend Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com) This PR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [io_bazel_rules_go](https://togithub.com/bazelbuild/rules_go) | http_archive | minor | `v0.32.0` -> `v0.33.0` | --- ### Release Notes <details> <summary>bazelbuild/rules_go</summary> ### [`v0.33.0`](https://togithub.com/bazelbuild/rules_go/releases/tag/v0.33.0) [Compare Source](https://togithub.com/bazelbuild/rules_go/compare/v0.32.0...v0.33.0) ##### Breaking changes - [cover](https://togithub.com/bazelbuild/rules_go/blob/master/go/toolchains.rst#cover) has been removed in v0.32.0. Please [leave a comment](https://togithub.com/bazelbuild/rules_go/issues/3168) if you are affected by this. ##### Deprecations - The [asm](https://togithub.com/bazelbuild/rules_go/blob/master/go/toolchains.rst#asm), [compile](https://togithub.com/bazelbuild/rules_go/blob/master/go/toolchains.rst#compile), and [pack](https://togithub.com/bazelbuild/rules_go/blob/master/go/toolchains.rst#pack) action generators provided by [go_context](https://togithub.com/bazelbuild/rules_go/blob/master/go/toolchains.rst#go_context) are deprecated and planned for removal in version v0.36.0. Please leave a comment on the [tracking bug](https://togithub.com/bazelbuild/rules_go/issues/3168) if [archive](https://togithub.com/bazelbuild/rules_go/blob/master/go/toolchains.rst#archive) and [link](https://togithub.com/bazelbuild/rules_go/blob/master/go/toolchains.rst#link) are not suitable replacements for your use cases. ##### Bug Fixes - [@sluongng](https://togithub.com/sluongng) fixed a race condition that could cause non-sandboxed builds of `go_test` targets to fail ([https://github.com/bazelbuild/rules_go/pull/3145](https://togithub.com/bazelbuild/rules_go/pull/3145)) - [@abhinav](https://togithub.com/abhinav) made `//go:embed` work with `go_path` ([https://github.com/bazelbuild/rules_go/pull/3163](https://togithub.com/bazelbuild/rules_go/pull/3163)) - [@xytan0056](https://togithub.com/xytan0056) made gopackagesdriver work with Go 1.18 ([https://github.com/bazelbuild/rules_go/pull/3157](https://togithub.com/bazelbuild/rules_go/pull/3157)) - [@nickgooding](https://togithub.com/nickgooding) ensured that `gomock` can be used with any Gazelle naming convention ([https://github.com/bazelbuild/rules_go/pull/3155](https://togithub.com/bazelbuild/rules_go/pull/3155)) - `go_library` targets using CGo can now reference unresolved symbols ([https://github.com/bazelbuild/rules_go/pull/3174](https://togithub.com/bazelbuild/rules_go/pull/3174)) Thanks to all of the contributors! ##### Compatibility The minimum required version of Bazel remains at 4.2.1. ##### Updated dependencies - Updated `org_golang_x_sys`, `org_golang_x_xerrors`, `org_golang_google_genproto`, `go_googleapis` to their most recent commit as of 2022-06-05 As always, you can use higher versions of rules_go's dependencies by declaring them in WORKSPACE before calling go_rules_dependencies. Lower versions may work but are not supported. ##### `WORKSPACE` code load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive") http_archive( name = "io_bazel_rules_go", sha256 = "685052b498b6ddfe562ca7a97736741d87916fe536623afb7da2824c0211c369", urls = [ "https://mirror.bazel.build/github.com/bazelbuild/rules_go/releases/download/v0.33.0/rules_go-v0.33.0.zip", "https://github.com/bazelbuild/rules_go/releases/download/v0.33.0/rules_go-v0.33.0.zip", ], ) load("@io_bazel_rules_go//go:deps.bzl", "go_register_toolchains", "go_rules_dependencies") go_rules_dependencies() go_register_toolchains(version = "1.18.3") </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR is behind base branch, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] If you want to rebase/retry this PR, click this checkbox. --- This PR has been generated by [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View repository job log [here](https://app.renovatebot.com/dashboard#github/googleapis/gapic-generator-go).

[![Mend Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com) This PR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [io_bazel_rules_go](https://togithub.com/bazelbuild/rules_go) | http_archive | minor | `v0.32.0` -> `v0.33.0` | --- ### Release Notes <details> <summary>bazelbuild/rules_go</summary> ### [`v0.33.0`](https://togithub.com/bazelbuild/rules_go/releases/tag/v0.33.0) [Compare Source](https://togithub.com/bazelbuild/rules_go/compare/v0.32.0...v0.33.0) #### Breaking changes - [cover](https://togithub.com/bazelbuild/rules_go/blob/master/go/toolchains.rst#cover) has been removed in v0.32.0. Please [leave a comment](https://togithub.com/bazelbuild/rules_go/issues/3168) if you are affected by this. #### Deprecations - The [asm](https://togithub.com/bazelbuild/rules_go/blob/master/go/toolchains.rst#asm), [compile](https://togithub.com/bazelbuild/rules_go/blob/master/go/toolchains.rst#compile), and [pack](https://togithub.com/bazelbuild/rules_go/blob/master/go/toolchains.rst#pack) action generators provided by [go_context](https://togithub.com/bazelbuild/rules_go/blob/master/go/toolchains.rst#go_context) are deprecated and planned for removal in version v0.36.0. Please leave a comment on the [tracking bug](https://togithub.com/bazelbuild/rules_go/issues/3168) if [archive](https://togithub.com/bazelbuild/rules_go/blob/master/go/toolchains.rst#archive) and [link](https://togithub.com/bazelbuild/rules_go/blob/master/go/toolchains.rst#link) are not suitable replacements for your use cases. #### Bug Fixes - [@sluongng](https://togithub.com/sluongng) fixed a race condition that could cause non-sandboxed builds of `go_test` targets to fail ([https://github.com/bazelbuild/rules_go/pull/3145](https://togithub.com/bazelbuild/rules_go/pull/3145)) - [@abhinav](https://togithub.com/abhinav) made `//go:embed` work with `go_path` ([https://github.com/bazelbuild/rules_go/pull/3163](https://togithub.com/bazelbuild/rules_go/pull/3163)) - [@xytan0056](https://togithub.com/xytan0056) made gopackagesdriver work with Go 1.18 ([https://github.com/bazelbuild/rules_go/pull/3157](https://togithub.com/bazelbuild/rules_go/pull/3157)) - [@nickgooding](https://togithub.com/nickgooding) ensured that `gomock` can be used with any Gazelle naming convention ([https://github.com/bazelbuild/rules_go/pull/3155](https://togithub.com/bazelbuild/rules_go/pull/3155)) - `go_library` targets using CGo can now reference unresolved symbols ([https://github.com/bazelbuild/rules_go/pull/3174](https://togithub.com/bazelbuild/rules_go/pull/3174)) Thanks to all of the contributors! #### Compatibility The minimum required version of Bazel remains at 4.2.1. #### Updated dependencies - Updated `org_golang_x_sys`, `org_golang_x_xerrors`, `org_golang_google_genproto`, `go_googleapis` to their most recent commit as of 2022-06-05 As always, you can use higher versions of rules_go's dependencies by declaring them in WORKSPACE before calling go_rules_dependencies. Lower versions may work but are not supported. #### `WORKSPACE` code load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive") http_archive( name = "io_bazel_rules_go", sha256 = "685052b498b6ddfe562ca7a97736741d87916fe536623afb7da2824c0211c369", urls = [ "https://mirror.bazel.build/github.com/bazelbuild/rules_go/releases/download/v0.33.0/rules_go-v0.33.0.zip", "https://github.com/bazelbuild/rules_go/releases/download/v0.33.0/rules_go-v0.33.0.zip", ], ) load("@io_bazel_rules_go//go:deps.bzl", "go_register_toolchains", "go_rules_dependencies") go_rules_dependencies() go_register_toolchains(version = "1.18.3") </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR is behind base branch, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] If you want to rebase/retry this PR, click this checkbox. --- This PR has been generated by [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View repository job log [here](https://app.renovatebot.com/dashboard#github/googleapis/gapic-config-validator).

bolitt · 2023-04-14T15:59:30Z

I can still reproduce the error for stdlib (same as #3083) in the following:

// Force import, and `p256_asm_table.bin` is where the error comes from.
import (
  _ "crypto/internal/nistec"
)

I think the source of problem is: p256_asm_table.bin is not included in the dependency.

Error message:

.../external/go_sdk/src/crypto/internal/nistec/p256_asm.go:322:12: pattern p256_asm_table.bin: cannot embed irregular file p256_asm_table.bin
bazel-out/darwin-opt-exec-2B5CBBC6/bin/external/org_golang_x_mobile/cmd/gomobile/gomobile_/gomobile: go build -v -buildmode=c-shared -o=/var/folders/lm/g6r0qpk15dsdklyy9w35dh040000gp/T/gomobile-work-1450563799/android/src/main/jniLibs/x86/libgojni.so ./gobind failed: exit status 1

Here is my environment:

OS: MacOS amd64
rules_go : 0.39.0

google.org/x/mobile is already updated the latest one:

    go_repository(
        name = "org_golang_x_mobile",
        importpath = "golang.org/x/mobile",
        sum = "h1:Gk61ECugwEHL6IiyyNLXNzmu8XslmRP2dS0xjIYhbb4=",
        version = "v0.0.0-20230301163155-e0f57694e12c",
    )

abhinav and others added 12 commits May 11, 2022 23:47

test/stdlib: Depend on _list_json

6c94802

Add a dependency on `GoStdLib._list_json` to the stdlib test. This ensures that we can successfully build the JSON data needed by the gopackagesdriver.

address comments

41c0139

wip

1de0b73

rebase

bcb289f

recover tests/core/stdlib/stdlib_files.bzl

buildifier

89e0cf6

clean up

68b94d1

address comments

8b0dde2

works

0c12fc9

simpler version

d0e87d6

assume absolute path

309f5de

xytan0056 mentioned this pull request May 18, 2022

Fix gopackagesdriver for Go 1.18 by replicating stdlib and add test for stdliblist.go #3153

Closed

xytan0056 commented May 18, 2022

View reviewed changes

go/tools/builders/stdliblist.go Outdated Show resolved Hide resolved

xytan0056 added 3 commits May 18, 2022 01:17

test

7913ea3

use external/go_sdk

ca25b77

buildifier

29449cc

xytan0056 changed the title ~~Tanx/stdliblist abs~~ Fix gopackagesdriver for Go 1.18 by replicating stdlib and add test for stdliblist.go May 18, 2022

use workDir

9a85829

xytan0056 marked this pull request as ready for review May 18, 2022 18:39

linzhp reviewed May 18, 2022

View reviewed changes

xytan0056 marked this pull request as draft May 18, 2022 23:29

xytan0056 force-pushed the tanx/stdliblist-abs branch from d61ec66 to 6918fd7 Compare May 18, 2022 23:32

address comments

a9f613a

xytan0056 force-pushed the tanx/stdliblist-abs branch from 6918fd7 to a9f613a Compare May 18, 2022 23:33

xytan0056 marked this pull request as ready for review May 18, 2022 23:48

linzhp approved these changes May 19, 2022

View reviewed changes

go/tools/builders/stdliblist.go Outdated Show resolved Hide resolved

minor renaming

e568d43

linzhp merged commit db94584 into bazel-contrib:master May 19, 2022

linzhp mentioned this pull request May 19, 2022

Fix gopackagesdriver for Go 1.18 by replicating stdlib #3083

Closed

iamricard mentioned this pull request May 30, 2022

VS Code setup of language server fails to load packages. #3110

Open

fmeum mentioned this pull request Jun 5, 2022

Prepare release v0.33.0 #3175

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix gopackagesdriver for Go 1.18 by replicating stdlib and add test for stdliblist.go #3157

Fix gopackagesdriver for Go 1.18 by replicating stdlib and add test for stdliblist.go #3157

xytan0056 commented May 18, 2022 •

edited

Loading

linzhp May 18, 2022

xytan0056 May 18, 2022 •

edited

Loading

xytan0056 commented May 18, 2022

bolitt commented Apr 14, 2023 •

edited

Loading

Fix gopackagesdriver for Go 1.18 by replicating stdlib and add test for stdliblist.go #3157

Fix gopackagesdriver for Go 1.18 by replicating stdlib and add test for stdliblist.go #3157

Conversation

xytan0056 commented May 18, 2022 • edited Loading

linzhp May 18, 2022

Choose a reason for hiding this comment

xytan0056 May 18, 2022 • edited Loading

Choose a reason for hiding this comment

xytan0056 commented May 18, 2022

bolitt commented Apr 14, 2023 • edited Loading

xytan0056 commented May 18, 2022 •

edited

Loading

xytan0056 May 18, 2022 •

edited

Loading

bolitt commented Apr 14, 2023 •

edited

Loading