rules-proto-grpc / rules_proto_grpc

Bazel rules for building Protobuf and gRPC code and libraries from proto_library targets
https://rules-proto-grpc.com
Apache License 2.0
249 stars 156 forks source link

[Platform Transitions] Fix platform transitions for selecting tools. #272

Open ankit-agarwal1999 opened 1 year ago

ankit-agarwal1999 commented 1 year ago

I ran into an issue the other day where I have a multi-platform repo and I was compiling code for a different platform (in this case QNX).

I ran into an issue with the docs compile rule where I would get the following error:

ERROR: /home/jzahn/.cache/bazel/_bazel_jzahn/7bf1254182ebae36d2817c38fe27106c/external/rules_proto_grpc/doc/BUILD.bazel:63:13: configurable attribute "tool" in @rules_proto_grpc//doc:markdown_plugin doesn't match this configuration. Would a default condition help?

Conditions checked:
 @bazel_tools//src/conditions:darwin_arm64
 @bazel_tools//src/conditions:darwin_x86_64
 @bazel_tools//src/conditions:linux_x86_64
 @bazel_tools//src/conditions:windows

The issue was we were compiling for QNX, but even though the tool is for the execution platform, the select operates on the target platform when selecting the tool. So added an alias buffer rule in between the tool and the select so the select would only operate on the execution platform. That seemed to work, so I implemented the buffer for everywhere where I saw this pattern.

ankit-agarwal1999 commented 1 year ago

cc @aaliddell

aaliddell commented 1 year ago

The select shouldn't be taking the target config, as we specifically request the exec config for the tools:

https://github.com/rules-proto-grpc/rules_proto_grpc/blob/7064b28a75b3feb014b20d3276e17498987a68e2/internal/plugin.bzl#L31-L36

The fact the select is failing indicates that your exec config may be an unsupported platform (QNX presumably), which is what needs solving here rather than resorting to host config by introducing the alias. Note that if we actually wanted to switch to host config, changing cfg = "exec" to cfg = "host" would be the easier way rather than introducing the aliases, but this would break remote build when exec != host.

Have you tried constraining the doc building rules with exec_compatible_with etc to prevent it trying to cross-compile the docs rules?

ankit-agarwal1999 commented 1 year ago

Sorry, I am a bit confused by your previous comment.

According to the bazel docs: https://bazel.build/rules/lib/toplevel/attr#parameters_2

They say that cfg can either be exec depending on whether it is building for the execution platform, or target depending on whether it is building for the target platform. I don't think the host platform is involved here (i.e there is no cfg=host). In this case I believe execution platform is the platform where bazel itself is executing on. What I am saying is happening is the proto_plugin rule is being built for the target platform (in my case QNX). Since that rule is built for the target platform, the select chooses based on the target platform, which in my case is incorrect, since the tool itself is to be executed via the execution platform.

Since I believe bazel resolves selects before it actually builds things, my understanding is if I were to compile for the target platform QNX, the proto_plugin rule would resolve its select based on the target platform. But this is incorrect. So we do the alias thing, which builds the alias with a different platform configuration (i.e for the execution platform which is Linux in our case) and that resolves to the correct execution platform.

Let me know if I have any misunderstandings here, but afaik, we aren't executing bazel on QNX. We are building for it, but that would be the target platform (ie cfg=target).

aaliddell commented 1 year ago

So host config was dropped in v6: https://github.com/bazelbuild/bazel/commit/6464f1cbdf14f0b8e8f29f7b57990a40ea584062

That section of code I highlighted above is exactly where we are asking the proto plugin to be produced in the exec config as you are looking for, so the select should be resolved appropriately and the fact you are getting target config there indicates bazel is in the wrong configuration. So switching to an alias select is just hiding the problem rather than actually solving it. Are you using configuration transitions?

Could you follow the steps here to check your config for the relevant targets? As it stands I don't have a cross compiling setup to be able to test with.