Initial rules_toolchain implementation #9
Conversation
matts1
force-pushed
the
push-zvlypmywpvpq
branch
2 times, most recently
from
d901782 to
05cd210
Compare
matts1
force-pushed
the
push-zvlypmywpvpq
branch
from
05cd210 to
a4d5035
Compare
matts1
force-pushed
the
push-zvlypmywpvpq
branch
8 times, most recently
from
47fe0af to
6a10f53
Compare
| args_add_strings( | ||
| name = "subcommand_compile", | ||
| actions = ["//lang/toolchain/actions:compile"], | ||
| env = {"LANG_ACTION_TYPE": "compile"}, |
There was a problem hiding this comment.
Slightly confusing to see env on args_add_strings. How does this relate to the added string values?
There was a problem hiding this comment.
env is on all args_add_*, not just args_add_strings. This is a bit of a legacy from the C++ toolchains' flags, which support both args and env. I'm open to changing this. I could create a new rule args_set_env or something similar if you'd prefer, and remove the flag from elsewhere?
| "//lang/toolchain/features:feature1", | ||
| ], | ||
| # If you have tools such as clang which have seperate binaries per exec-target pair (eg. gcc-<exec>-<target>), | ||
| # you can perform a select here over the target platform, then perform a select over the exec platform in the tool. |
There was a problem hiding this comment.
As discussed for rules_cc, this won't work. Since we control the toolchain, could we expose a single exec attr in the right place in which users can use select on the exec platform?
There was a problem hiding this comment.
I came up with an alternative approach to this which I think I prefer. I took inspiration from bazel-lib, and changed it so that the user will explicitly say "I support platform x and platform y", and then it will automatically perform a transition to platform x and y respectively (not the exec platform, which as we've determined in that other bug, is different to those).
What do you think?
examples/rules_lang/lang/toolchain/default_toolchain/BUILD.bazel
Outdated
Show resolved
Hide resolved
matts1
force-pushed
the
push-zvlypmywpvpq
branch
from
6a10f53 to
64e9ec8
Compare
|
@matts1 As a general comment, please don't force push massive PRs as GitHub's review UI just doesn't work well in that case. |
|
Sorry, I'm used to the amend model rather than the "stack additional commits" model that github prefers, and only realized once I'd already amended the big commit. I tried to put most of the important changes in a seperate commit, so the force push of the original commit should only have a few lines of mostly trivial changes. |
No description provided.