Migrate rules_go for incompatible_use_platforms_repo_for_constraints

[hlopko]

bazelbuild/bazel#8622

[jayconrod]

I'm not sure how this passes with the latest Bazel version but fails with 0.23.0 (current minimum supported). Is the platforms repository declared automatically? From the migration instructions, it sounds like it should be declared in go_rules_dependencies in go/private/repositories.bzl.

Is there any need to drop support for Bazel 0.23.0 or will this work automatically back that far? Mainly I'm wondering whether this should be cherry-picked to release branches.

rules_go might make Bazel 1.0.1 the minimum supported version in the next release. We'll need to pick up support for the new build_setting / transition stuff.

[hlopko]

Oh wow I didn't know you support Bazels back to 0.23. Yeah in that case I can add @platforms explicitly (it's bundled in Bazel since 0.28 but if there are @platforms already in the workspace Bazel's version is not activated).

[jayconrod]

Cool, that would be awesome. I try not to break compatibility unless we need to (it was a big user complaint a while back, so lessons learned).

If it's predeclared in Bazel 0.28, does it show up in native.existing_rules()? If not, it might be a little more complicated to avoid overriding the default, but we can probably figure something out.

[hlopko]

Bazel appends stuff to the WORKSPACE file and that's how @platforms are patched. We use the same maybe approach as rules_go, just that our call is below rules_go's. Does it make sense?

[hlopko]

Hmm ok buildkite tells me I broke things :) I'll have to take a look tomorrow.

[jayconrod]

Bazel appends stuff to the WORKSPACE file and that's how @platforms are patched. We use the same maybe approach as rules_go, just that our call is below rules_go's. Does it make sense?

I think so. So that means that native.existing_rules will not contain platforms when go_rules_dependencies calls it in _maybe. So go_rules_dependencies would pick a version here that may be different than what Bazel picks by default.

I'd suggest we add an additional guard:

if not versions.is_at_least("0.28.0", bazel_version = native.bazel_version):
    ...

About BuildKite failures: not sure why this would break toolchain resolution for 0.23.0. I think there was an old version of @platforms that just aliased constraint_values in @bazel_tools. Maybe we could use that?

[hlopko]

Oh I actually think rules_go should override the default platforms repo that is bundled in Bazel. Ideally we wouldn't bundle it in Bazel at all but some things are not ready for that yet. This way rules_go are in control, and you have a good mechanism to update platforms repo should you need to do it (e.g. after adding more constraints that rules_go need).

But I'm happy to add the guard if you prefer that.

Now off to the 0.23 investigations.

[hlopko]

Ok, this is quite complicated. We need to use labels to @bazel_tools//platforms for Bazel < 0.28 and labels to @platforms for >= 0.28. Which we could do using the nice @io_bazel_rules_go_compat repository for analysis phase. We cannot easily use it for files such as //go/platform:list.bzl that is loaded from the repositories.bzl (at at that time compat repository is not yet created). No that bad, we can use plan functions in the repository implementation, such as:

def platforms_cpu_constraint_value(value):
    if not native.bazel_version or versions.parse(native.bazel_version) >= (0, 28, 0):
      return "@platforms//cpu:" + value
    else:
      return "@bazel_tools//platforms:" + value

But there's a problem with that too - go/private/platforms.bzl is currently loaded in both repository context and in rule context. And native.bazel_version is only defined in the repository context.

I might be able to fix that using hasattr, in the spirit of:

def platforms_cpu_constraint_value(value):
    if not hasattr(native, "bazel_version"): 
      # means we are in the rule context and compat repository is already defined
      return "@io_bazel_rules_go_compat//:cpu:" + value
    else if not native.bazel_version or versions.parse(native.bazel_version) >= (0, 28, 0):
      return "@platforms//cpu:" + value
    else:
      return "@bazel_tools//platforms:" + value

I'll try that tomorrow, but if you have better ideas feel free to stop me :)

[jayconrod]

Superceded by #2275