Conversation
| ".", | ||
| "runtimes/musl", | ||
| ], | ||
| target_compatible_with = [ |
There was a problem hiding this comment.
we should probably run buildifier over the repo and check it in to minimize this PR (and also add a CI pass haha cc @cerisier )
| "@glibc//:gnu_libc_headers", | ||
| ], | ||
| "@platforms//os:windows": [ | ||
| "@toolchains_llvm_bootstrapped//platforms/config:abi_gnu": [ |
There was a problem hiding this comment.
i don't have a great suggestion, but I do think it's very confusing to have config:gnu and config:abi_gnu that are very different things :) perhaps windows_gnu and windows_msvc? Or windows_mingw and windows_msvc? not sure :/
There was a problem hiding this comment.
I'll have a think of what defines this best.
Neither libc or abi is good here. But we lack something else for now.
Most probably we'll need to develop those constraints a bit more as a prerequisite. I'll tackle that this week.
|
Just a note, I did not abandon that work, I just am short in time at the moment, but I will return ASAP. |
dzbarsky
left a comment
dzbarsky
left a comment
There was a problem hiding this comment.
Oops realized I had some review comments that were stuck in pending state for a while ..
| name = prefix + "/bin/clang-cl", | ||
| platform = prefix + "_platform", | ||
| actual = "@llvm-project//clang:clang.stripped", | ||
| # TODO: is it also the case for clang-cl? |
There was a problem hiding this comment.
i guess it depends how clang-cl locates the linker; i don't know if it accepts the -fuse-ld=lld-style flag. do you know?
| cc_toolchain( | ||
| name = msvc_cc_toolchain_name, | ||
| tool_map = ":{}_{}/tools_for_msvc".format(exec_os, exec_cpu), | ||
| compiler = "clang", # FIXME: should we change to clang-cl? |
There was a problem hiding this comment.
now that i'm looking at this more, i'm wondering if it's actually necessary to use clang-cl vs normal clang with an msvc target? I guess it depends on how build targets in the ecosystem pass their args? In any case, I do think this this should be clang-cl if the underlying binary is clang-cl, otherwise well-intentioned code that selects on clang-cl instead of @platforms//os:windows (trying to handle mingw properly) will break here, right?
toolchain/cc_toolchain.bzl
Outdated
| "//conditions:default": "//runtimes:dynamic_runtime_lib", | ||
| }), | ||
| compiler = "clang", | ||
| compiler = compiler, # TODO: actually, I'm not sure `rules_cc` supports `clang-cl` yet, so I'm not sure what it would do (https://github.com/bazelbuild/rules_cc/pull/561) |
There was a problem hiding this comment.
yeah i think it's a little half-baked upstream, we might need that to land before this becomes functional, though you could patch it here for experimenting
|
Quick heads up, while this PR was pending we switched to building a single llvm binary in the multicall/BusyBox style (@llvm-project//llvm) and making all the tools symlinks, so you'll want to make some adjustments as you rebase. The advantage is a much cheaper build (fewer links) and overall smaller size |
|
Oh hey, it's Titouan 👋 I am very excited about this PR. We have a user who is interested in using this. |
ArchangelX360
force-pushed
the
ArchangelX360/msvc_support
branch
from
6cc43d9 to
0bdc368
Compare
| }), | ||
| compiler = "clang", | ||
| compiler = select({ | ||
| "@llvm//constraints/abi:msvc": "clang-cl", |
ArchangelX360
force-pushed
the
ArchangelX360/msvc_support
branch
4 times, most recently
from
3607d8b to
dd571e6
Compare
runtimes/msvc/extension/msvc.bzl
Outdated
| return [] | ||
|
|
||
| tag = repository_ctx.attr.winsysroot_version | ||
| repo_base = "https://github.com/ArchangelX360/winsysroot/releases/download/{}".format(tag) |
There was a problem hiding this comment.
move that winsysroot tool to hermeticbuild org
runtimes/msvc/extension/msvc.bzl
Outdated
| "integrity": attr.string( | ||
| doc = "Optional SRI integrity for the downloaded winsysroot binary.", | ||
| ), | ||
| "visual_studio_release": attr.string( |
There was a problem hiding this comment.
Probably should be the MSVC version directly, as I'm pretty sure this will not be reproducible
ArchangelX360
force-pushed
the
ArchangelX360/msvc_support
branch
from
dd571e6 to
952f484
Compare
ArchangelX360
force-pushed
the
ArchangelX360/msvc_support
branch
from
bc1464d to
cb05f18
Compare
ArchangelX360
force-pushed
the
ArchangelX360/msvc_support
branch
from
142573b to
ab7f703
Compare
…able to test on Windows
ArchangelX360
force-pushed
the
ArchangelX360/msvc_support
branch
from
449518d to
b5d3f96
Compare
4 participants
Closes #156