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

Stop relying on --noincompatible_enable_cc_toolchain_resolution in Windows build #1112

Closed
yukawa opened this issue Nov 2, 2024 · 0 comments · Fixed by #1156
Closed

Stop relying on --noincompatible_enable_cc_toolchain_resolution in Windows build #1112

yukawa opened this issue Nov 2, 2024 · 0 comments · Fixed by #1156
Assignees
@yukawa

Comments

@yukawa
Copy link
Collaborator

yukawa commented Nov 2, 2024

Description

Follows up to the following issue.

As a long term solution, we should complete the migration to platforms in Windows build.

This is not a release blocker of our migration from GYP to Bazel though.

Steps to reproduce

Steps to reproduce the behavior:

  1. Build Mozc64.msi without specifying --noincompatible_enable_cc_toolchain_resolution

Expected behavior

There is no regression.

Actual behavior

Currently mozc_tip32.dll cannot be built as a 32-bit binary.

Version or commit-id

e1b40ed

Environment

  • OS: Windows 11 23H2
@yukawa yukawa self-assigned this Nov 2, 2024
yukawa added a commit to yukawa/mozc that referenced this issue Dec 28, 2024
@yukawa
Rely on CC toolchain resolution on Windows
df80deb 
This is a follow up commit to my previous commit [1], which updated the
Bazel version from 7.4.1 to 8.0.0.

It turns out that

  --noincompatible_enable_cc_toolchain_resolution

