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

feat: windows toolchain for running clang tools #342

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

peakschris
Copy link
Contributor

This is a start on windows toolchain support (#4). Enough is in place for the distribution to be downloaded, extracted, and tools made available through symlinks and a build file.

working examples:
bazel run @llvm_toolchain_llvm//:bin/clang-tidy.exe
bazel run @llvm_toolchain_llvm//:bin/clang-format.exe

llvm 18+ is required.

Comments, suggestions, help all very welcome

toolchain/BUILD.llvm_repo_windows Outdated Show resolved Hide resolved
toolchain/internal/configure.bzl Outdated Show resolved Hide resolved
toolchain/internal/configure.bzl Outdated Show resolved Hide resolved
toolchain/internal/release_name.bzl Outdated Show resolved Hide resolved
Copy link
Collaborator

@rrbutani rrbutani left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@peakschris Thanks for taking a stab at this.

I'm not super familiar with Windows C/C++ development but I left some notes.

If you haven't come across it already, #154 may be of interest to you; it uses cc_toolchain_config from @bazel_tools//tools/cpp:windows_cc_toolchain_config.bzl which we'll probably also want to eventually do.

toolchain/BUILD.llvm_repo_windows Show resolved Hide resolved
toolchain/BUILD.llvm_repo_windows Outdated Show resolved Hide resolved
toolchain/BUILD.llvm_repo_windows Outdated Show resolved Hide resolved
toolchain/BUILD.llvm_repo_windows Outdated Show resolved Hide resolved
@@ -0,0 +1,140 @@
# Copyright 2021 The Bazel Authors.
Copy link
Collaborator

@rrbutani rrbutani Jun 17, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WDYT about unifying this with BUILD.llvm_repo? It seems like the discrepancies are mostly just in the file extensions (.exe, .lib for Windows).

Maybe something like:

is_windows = %{is_windows} # provided by `llvm_repo`
executable_extension = ".exe" if is_windows else ""

filegroup(
  name = "clang",
  srcs = [
    "bin/" + e + executable_extension for e in [
      "clang", "clang++", "clang-cpp", "clang-cl",
    ]
  ],
)

# ...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This sounds good but I'm not sure how the templating would work, could you elaborate?

Comment on lines +53 to +56
filegroup(
name = "bin",
srcs = glob(["bin/**"]),
)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not very familiar with Windows but IIUC the binaries require the DLLs in bin/ in the Windows release tarball to function; I think we need to add them to bin and clang, as, ar, etc?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't need the DLLs as I already have them installed via MSVC on our systems. But for others without MSVC this is a good point. Have a look at my fix, as I don't know how these filegroups will be consumed downstream I'm not sure if the DLLs should go in srcs or data or simply allow downstream to consume separately from the dlls filegroup.

@peakschris
Copy link
Contributor Author

If you haven't come across it already, #154 may be of interest to you; it uses cc_toolchain_config from @bazel_tools//tools/cpp:windows_cc_toolchain_config.bzl which we'll probably also want to eventually do.

This is a great reference, thank you! I don't understand cross compilation, but could give a go at integrating the windows_cc_toolchain_config to a level where it works on a windows machine. I'd like to do this as a followup PR if that's OK. Maybe someone else could take on the cross compilation aspect, I can see that would be hugely valuable for CI runners.

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.

3 participants