Support MSVC with rule-based toolchain#561
Support MSVC with rule-based toolchain#561calebzulawski wants to merge 2 commits intobazelbuild:mainfrom
Conversation
calebzulawski
requested review from
armandomontanez,
c-mita,
hvadehra,
pzembrod and
trybka
as code owners
calebzulawski
force-pushed
the
msvc
branch
4 times, most recently
from
3026007 to
de3d08d
Compare
calebzulawski
force-pushed
the
msvc
branch
2 times, most recently
from
3a3bcfd to
a16e39b
Compare
|
Wow, this is AWESOME! I'd like to help get this landed in the next week. At first glance this looks pretty good, need to spend a bit more time to look through the rest. |
|
We're also very interested as part of https://github.com/cerisier/toolchains_llvm_bootstrapped ! |
armandomontanez
left a comment
armandomontanez
left a comment
There was a problem hiding this comment.
Sorry for the radio silence. This is awesome work. Posting my initial concern, but I need to do a bit of a deeper dive to make sure the foundational bits are double-checked.
| @@ -0,0 +1,33 @@ | |||
| load("//cc/toolchains:feature.bzl", "cc_feature") | |||
|
|
|||
| package(default_visibility = ["//visibility:public"]) | |||
There was a problem hiding this comment.
This is one piece that jumped out at me. Hyrum's law will likely immediately take effect and people will start depending on these. I think something roughly like this should exist, but I don't want to slip in subtly as part of a larger change.
There was a problem hiding this comment.
Hmm, yeah that makes sense. It's possible that by not providing this, people will instead use the underlying compiler_ feature names to make compiler-specific args, but I figure that's fine for now.
| cc_args_for_compiler( | ||
| name = "output_execpath_windows", | ||
| actions = ["//cc/toolchains/actions:ar_actions"], | ||
| args = ["/OUT:{output_execpath}"], |
There was a problem hiding this comment.
When using llvm-ar, this fails with:
external/llvm++_repo_rules+llvm-toolchain-minimal-22.1.0-darwin-arm64/bin/llvm-ar: error: unknown option /
OVERVIEW: LLVM Archiver
USAGE: llvm-ar [options] [-]<operation>[modifiers] [relpos] [count] <archive> [files]
llvm-ar -M [< mri-script]
[...]
I'm ensure if that should be tweaked at the toolchain code level, or in that rules_cc one. Please advise.
armandomontanez
added
type: feature request
4 participants
Based on #436, but contains the full MSVC implementation.