is now no-op in Bazel 8.0. Thus there remains no way other than fully
migrating to the new CC toolchain resolution as planned in google#1112.
Otherwise 'mozc_tip32.dll' will be built as a 64-bit executable (google#1102).

This commit consists of two parts:

 1. explicitly register CC toolchains in 'windows.bazelrc'.
 2. Switch from '--cpu' commandline option to '--platforms' commandline
    option in '_win_executable_transition'.

With above 'mozc_tip32.dll' will be built as a 32-bit executable again.

Closes google#1102.
Closes google#1112.

 [1]: 6dadef1
yukawa added a commit to yukawa/mozc that referenced this issue Dec 28, 2024
@yukawa
Rely on CC toolchain resolution on Windows
6adc9de 
This is a follow up commit to my previous commit [1], which updated the
Bazel version from 7.4.1 to 8.0.0 (google#1118).

It turns out that

  --noincompatible_enable_cc_toolchain_resolution

is now no-op in Bazel 8.0. Thus there remains no way other than fully
migrating to the new CC toolchain resolution as planned in google#1112.
Otherwise 'mozc_tip32.dll' will be built as a 64-bit executable (google#1102).

This commit consists of two parts:

 1. explicitly register CC toolchains in '.bazelrc'.
 2. Switch from '--cpu' commandline option to '--platforms' commandline
    option in '_win_executable_transition'.

With above 'mozc_tip32.dll' will be built as a 32-bit executable again.

Closes google#1102.
Closes google#1112.

 [1]: 6dadef1
@yukawa yukawa mentioned this issue Dec 28, 2024
Merged
yukawa added a commit to yukawa/mozc that referenced this issue Dec 28, 2024
@yukawa
Rely on CC toolchain resolution on Windows
15f7dae 
This is a follow up commit to my previous commit [1], which updated the
Bazel version from 7.4.1 to 8.0.0 (google#1118).

It turns out that

  --noincompatible_enable_cc_toolchain_resolution

is now no-op in Bazel 8.0. Thus there remains no way other than fully
migrating to the new CC toolchain resolution as planned in google#1112.
Otherwise 'mozc_tip32.dll' will be built as a 64-bit executable (google#1102).

This commit consists of two parts:

 1. explicitly register CC toolchains in '.bazelrc'.
 2. Switch from '--cpu' commandline option to '--platforms' commandline
    option in '_win_executable_transition'.

With above 'mozc_tip32.dll' will be built as a 32-bit executable again.

Closes google#1102.
Closes google#1112.

 [1]: 6dadef1
yukawa added a commit to yukawa/mozc that referenced this issue Jan 5, 2025
@yukawa
Rely on CC toolchain resolution on Windows
acfd870 
This is a follow up commit to my previous commit [1], which updated the
Bazel version from 7.4.1 to 8.0.0 (google#1118).

It turns out that

  --noincompatible_enable_cc_toolchain_resolution

is now no-op in Bazel 8.0. Thus we have no other choice than fully
migrating to the new CC toolchain resolution as planned in google#1112.
Otherwise 'mozc_tip32.dll' will be built as a 64-bit executable (google#1102).

This commit consists of two parts:

 1. explicitly register CC toolchains in '.bazelrc'.
 2. Switch from '--cpu' commandline option to '--platforms' commandline
    option in '_win_executable_transition'.

With above 'mozc_tip32.dll' will be built as a 32-bit executable again.

Closes google#1102.
Closes google#1112.

 [1]: 6dadef1
yukawa added a commit to yukawa/mozc that referenced this issue Jan 20, 2025
@yukawa
Register CC toolchain from rules_cc
b4d82ec 
This reworks my previous PR google#1156 [1] to make Windows bazel build
compatible with

  --noincompatible_enable_cc_toolchain_resolution

option, which is now unconditionally enabled (google#1102) (google#1112).

The difference from the previous approach is that this commit uses
CC toolchains defined by 'rules_cc' rather than the ones implicitly
defined by the Bazel itself.

Given that 'rules_cc' becomes the new home of CC-related rules and
configurations, hopefully this change will reduce the likelihood of
future troubles.

There must be no observable behavior change in the final artifacts.

 [1]: 4aad25e
yukawa added a commit to yukawa/mozc that referenced this issue Jan 20, 2025
@yukawa
Register CC toolchain from rules_cc
1d72af8 
This reworks my previous PR google#1156 [1] to make Windows bazel build
compatible with

  --noincompatible_enable_cc_toolchain_resolution

option, which is now unconditionally enabled (google#1102) (google#1112).

The difference from the previous approach is that this commit uses
CC toolchains defined by 'rules_cc' rather than the ones implicitly
defined by the Bazel itself.

Given that 'rules_cc' becomes the new home of CC-related rules and
configurations, hopefully this change will reduce the likelihood of
future troubles.

There must be no observable behavior change in the final artifacts.

 [1]: 4aad25e
@yukawa yukawa mentioned this issue Jan 20, 2025
Merged
yukawa added a commit to yukawa/mozc that referenced this issue Jan 21, 2025
@yukawa
Register CC toolchain from rules_cc
de6ea52 
This reworks my previous PR google#1156 [1], which was to make Windows bazel
build compatible with

  --noincompatible_enable_cc_toolchain_resolution

option (google#1102) (google#1112).

The difference from the previous approach is that this commit uses
CC toolchains defined by 'rules_cc' rather than the ones implicitly
defined by the Bazel itself.

Given that 'rules_cc' becomes the new home of CC-related rules and
configurations, hopefully this change will reduce the likelihood of
future troubles.

There must be no observable behavior change in the final artifacts.

 [1]: 4aad25e
yukawa added a commit to yukawa/mozc that referenced this issue Jan 21, 2025
@yukawa
Register CC toolchain from rules_cc
d610d15 
This reworks my previous PR google#1156 [1], which was to make Windows bazel
build compatible with

  --noincompatible_enable_cc_toolchain_resolution

option (google#1102) (google#1112).

The difference from the previous approach is that this commit uses
CC toolchains defined by 'rules_cc' rather than the ones implicitly
defined by the Bazel itself.

Given that 'rules_cc' becomes the new home of CC-related rules and
configurations, hopefully this change will reduce the likelihood of
future troubles.

There must be no observable behavior change in the final artifacts.

 [1]: 4aad25e
yukawa added a commit to yukawa/mozc that referenced this issue Jan 21, 2025
@yukawa
Register CC toolchain from rules_cc
b3f9cdc 
This reworks my previous PR google#1156 [1], which was to make Windows bazel
build compatible with

  --noincompatible_enable_cc_toolchain_resolution

option (google#1102) (google#1112).

The difference from the previous approach is that this commit uses
CC toolchains defined by 'rules_cc' rather than the ones implicitly
defined by the Bazel itself.

Given that 'rules_cc' becomes the new home of CC-related rules and
configurations, hopefully this change will reduce the likelihood of
future troubles.

There must be no observable behavior change in the final artifacts.

 [1]: 4aad25e
hiroyuki-komatsu pushed a commit that referenced this issue Jan 21, 2025
@yukawa
Register CC toolchain from rules_cc (#1163)
b878663 
This reworks my previous PR #1156 [1], which was to make Windows bazel
build compatible with

  --noincompatible_enable_cc_toolchain_resolution

option (#1102) (#1112).

The difference from the previous approach is that this commit uses
CC toolchains defined by 'rules_cc' rather than the ones implicitly
defined by the Bazel itself.

Given that 'rules_cc' becomes the new home of CC-related rules and
configurations, hopefully this change will reduce the likelihood of
future troubles.

There must be no observable behavior change in the final artifacts.

 [1]: 4aad25e

PiperOrigin-RevId: 717738755
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
Merged
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.
hiroyuki-komatsu pushed a commit that referenced this issue Feb 26, 2025
@yukawa
Introduce platform host-windows-clang-cl (#1195)
e3f30a9 
This is a preparation to build Mozc for Windows with clang-cl (#1179).

Now that Windows Bazel builds rely on CC toolchain resolution (#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.

PiperOrigin-RevId: 730872676
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

Successfully merging a pull request may close this issue.

Rely on CC toolchain resolution on Windows Bazel builds yukawa/mozc
1 participant
@yukawa