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

CGO linker options for _all.o #790

Closed
emcfarlane opened this issue Sep 4, 2017 · 7 comments
Closed

CGO linker options for _all.o #790

emcfarlane opened this issue Sep 4, 2017 · 7 comments

Comments

@emcfarlane
Copy link

Building a cgo library with custom linker options fails to compile as the linker doesn't receive the correct flags. With bazel build --linkopt="-no-pie", from this issue #252, the build fails to generate go_default_library.cgo.dir/_all.o'] https://github.com/bazelbuild/rules_go/blob/master/go/private/rules/cgo.bzl#L238 as the flag is not set in the command.

@jayconrod
Copy link
Contributor

Confirmed this issue in #646.

The correct way to pass link flags to cgo code is with the clinkopts attribute. These flags need to be handled in multiple places, and I wouldn't expect --linkopt to work for that. Use select expressions if you need different link flags in different configurations.

That being said, it appears that we are passing clinkopts when we process _all.o, but they aren't propagated to the external linker after that.

@emcfarlane
Copy link
Author

@jayconrod I wanted to look at this locally, is there a good way to test on a different project without pushing to a git repository?

@jayconrod
Copy link
Contributor

Work on rules_go locally you mean? My usual setup is to have a local clone of rules_go, then reference it from WORKSPACE in another test project using local_repository. Something like this:

local_repository(
    name = "io_bazel_rules_go",
    path = "../rules_go",
)
load("@io_bazel_rules_go//go:def.bzl", "go_rules_dependencies", "go_register_toolchains")

go_rules_dependencies()
go_register_toolchains()

@ianthehat
Copy link
Contributor

I use a bash alias for bazel (bl) that sets --override_repository io_bazel_rules_go=/my/local/path

@emcfarlane
Copy link
Author

On current master branch running on Ubuntu 17.04 all tests fail with:

/usr/bin/ld.gold: fatal error: -pie and -r are incompatible

I'm not sure what the best way to solve this issue is. The simplest solution I found was to include "-no-pie" flag as part of the toolchain declaration emcfarlane@91a863c . However this may cause issues on other linux platforms. Can we access the clinkopts to all the users to specify build flags with //bazel test --linkopt="-no-pie" //.... This would make it easy to include in a bazel config. Another option would be to check supported flags using something like https://github.com/golang/go/blob/33cb1481f26c6f1acca445fafa18e7ad1d49efed/src/cmd/go/internal/work/build.go#L3229 . How would this command be written as a bazel rule? An action with a text file out? Can you capture failed actions?

@jayconrod
Copy link
Contributor

@afking I spent some time looking at this issue again this morning. I think your suggestion is exactly what we need to do: create a new internal rule to test which flags are supported by the external linker, then pass the results through a text file to the _cgo_object rule.

I need to confirm with @ianthehat before implementing this (he knows more about toolchains). A dynamic check like this seems kludgy, but I don't think we can determine whether the external linker supports these flags when Go toolchains are declared. We could technically test what the local gcc supports, but it might not be the external linker used later.

Thanks for looking into this. I'm sorry this bug has been open for so long.

@jayconrod
Copy link
Contributor

An update: I discussed with a few people, including Ian Taylor, who sent out https://golang.org/cl/64793, which removes the partial link from the go tool. Turns out it's not necessary anymore. I plan to remove it from the Bazel rules as well.

I'll close this issue now as a duplicate of #252, which I hope to resolve late this week or early next week.

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

No branches or pull requests

3 participants