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

Register CC toolchain from rules_cc #1163

Merged
merged 1 commit into from
Jan 21, 2025

Conversation

yukawa
Copy link
Collaborator

@yukawa yukawa commented Jan 20, 2025

Description

This reworks my previous PR #1156 that was to make Windows bazel build compatible with

--noincompatible_enable_cc_toolchain_resolution

option, which is now unconditionally enabled (#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.

Issue IDs

Steps to test new behaviors (if any)

  • OS: Windows 11 24H2
  • Steps:
    1. Build Mozc64.msi with Bazel
    2. Install Mozc64.msi
    3. dumpbin /headers "C:\Program Files (x86)\Mozc\mozc_tip32.dll" | findstr machine
    4. Confirm 14C machine (x86) is shown.

@yukawa yukawa force-pushed the cc_toolchain_from_rules_cc branch from 1d72af8 to de6ea52 Compare January 21, 2025 01:00
@yukawa
Copy link
Collaborator Author

yukawa commented Jan 21, 2025

Sorry, it seems that mac build starts failing with this change. Let us hold until I figure out what's going on.

@yukawa yukawa marked this pull request as draft January 21, 2025 02:19
@yukawa yukawa force-pushed the cc_toolchain_from_rules_cc branch from de6ea52 to d610d15 Compare January 21, 2025 02:24
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 yukawa force-pushed the cc_toolchain_from_rules_cc branch from d610d15 to b3f9cdc Compare January 21, 2025 02:25
@yukawa yukawa marked this pull request as ready for review January 21, 2025 02:43
@yukawa
Copy link
Collaborator Author

yukawa commented Jan 21, 2025

It turns out that there is an order dependency between rules_cc and apple_support.

https://github.com/bazelbuild/apple_support/blob/d87e8b07f3345e750834dbb6ce38c7c7d3b8b44b/README.md#bazel-7-setup

If you also depend on rules_cc, apple_support must come above rules_cc in your MODULE.bazel or WORKSPACE file because Bazel selects toolchains based on which is registered first.

Fixed in the latest commit and the CI looks to be passing.
https://github.com/yukawa/mozc/actions/runs/12878937354

This is now ready for review again.

@hiroyuki-komatsu hiroyuki-komatsu merged commit b878663 into google:master Jan 21, 2025
1 check passed
@hiroyuki-komatsu
Copy link
Collaborator

We have merged your PR.
Thank you for the contribution!

@yukawa yukawa deleted the cc_toolchain_from_rules_cc branch January 21, 2025 06:41
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.

2 participants