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.

Sign up for GitHub

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

Jump to bottom

See if we can use clang-cl to build Mozc for Windows with Bazel #1179

Open
yukawa opened this issue Feb 9, 2025 · 1 comment
Open

See if we can use clang-cl to build Mozc for Windows with Bazel #1179

yukawa opened this issue Feb 9, 2025 · 1 comment
Assignees
@yukawa

Comments

@yukawa
Copy link
Collaborator

yukawa commented Feb 9, 2025

Description

Spun off from the following issues.

The build failure in protobuf with bazel and cl.exe due to path length limit has been an issue in Mozc as well.
As you can see below, we are now passing --output_base=C:/x to shorten the absolute path.

mozc/src/windows.bazelrc

Lines 10 to 15 in 3baa8d8

# There has been a long standing path-length issue on building protobuf on
# Windows with bazel. See the following for details.
# https://github.com/protocolbuffers/protobuf/issues/12947
# https://github.com/bazelbuild/bazel/issues/18683
# Here is an ugly workaround, which we really hope to get rid of.
startup --output_base=C:/x --windows_enable_symlinks

If protobuf team is even giving up this workaround, maybe it's time for us to switch to clang-cl.exe as well.

Let's see what tasks need to be done to switch to clang-cl.exe in Mozc.

Version or commit-id

3baa8d8

Environment

  • OS: Windows 24H2
