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

Always run Turbine native image with a UTF-8 code page on Windows #25444

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

Conversation

fmeum
Copy link
Collaborator

@fmeum fmeum commented Mar 4, 2025

We are building Turbine native images with a UTF-8 code page and need to match the runtime code page for UTF-8 paths to be handled properly. This change splits the existing tool for patching the app manifest of the embedded JDK into separate tools for reading and writing app manifests. The write tool is then used to add a manifest to the turbine binary.

@github-actions github-actions bot added the awaiting-review PR is awaiting review from an assigned reviewer label Mar 4, 2025
@fmeum fmeum requested a review from hvadehra March 4, 2025 11:25
@fmeum fmeum changed the title Enable Turbine Unicode test on Windows and the released toolchain Run Turbine Unicode test unconditionally Mar 4, 2025
@fmeum fmeum marked this pull request as draft March 4, 2025 12:04
@iancha1992 iancha1992 added the team-Rules-Java Issues for Java rules label Mar 5, 2025
@fmeum fmeum changed the title Run Turbine Unicode test unconditionally Always run Turbine native image with a UTF-8 code page on Windows Mar 6, 2025
@fmeum fmeum requested a review from meteorcloudy March 6, 2025 21:51
@fmeum fmeum marked this pull request as ready for review March 6, 2025 21:51
@fmeum fmeum requested a review from a team as a code owner March 6, 2025 21:51
@fmeum
Copy link
Collaborator Author

fmeum commented Mar 6, 2025

@bazel-io fork 8.2.0

Copy link
Member

@meteorcloudy meteorcloudy left a comment

Choose a reason for hiding this comment

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

I'm fine with the custom tool to add/update windows manifest file. Just in case we need more complex feature in future, we could switch to use mt.exe from the windows SDK, which can be fetched by updating https://github.com/bazelbuild/bazel/blob/master/src/main/res/winsdk_configure.bzl.

@meteorcloudy meteorcloudy added awaiting-PR-merge PR has been approved by a reviewer and is ready to be merge internally and removed awaiting-review PR is awaiting review from an assigned reviewer labels Mar 7, 2025
@meteorcloudy
Copy link
Member

@hvadehra Please take a final look

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting-PR-merge PR has been approved by a reviewer and is ready to be merge internally team-Rules-Java Issues for Java rules
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants