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

Update go_toolchain to use execution transitons. #2286

Merged
merged 3 commits into from
Jan 9, 2020
Merged

Update go_toolchain to use execution transitons. #2286

merged 3 commits into from
Jan 9, 2020

Conversation

katre
Copy link
Member

@katre katre commented Nov 19, 2019

No description provided.

@jayconrod
Copy link
Contributor

Is cfg = "host" being renamed to cfg = "exec"? What is the minimum Bazel version that supports that?

The current minimum Bazel version for rules_go is 0.23.0, but we'll make 1.1.0 the minimum somewhat soon.

@katre
Copy link
Member Author

katre commented Nov 19, 2019

Host and exec behave differently. Host is the current host transition: there's only one, it depends on the default flag values, and is set for building tools to be invoked on Host. The execution transition, on the other hand, allows for multiple different execution platforms and is more flexible.

Eventually I want to move everything to use the execution transition and get rid of the host, but that's not feasible anytime soon.

The execution transition was added before 1.0, but I would need to search to see what version exactly.

This draft PR was just created to run the tests and see if there are obvious failures. Do you have a way to run downstream projects with a release candidate?

@jayconrod
Copy link
Contributor

Ah, interesting. My understanding was that cfg = "host" actually meant the execution platform (a misnomer from way back). So anywhere that appears in rules_go, cfg = "exec" is what's actually intended.

This draft PR was just created to run the tests and see if there are obvious failures. Do you have a way to run downstream projects with a release candidate?

I'm afraid not. Kubernetes is the biggest open source client but they're not using the most recent version. Most other clients are closed-source.

@katre katre requested a review from jayconrod January 9, 2020 13:45
@katre katre marked this pull request as ready for review January 9, 2020 13:45
@katre katre requested a review from ianthehat as a code owner January 9, 2020 13:45
@katre
Copy link
Member Author

katre commented Jan 9, 2020

Thanks for the additional commit. If you're fine with this change let's go ahead and commit (it should be a no-op until the actual toolchain transition changes land).

@katre
Copy link
Member Author

katre commented Jan 9, 2020

Is there a way to kick off tests again?

@jayconrod
Copy link
Contributor

Ran the tests again through BuildKite. Looks good. Thanks for sending this.

(I resurrected this since the rules_go minimum version of Bazel is now 1.2.0).

@jayconrod jayconrod merged commit 88bc72a into bazelbuild:master Jan 9, 2020
@steeve
Copy link
Contributor

steeve commented Jan 9, 2020

For the record, I ran into problems with the exec transition on my custom rules (it would fail to detect Android toolchain).
Not sure how the reason, I'll try again and report back here if I run into issues.

@jayconrod
Copy link
Contributor

@steeve Interesting, please do open an issue if you're able to reproduce that.

I'm hoping to get a rules_go release out today with this, but we can do a point release if it causes problems.

@katre
Copy link
Member Author

katre commented Jan 9, 2020

If you see an exec transition failure, please open a bug and let me know.

The change here is a no-op: toolchains are always in the host transition currently (until bazelbuild/bazel#10523 is done), and the host transition is terminal, so these cfg changes don't change anything.

@katre katre deleted the use-exec-trans branch January 9, 2020 16:23
@steeve
Copy link
Contributor

steeve commented Apr 18, 2020

Reviving this issue.

Having a go_binary -> cc_import -> android_binary breaks with cfg = "exec" with the following:

ERROR: /private/var/tmp/_bazel_steeve/8c5731f383ec5885e1a8afb0f4a77ff6/external/androidndk/BUILD.bazel:41:1: in cc_toolchain_suite rule @androidndk//:toolchain-libcpp: cc_toolchain_suite '@androidndk//:toolchain-libcpp' does not contain a toolchain for cpu 'darwin'

I bisected the problem down to 88bc72a. Hence me here :)

@katre do you want me to open an issue on bazel itself ?

@katre
Copy link
Member Author

katre commented Apr 20, 2020

@steeve Can you please file an issue on Bazel with detailed reproduction steps? I'll look at is as soon as I can.

@steeve
Copy link
Contributor

steeve commented Apr 20, 2020

Sure thing!

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

Successfully merging this pull request may close these issues.

None yet

4 participants