@yukawa yukawa self-assigned this Feb 11, 2025
hiroyuki-komatsu added a commit that referenced this issue Feb 12, 2025
@hiroyuki-komatsu
Rollback the version of third_party/protobuf from v29.3 to v29.1
2da6c96 
Due to the build error on Windows (#1179).
@yukawa yukawa mentioned this issue Feb 12, 2025
Open
@yukawa
Copy link
Collaborator Author

yukawa commented Feb 13, 2025

One of the blocker is the fact that clang-cl configured with rules_cc does not support to generate 32-bit executables right now.

yukawa added a commit to yukawa/mozc that referenced this issue Feb 15, 2025
@yukawa
Patch rules_cc 0.0.17 to use clang-cl for x86
a6e77f6 
This is a preparation to build Mozc for Windows with clang-cl (google#1179).

This commit pulls my pull request [1] to let rules_cc register clang-cl
as a valid toolchain to build x86 (32-bit) Windows executables. While
when and how my pull request will get merged into upstream, having these
local patches allows us to go ahead to see if we can fully migrate to
clang-cl or not.

There must be no impact on Bazel with msvc build in this commit.

 [1]: bazelbuild/rules_cc#360
@yukawa yukawa mentioned this issue Feb 15, 2025
Merged
hiroyuki-komatsu pushed a commit that referenced this issue Feb 18, 2025
@yukawa
Patch rules_cc 0.0.17 to use clang-cl for x86 (#1186)
76343ff 
This is a preparation to build Mozc for Windows with clang-cl (#1179).

This commit pulls my pull request [1] to let rules_cc register clang-cl
as a valid toolchain to build x86 (32-bit) Windows executables. While
when and how my pull request will get merged into upstream, having these
local patches allows us to go ahead to see if we can fully migrate to
clang-cl or not.

There must be no impact on Bazel with msvc build in this commit.

 [1]: bazelbuild/rules_cc#360

PiperOrigin-RevId: 728118916
yukawa added a commit to yukawa/mozc that referenced this issue Feb 18, 2025
@yukawa
Extract out ProgressPrinter
ca202a1 
This is a preparation before supporting 'clang-cl' for Windows (google#1179).

Our plan is to download LLVM for Windows in 'update_deps.py', and in
that process we would like to use 'ProgressPrinter' to show progress for
both downloading the archive and extracting it.

To avoid code duplicate between 'build_qt.py' and 'update_deps.py',
let's extract 'ProgressPrinter' into a new file first.

This is a mechanical refactoring commit. There must be no observable
behavior change.
@yukawa yukawa mentioned this issue Feb 18, 2025
Merged
hiroyuki-komatsu pushed a commit that referenced this issue Feb 21, 2025
@yukawa
Extract out ProgressPrinter (#1190)
5c0b4bd 
This is a preparation before supporting 'clang-cl' for Windows (#1179).

Our plan is to download LLVM for Windows in 'update_deps.py', and in
that process we would like to use 'ProgressPrinter' to show progress for
both downloading the archive and extracting it.

To avoid code duplicate between 'build_qt.py' and 'update_deps.py',
let's extract 'ProgressPrinter' into a new file first.

This is a mechanical refactoring commit. There must be no observable
behavior change.

PiperOrigin-RevId: 729457855
yukawa added a commit to yukawa/mozc that referenced this issue Feb 21, 2025
@yukawa
Download LLVM to use clang-cl for Window
863ab9a 
As a preparation to start using clang-cl for Windows (google#1179), with this
commit

  python build_tools/update_deps.py

will start deploying LLVM for windows into

  src/third_party/llvm/

by downloading the archive from LLVM's GitHub releases page.

The downloaded clang-cl is not yet used.
@yukawa yukawa mentioned this issue Feb 21, 2025
Open
yukawa added a commit to yukawa/mozc that referenced this issue Feb 21, 2025
@yukawa
Download LLVM to use clang-cl for Window
a6267bb 
As a preparation to start using clang-cl for Windows (google#1179), with this
commit

  python build_tools/update_deps.py

will start deploying LLVM for windows into

  src/third_party/llvm/

by downloading the archive from LLVM's GitHub releases page.

The downloaded clang-cl is not yet used.
yukawa added a commit to yukawa/mozc that referenced this issue Feb 21, 2025
@yukawa
Download LLVM to use clang-cl for Window
69bf7f5 
As a preparation to start using clang-cl for Windows (google#1179), with this
commit

  python build_tools/update_deps.py

will start deploying LLVM for windows into

  src/third_party/llvm/

by downloading the archive from LLVM's GitHub releases page.

The downloaded clang-cl is not yet used.
yukawa added a commit to yukawa/mozc that referenced this issue Feb 23, 2025
@yukawa
Introduce platform host-windows-clang-cl
f3b3e6d 
This is a preparation to build Mozc for Windows with clang-cl (google#1179).

Now that Windows Bazel builds rely on CC toolchain resolution (google#1112),
'clang-cl' needs to be picked up based on 'platform' specified to Bazel.
This gives us an inverse problem what needs to be passed to Bazel so
that such a resolution can happen.

The simplest solution as far as I have figures out is doing the
following two steps.

1. Specify '--host_platform=//:host-windows-clang-cl', where it is
   defined as follows.

     platform(
         name = "host-windows-clang-cl",
         constraint_values = [
             "@platforms//os:windows",
             "@bazel_tools//tools/cpp:clang-cl",
         ],
         parents = ["@local_config_platform//:host"],
     )

   By doing this, '@bazel_tools//tools/cpp:clang-cl' can be propagated
   into each execution platform, which is mandatory for 'clang-cl' to be
   picked up.

2. Specify '--extra_toolchains' command line options to put the
   following toolchains from 'rules_cc' in higher priority than cl.exe.

     * '@local_config_cc//:cc-toolchain-x64_x86_windows-clang-cl'
     * '@local_config_cc//:cc-toolchain-x64_windows-clang-cl'

   This is important because CC toolchain resolution picks up the first
   toolchain that satisfies all the constraints. To give a higher
   priority to 'clang-cl' toolchains, they need to be registered before
   'cl' toolchains are registered.

   Note that

     register_toolchains("@local_config_cc//:all")

   in 'MODULE.bazel' registers toolchains in the alphabeticall order. To
   give a higher priority to 'clang-cl' toolchains, '--extra_toolchains'
   command line looks to be the best way.

What this commit does is the step 1. Without the step 2, there must be
no observable behavior change yet.
@yukawa yukawa mentioned this issue Feb 23, 2025
Open
yukawa added a commit to yukawa/mozc that referenced this issue Feb 23, 2025
@yukawa
Introduce platform host-windows-clang-cl
282cfd6 
This is a preparation to build Mozc for Windows with clang-cl (google#1179).

Now that Windows Bazel builds rely on CC toolchain resolution (google#1112),
'clang-cl' needs to be picked up based on 'platform' specified to Bazel.
This gives us an inverse problem what needs to be passed to Bazel so
that such a resolution can happen.

The simplest solution as far as I have figures out is doing the
following two steps.

1. Specify '--host_platform=//:host-windows-clang-cl', where it is
   defined as follows.

     platform(
         name = "host-windows-clang-cl",
         constraint_values = [
             "@platforms//os:windows",
             "@bazel_tools//tools/cpp:clang-cl",
         ],
         parents = ["@local_config_platform//:host"],
     )

   By doing this, '@bazel_tools//tools/cpp:clang-cl' can be propagated
   into each execution platform, which is mandatory for 'clang-cl' to be
   picked up.

2. Specify '--extra_toolchains' command line options to put the
   following toolchains from 'rules_cc' in higher priority than cl.exe.

     * '@local_config_cc//:cc-toolchain-x64_x86_windows-clang-cl'
     * '@local_config_cc//:cc-toolchain-x64_windows-clang-cl'

   This is important because CC toolchain resolution picks up the first
   toolchain that satisfies all the constraints. To give a higher
   priority to 'clang-cl' toolchains, they need to be registered before
   'cl' toolchains are registered.

   Note that

     register_toolchains("@local_config_cc//:all")

   in 'MODULE.bazel' registers toolchains in the alphabetical order. To
   give a higher priority to 'clang-cl' toolchains, '--extra_toolchains'
   command line looks to be the best way.

What this commit does is the step 1. Without the step 2, there must be
no observable behavior change yet.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@yukawa yukawa

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@yukawa