[FR/Proposal] Support use of multiple SDK versions in the same bazel workspace

[JamesMBartlett]

I didn't see an FR issue template and the bug template didn't seem relevant, so sorry for not using the template.

Use Case

Pixie would like to be able to build go libraries/binaries with different versions of the go SDK. Pixie has to work with multiple versions of user's go binaries, so we use go binaries built by different versions of go as data inputs to our tests to ensure our code can interact with go binaries regardless of what version built them.

Proposal

I've created a patch of rules_go, that allows specifying gosdk (as a label to the GoSDK provider) on any go_binary or go_test target. Then using a bazel transition all of that targets dependencies will be built with the given SDK. By default the first go_download_sdk/go_host_sdk/go_.*_sdk rule will become the default SDK to use for all rules that don't explicitly specify gosdk.

I've attached a link to the patch below, if you are OK with this sort of change I can add tests, open a PR and continue review there, but I figured I should start a discussion here first since its a non-trivial change. (Note that this patch is based off of v0.32, but I can easily rebase for a PR)

Patch

You can also see this patch in action in our build files here

Also, I'm happy to work on a different solution to building with multiple SDK versions, if people are unhappy with this solution.

Related Issues

This feature request is similar to #2527.

[fmeum]

Being able to select between Go SDK versions sounds very useful. Thanks for the suggestion and willingness to implement it!

I would prefer a slightly more general approach. What do you think about the following? Also @linzhp @achew22.

1. Mimic Bazel's --java_runtime_version with a Starlark string setting such as //go/toolchain:sdk_version (see https://github.com/bazelbuild/bazel/tree/master/tools/jdk for how this is done by the Java rules).

2. Add appropriate version constraints to the toolchains using config_setting_group. E.g., if the SDK is 1.18.3, it should match the values "" and "1.18" and "1.18.3". As a result, if the flag is at its default, any Go SDK is matched. If the flag is at some non-empty value, only the SDKs with that version are matched.

3. Offer a way for users to select a per-target SDK. I would probably prefer a separate rule for that rather than a new attribute on go_binary (we already want to get rid of goos/goarch at some point), but that can be discussed.

I can offer assistance along the way.

[achew22]

+1 to @fmeum. I think moving to a "more standard" configuration would probably be a good path forward. If we were using toolchains in the more traditional sense, it would be possible to add the toolchain as a transition in a rule external to go_binary (go_distributable_binary?). Then we aren't managing our own targeting independent of transitions, which have proven to be complex to maintain and debug.

[JamesMBartlett]

@fmeum That makes sense to me. I understand # 1 and # 2. I'm a little confused what you mean by a separate rule for # 3.

[fmeum]

I'm thinking of a rule that allows the following:

go_binary(
    name = "foo",
    ...
)

go_cross(
    name = "special_foo",
    target = "foo",
    platform = "//:my_custom_platform",
    sdk_version = "1.17",
)

The name and attributes are of course TBD, but the general concept should be a simple wrapper around a go_binary target with the ability to transition on platform and toolchain.

[kmicklas]

I'm planning on implementing this soon, if there are no other attempts yet.

[fmeum]

@kmicklas Thanks! Let me know in case you get stuck, happy to provide guidance.

[JamesMBartlett]

I have an attempt, that I can clean up and put out in the next couple days, unless your timeline is faster

[JamesMBartlett]

Created 2 PRs (#3260, #3261) to support this use case. The first adds the sdk_version build flag. The second adds go_cross, using the added sdk_version flag to transition on sdk version.

[kmicklas]

Thanks @JamesMBartlett! You beat me to it.

[mashail]

thank you @JamesMBartlett we are looking for it