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

Make platforms targets publicly visible #107

Merged
merged 1 commit into from
Sep 27, 2021

Conversation

dududko
Copy link
Contributor

@dududko dududko commented Sep 27, 2021

Make targets in the platforms package publicly visible so they could be used in the attributes of other targets.
Example:

with_platform(
    name = "linux",
    platform = "@com_grail_bazel_toolchain//platforms:linux-x86_64",
    tool = ":binary",
    visibility = ["//visibility:public"],
)

@rrbutani rrbutani self-assigned this Sep 27, 2021
@rrbutani
Copy link
Collaborator

Thanks for the PR!

Can you tell me a little bit more about your use case? //platforms really only exists for our own tests; I'm a bit wary to have users depend on the stuff there.

In my mind, users would either just use the auto-generated @local_config_platform//:host as their host platform (and let toolchain resolution pick the appropriate toolchain) or define their own platform when cross-compiling or doing something a bit more special.

In your example, what does with_platform do?

@dududko
Copy link
Contributor Author

dududko commented Sep 27, 2021

There are two way to do cross-compilation:

  • by specifying target platform with --platform option
  • using transition

There is a third option in rules_go, where you can specify goos/goarch attributes in go_binary, but this approach is actually based on transitions.

In my case I need to build binary for multiple platforms (darwin/linux) and publish build artifacts. I prefer doing this in a single bazel executable target without specifying --platform option on each build, thus I use transitions. A have a very simple rule for this which is called with_platform, and all this rules does is apply a target platform in the build configuration of a tool attribute, which can be any binary target.

I use this approach when build a custom gazelle gazelle_binary for multiple platforms. This target does not support goos/goarch anymore, so I had to implement a custom transition. And I use the same transition for building go_binary with cgo for multiple platforms using the toolchains that you provide.

I'm a bit wary to have users depend on the stuff there.

In rules_go platforms are also publicly visible, and users depend on those targets. I do no see the reason why not to make your platform targets visible as well.

@rrbutani
Copy link
Collaborator

rrbutani commented Sep 27, 2021

In rules_go platforms are also publicly visible, and users depend on those targets. I do no see the reason why not to make your platform targets visible as well.

My hesitation mostly comes from the fact that while we have a small list of supported target/host platforms at the moment, we eventually hope to support arbitrary target triples in which case we'd want to generate the platforms (as #85 does with constraints). Under that scenario it's potentially problematic to have users depend on the generated platforms since they'd be tied to the value of extra_targets that's provided (I'd hope that, for example, @llvm_toolchain//platforms:wasi only being present if wasm32-unknown-wasi is specified as an extra target is expected and reasonable behavior but I'm not sure this is the case; I think it'd be easy to accidentally assume the generated platforms are always present and to use them for things that have nothing to do with this toolchain).

That said, I see the utility in providing these and I think it makes sense to expose these platforms for now with the caveat that they may move to the generated toolchain repo in a future release (0.7+).

platforms/BUILD.bazel Outdated Show resolved Hide resolved
@dududko dududko force-pushed the platforms-visibility branch from b67c7f0 to 404ffef Compare September 27, 2021 16:15
@rrbutani rrbutani changed the title make platforms targets publicly visible Make platforms targets publicly visible Sep 27, 2021
@rrbutani rrbutani merged commit 4760f4a into bazel-contrib:master Sep 27, 2021
@dududko dududko deleted the platforms-visibility branch September 27, 2021 16:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants