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

Dependency on golang.org/x/sys/unix missing in 0.14 #1648

Closed
Xjs opened this issue Aug 10, 2018 · 14 comments
Closed

Dependency on golang.org/x/sys/unix missing in 0.14 #1648

Xjs opened this issue Aug 10, 2018 · 14 comments
Labels

Comments

@Xjs
Copy link
Contributor

Xjs commented Aug 10, 2018

0.14.0 upgraded the google.golang.org/grpc dependency to version 1.14 (c73c25f#diff-fdb6ae35486586f3473cd99b5d4c4815R128). However, google.golang.org/grpc introduced a dependency on golang.org/x/sys/unix from 1.13 to 1.14. (Exactly here: grpc/grpc-go@7268ca4#diff-be8985446e731a737c227f2b89d6bf1c)

This leads to targets depending on @org_golang_google_grpc//:go_default_library breaking with rules_go 0.14.0. Artificial example: https://github.com/Xjs/protoTest/tree/v0.14.0. (Breaks on linux only for me, works on Windows, interestingly enough.)

Would it break stuff to add golang.org/x/sys/unix to go/private/repositories.bzl? Am I missing something? (I remember seeing issues relating to golang.org/x/sys/unix...)

@jayconrod jayconrod added the bug label Aug 10, 2018
@jayconrod
Copy link
Contributor

For consistency with the other dependencies (e.g., org_golang_x_net), we should probably declare it.

I'm a bit hesitant to declare this because a lot of people may be depending on specific versions, and providing something within go_rules_dependencies may block that. It is possible to override repositories, but Bazel can ignore overrides for repositories that have already been resolved, so this may cause some confusion.

So... needs investigation, but should be done soon (or not).

jayconrod pushed a commit to jayconrod/rules_go that referenced this issue Aug 10, 2018
The newest version of gRPC depends on @org_golang_x_sys. Since we
provide other gRPC dependencies, we should declare this one as well.

Fixes bazel-contrib#1648
@jayconrod
Copy link
Contributor

Okay, after some experimentation, it seems that it is now consistently possible to override rules by calling go_repository before go_rules_dependencies(). That didn't used to be the case because Gazelle had some external dependencies (which are now vendored).

So #1649 will add org_golang_x_sys to go_rules_dependencies. I will likely cherry-pick that to release-0.14 and tag 0.14.1 next week.

#1650 adds a FAQ and clarifies the documentation on overriding repository rules.

jayconrod added a commit that referenced this issue Aug 10, 2018
The newest version of gRPC depends on @org_golang_x_sys. Since we
provide other gRPC dependencies, we should declare this one as well.

Fixes #1648
jayconrod pushed a commit that referenced this issue Aug 22, 2018
* Move legacy reproducibility test to new setup (#1585)

* Move legacy reproducibility test to new setup

Also included:
1. Adds an LLVM toolchain to the WORKSPACE file that can be used to
manually test changes against clang on linux.
2. Adds os.PathSeparator to stripped absolute paths to be consistent
with stripping being done everywhere else.
3. Adds a test to check the string "bazel-sandbox" in the binary.
4. Uses the cgo binary instead of a pure binary to check for
reproducibility in the go_test.
5. Tags the target "collect_digests" as manual.

To see how binaries are not reproducible with clang and `-g`, use
```
bazel test --copt=-g --crosstool_top=@llvm_toolchain//:toolchain \
  //tests/reproducibility:go_default_test
```

* remove manual tag from reproducibility test

* Do not use debug mode for reproducibility test.

* Update bazel-toolchain to auto detect OS version

* Add a primitive benchmark (#1599)

bazel_benchmark checks out rules_go to a temporary directory, creates
a temporary workspace, measures the time it takes to build various
targets, then appends the times to a .csv file.

This will probably be much more sophisticated in the future, but it's
good to have something basic now.

* go/tools/bazel_benchmark: extract some logic into bash script (#1601)

bazel_benchmark.sh is now responsible for cloning rules_go at master
into a temp directory. This script can be copied to a bin directory
and run with a timer. The rest of the bazel_benchmark.go logic will
run at the tip of master.

Also: record Bazel version in the output file.

* Set Bazel version tested in Travis CI to 0.15.0 (#1604)

* Speed up downloading of @go_googleapis by using http_archive. (#1603)

The googleapis repository seems to be of such a size that it takes a
long time to clone on a link with lower bandwidth. So long, in fact,
that it causes timeouts in my case.

* Use add_all(), add_joined() instead of deprecated functionality of add() (#1602)

* Change crosstool dependency to @bazel_tools//tools/cpp:current_cc_toolchain (#1605)

//tools/defaults is a special case which is being removed in Bazel.

* Add go_sdk rule and GoSDK provider (#1606)

go_sdk is a new rule that gathers information about an SDK and returns
a GoSDK provider which will get wired into the toolchain.

package_list is a new rule that generates a list of importable
packages from the sources in the SDK (previously, we invoked go list,
which is slower).

* Wire go_sdk and go_toolchain together (#1607)

* go_toolchain has a new mandatory attribute, "sdk", which be
  something that provides GoSDK.
* go_host_sdk, go_local_sdk, and go_download_sdk are now macros that
  wrap the old rules. Each rule declares toolchains in its BUILD.bazel
  file that work on the host architecture. The macro calls
  register_toolchains with these.
* go_register_toolchains no longer calls register_toolchains, but it
  will an SDK rule if "go_sdk" isn't defined. This is a step toward
  allowing multiple SDKs to support multiple execution platforms.
* Action inputs are narrowed to use go.sdk.tools and go.stdlib.libs
  rather than larger sets of files.

* stdlib now uses precompiled libraries if the mode is compatible (#1608)

* Remove deprecated --batch flag (#1617)

* Add go_wrap_sdk rule (#1618)

go_wrap_sdk allows you to configure a Go SDK that was downloaded or
located with another repository rule.

Related #1611

* Optimize args and inputs construction (#1610)

* Use add_all() to lazily construct args

* add_joined() omits the argument if value is an empty list

* Optimize compile

* Optimize cover

* Simplify tags argument construction

* Use any() instead of a dict

* Undo depset() usage

* Statically link tool binaries (#1615)

* A few arguments construction cleanups (#1621)

* Update toolchain and provider documentation [skip ci] (#1622)

* Document race, msan and other attributes for go_binary, go_test [skip ci] (#1624)

* Create .bazelrc (#1626)

* Create .bazelrc

See bazelbuild/bazel#5756 (comment)

* Update .bazelrc

* Remove explicit Label() construction (#1627)

attr.label() converts strings into labels by itself.

* Make go_sdk's package_list optional (#1625)

This will eventually be removed when old versions of Gazelle are no
longer supported and nothing depends on the file by name.

If not provided as an input, go_sdk will generate the file itself.

* Update deprecated single_file -> allow_single_file attribute (#1628)

Also remove allow_files where it conflicts with allow_single_file (not
allowed).
Also remove both allow_files and allow_single_file in private attributes
where executable is set to True (a file cannot be specified anyway -
private attribute).

* Document how to avoid proto conflicts [skip ci] (#1631)

Fixes #1548

* Set RULES_GO_VERSION to 0.14.0 (#1633)

* Propagate mode aspect on "_coverdata" edges (#1632)

* Propagate mode aspect on "_coverdata" edges

This ensures the coverdata library is built in the same mode as the
binary that depends on it.

Fixes #1630

* set pure = "on" on test to make CI happy

* Update dependencies (#1634)

bazel_gazelle to master as of 2018-08-06
com_google_protobuf to v3.6.1
com_github_goog_protobuf to v1.1.1
org_golang_x_net to master as of 2018-08-06
org_golang_google_grpc to v1.14.0
org_golang_google_genproto to master as of 2018-08-06
go_googleapis to master as of 2018-08-06
com_github_kevinburke_go_bindata to v3.11.0
org_golang_x_tools to master as of 2018-08-07

* Announce release 0.14.0 [skip ci] (#1636)

Also, fix gazelle example to use prefix directive instead of
attribute.

* Add CI config to test on RBE. (#1638)

* Add CI config to test on RBE.

* Disable BAZEL_DO_NOT_DETECT_CPP_TOOLCHAIN.

This is set by default in the rbe_ubuntu1604 platform, but tests in this
repo need this to be disabled.

* Skip tests that are not RBE compatible.

* Use latest release version of bazel-toolchains repo. (#1639)

* Declare org_golang_x_sys in go_rules_dependencies (#1649)

The newest version of gRPC depends on @org_golang_x_sys. Since we
provide other gRPC dependencies, we should declare this one as well.

Fixes #1648

* Document how to override go_rules_dependencies [skip ci] (#1650)

Related #1649

* Update minimum version of Bazel for Travis CI to 0.16.0 (#1651)

Also, remove logic in .travis.yml for downloading Bazel at HEAD. We're
not doing that anymore.

* Actions that use go.args may now use param files automatically (#1652)

Multiple param files are now supported as well.

* Split go.args into go.builder_args and go.tool_args (#1653)

Both helpers enable multiline files. Any action using either of these
helpers should support them. Other actions may use go.actions.args.

go.builder_args adds default arguments that builders should be able to
interpret, including -sdk and -tags.

go.args is deprecated.

* Use -importcfg files for compiling and linking (#1654)

The Go toolchain has supported importcfg files since 1.9. These files
give the build system finer control over dependencies using importmap
and packagefile declarations. Using these files allows us to abandon
-I and -L flags, which will help us stay under command line length
limits.

Fixes #1637

* Update genproto dependencies (#1657)

* update org_golang_google_genproto

* update go_googleapis

* Add test generates long compile/link command lines (#1655)

Related #1637

* Remove 'cfg = "data"' from all attributes (#1658)

The "data" configuration has been deprecated for a while and has no
effect.

* Remove gazelle and its deps from go_rules_dependencies (#1659)

go_rules_dependencies no longer declares the following repositories:

* bazel_gazelle
* com_github_bazelbuild_buildtools
* com_github_pelletier_go_toml

The "gazelle" rule is removed from //go:def.bzl. It has been
deprecated for some time, and "gazelle fix" replaces it.

* Windows: Use absolute and shortened path for GOROOT environment variable (#1647)

* Update tests ahead of Go 1.11 (#1661)

A test in the old version of org_golang_x_crypto we were testing fails
with Go 1.11. This is fixed in newer versions.

* Remove uses of deprecated dictionary concatenation (#1663)

This removes the need for --incompatible_disallow_dict_plus

* Force absolute paths in builders (#1664)

* Update org_golang_x_tools to master as of 2018-08-15 (#1662)

* Set RULES_GO_VERSION to 0.15.0 (#1665)

* Announce release 0.15.0 [skip ci] (#1666)

* one character fix to README boilerplate [skip ci] (#1667)

* doc: fix grammatical error (#1671)
@krishicks
Copy link

I'm running into an issue around golang.org/x/sys/unix with 0.15.3. I've only noticed this failing when cross-compiling from macOS to a Linux target. Is this the same error that was meant to be fixed in 0.14.1 or is it different? Do I need to declare the repository?

INFO: From GoStdlib external/io_bazel_rules_go/linux_amd64_stripped/stdlib%/pkg:
# os/user
bazel-out/darwin-fastbuild/bin/external/io_bazel_rules_go/linux_amd64_stripped/stdlib%/src/os/user/getgrouplist_unix.go:16:35: warning: passing 'gid_t *' (aka 'unsigned int *') to parameter of type 'int *' converts between pointers to integer types with different sign [-Wpointer-sign]
/usr/include/unistd.h:653:43: note: passing argument to parameter here
ERROR: /Users/kris/workspace/lightstep/go/src/github.com/lightstep/vendor/google.golang.org/grpc/internal/channelz/BUILD:3:1: GoCompile vendor/google.golang.org/grpc/internal/channelz/linux_amd64_stripped/go_default_library%/vendor/google.golang.org/grpc/internal/channelz.a failed (Exit 1)
GoCompile: missing strict dependencies:
	/private/var/tmp/_bazel_kris/3ec06438aa9ef4cec825e8469e603db0/sandbox/darwin-sandbox/406/execroot/__main__/vendor/google.golang.org/grpc/internal/channelz/types_linux.go: import of "golang.org/x/sys/unix"
Known dependencies are:
	google.golang.org/grpc/connectivity
	google.golang.org/grpc/credentials
	google.golang.org/grpc/grpclog
Check that imports in Go sources match importpath attributes in deps.

@krishicks
Copy link

This is the BUILD file gazelle made for google.golang.org/grpc/internal/channelz:

load("@io_bazel_rules_go//go:def.bzl", "go_library")

go_library(
    name = "go_default_library",
    srcs = [
        "funcs.go",
        "types.go",
        "types_linux.go",
        "types_nonlinux.go",
        "util_linux_go19.go",
        "util_nonlinux_pre_go19.go",
    ],
    importmap = "vendor/google.golang.org/grpc/internal/channelz",
    importpath = "google.golang.org/grpc/internal/channelz",
    visibility = ["//vendor/google.golang.org/grpc:__subpackages__"],
    deps = [
        "//vendor/google.golang.org/grpc/connectivity:go_default_library",
        "//vendor/google.golang.org/grpc/credentials:go_default_library",
        "//vendor/google.golang.org/grpc/grpclog:go_default_library",
    ] + select({
        "@io_bazel_rules_go//go/platform:linux": [
            "//vendor/golang.org/x/sys/unix:go_default_library",
        ],
        "//conditions:default": [],
    }),
)

@krishicks
Copy link

It's also possible I'm just doing something stupid here. I'm using rules_docker's container_image to try and duplicate our Dockerfile-based images, and I wonder how bazel knows to compile a Linux binary for that container image. I'll keep investigating.

@krishicks
Copy link

krishicks commented Sep 22, 2018

My issue was fixed by specifying --platforms; is this the optimal way to go or is there an alternative?

bazel run --platforms=@io_bazel_rules_go//go/toolchain:linux_amd64

@krishicks
Copy link

To close this out, it seems --platforms is the way to go. From rules_go:

...it's almost always better to cross-compile by setting --platforms on the command line instead.

@jayconrod
Copy link
Contributor

+1 to using --platforms. The goos and goarch attributes ignore select expressions, and we need them in several places to avoid trying to build dependencies that are unbuildable on certain platforms.

Bazel folks are working on exposing cross-platform transitions in the Starlark API. That should launch in the next few months. I'm hoping that will allow us to re-implement goos and goarch so they work the way you'd expect.

@skyl
Copy link

skyl commented Jan 21, 2019

@krishicks I get much the same file from gazelle; but, I'm trying to run bazel test on darwin. So, using --platforms doesn't seem to be right and doesn't seem to work.

I'm vendoring everything with # gazelle:proto disable_global and using go mod.

So, my normal workflow is like:

cd to/my/gomod/file
if [ "$1" = "update" ]; then
    GO111MODULE=on go get -u
fi
GO111MODULE=on go mod vendor
GO111MODULE=on go mod tidy
bazel run //:gazelle
bazel test //...

I get the same GoCompile: missing strict dependencies: ... "golang.org/x/sys/unix"

My workaround for now is to modify the generated build files for, in my case:

  • vendor/google.golang.org/grpc/internal/channelz/BUILD.bazel
  • vendor/google.golang.org/grpc/internal/syscall/BUILD.bazel

If I manually make the deps like:

    deps = [
        "//go/src/vendor/google.golang.org/grpc/connectivity:go_default_library",
        "//go/src/vendor/google.golang.org/grpc/credentials:go_default_library",
        "//go/src/vendor/google.golang.org/grpc/grpclog:go_default_library",
    ] + select({
        "@io_bazel_rules_go//go/platform:linux": [
            "//go/src/vendor/golang.org/x/sys/unix:go_default_library",
        ],
        "//conditions:default": [
            **"//go/src/vendor/golang.org/x/sys/unix:go_default_library", # keep**
        ],
    }),

and

    deps = select({
        # ...
        "@io_bazel_rules_go//go/platform:darwin": [
            **"//go/src/vendor/golang.org/x/sys/unix:go_default_library", # keep **
            "//go/src/vendor/google.golang.org/grpc/grpclog:go_default_library",
        ],

then everything seems to work as expected. The problem then is, if I let go mod rewrite the vendor directory, I have to add back these lines to the dependencies. @jayconrod any suggestions for a workaround or maybe I should open a new issue?

@jayconrod
Copy link
Contributor

@skyl Could you open a new issue? Please include a small repro and mention what version of rules_go, Bazel, and Gazelle you're using. I wasn't able to reproduce this on Linux or Darwin.

@kalbasit
Copy link
Contributor

+1 to using --platforms. The goos and goarch attributes ignore select expressions, and we need them in several places to avoid trying to build dependencies that are unbuildable on certain platforms.

Bazel folks are working on exposing cross-platform transitions in the Starlark API. That should launch in the next few months. I'm hoping that will allow us to re-implement goos and goarch so they work the way you'd expect.

@jayconrod was this fixed upstream? I got a report today someone failing to build docker image (with rules_docker) of a go_binary specifying both goos and goarch.

@jayconrod
Copy link
Contributor

Yes, it's fixed upstream. The rules_go work still needs to be done though.

@kalbasit
Copy link
Contributor

Yes, it's fixed upstream. The rules_go work still needs to be done though.

Is there an issue we can follow for this fix?

@jayconrod
Copy link
Contributor

I just filed #2219 to track this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants