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

Make Bazel itself build under an output base with Unicode characters #24457

Draft
wants to merge 12 commits into
base: master
Choose a base branch
from
Draft

Make Bazel itself build under an output base with Unicode characters #24457

wants to merge 12 commits into from

Conversation

fmeum
Copy link
Collaborator

@fmeum fmeum commented Nov 22, 2024 •
edited
Loading

  • All decompression functions used in repository rules are modified to go through Path#getInputStream(), thus avoiding string encoding issues when constructing a FileInputStream manually.
  • A new Path#createTempDirectory function is introduced to replace error-prone usages of Files.createTempDirectory throughout the code base, which requires reencoding of both the arguments and the return value.
  • Fix encoding of argv0 in SubprocessBuilder and WorkerMultiplexer.
  • Fix encoding of the path of and the content in the server log file.
  • Fix encoding of info output.

Work towards #374
Fixes #24444

@fmeum fmeum changed the title Fix even more Unicode bugs Make Bazel itself buildable under an output base with Unicode characters Nov 22, 2024
@fmeum fmeum changed the title Make Bazel itself buildable under an output base with Unicode characters Make Bazel itself build under an output base with Unicode characters Nov 22, 2024
@fmeum fmeum force-pushed the 24444-log-path branch 2 times, most recently from 3d80129 to abc6d14 Compare November 22, 2024 18:29
@fmeum
Copy link
Collaborator Author

fmeum commented Nov 22, 2024

I suspect that the RBE failure is due to the remote execution environment not having the C.UTF-8 locale installed which is forced for Java compilation actions. Would it be possible to add it?

@tjgq
Copy link
Contributor

tjgq commented Nov 22, 2024

I suspect that the RBE failure is due to the remote execution environment not having the C.UTF-8 locale installed which is forced for Java compilation actions. Would it be possible to add it?

I think that entails creating a new docker image for RBE (or finding a suitable existing one). @coeuvre, do you know how to do this?

If this is blocking for 8.0.0, I suggest finding a way to either detect the existence of the locale in the test, or disable it when on RBE.

@fmeum
Copy link
Collaborator Author

fmeum commented Nov 22, 2024

The test is failing on other platforms, so I'll look into that. It's not a hard blocker for 8.0.0, it can equally well go into 8.1.0.

@fmeum fmeum mentioned this pull request Nov 25, 2024
Closed
@fmeum
Copy link
Collaborator Author

fmeum commented Nov 25, 2024

This needs a fix in rules_java to properly set a UTF-8 environment for bootstrap actions: bazelbuild/rules_java#243

copybara-service bot pushed a commit to bazelbuild/rules_java that referenced this pull request Nov 27, 2024
@fmeum
Build bootclasspath in a UTF-8 environment
c22454f 
Copybara Import from #243

BEGIN_PUBLIC
Build bootclasspath in a UTF-8 environment (#243)

`java` and `javac` convert file and classpaths to absolute paths and thus require a UTF-8 locale to work under a path that contains non-ASCII characters.

Unblocks bazelbuild/bazel#24457

Closes #243
END_PUBLIC

COPYBARA_INTEGRATE_REVIEW=#243 from fmeum:utf8-environment 05813e4
PiperOrigin-RevId: 700695134
Change-Id: I2f5753720ec3c838a4dd8b6aabf1050c6935ef3d
@fmeum fmeum mentioned this pull request Dec 3, 2024
Closed
@fmeum fmeum mentioned this pull request Dec 5, 2024
Closed
copybara-service bot pushed a commit that referenced this pull request Dec 6, 2024
@fmeum @copybara-github
Let Turbine handle Unicode paths
a37d288 
This requires patching rules_graalvm with sgammon/rules_graalvm#514 to ensure the native image runs with `sun.jnu.encoding` set to `UTF-8` on Linux.

Also enable unrelated tests after the recent java_tools update.

Work towards #24444
Unblocks #24457

Closes #24565.

PiperOrigin-RevId: 703409662
Change-Id: I44de4387de4a6da404436f5a35a2b27274122bbb
@fmeum fmeum force-pushed the 24444-log-path branch 3 times, most recently from 6a0811c to 4c78604 Compare December 13, 2024 07:40
@fmeum fmeum force-pushed the 24444-log-path branch 2 times, most recently from 9ad3d9a to ff647db Compare December 28, 2024 10:33
@fmeum fmeum force-pushed the 24444-log-path branch 2 times, most recently from 0ae309a to c51c9ea Compare January 17, 2025 22:00
@fmeum
Copy link
Collaborator Author

fmeum commented Jan 18, 2025 •
edited
Loading

@hvadehra Can you say whether rules_java 8.7.1 ships java_tools that include a37d288? Is there a way to tell which Bazel commit a given rules_java release was based on for java_tools?

@hvadehra
Copy link
Member

The README.md in the java_tools zip archives has provenance info. Looks like v13.14 was built at 7d2509e

@fmeum
Copy link
Collaborator Author

fmeum commented Jan 20, 2025

The README.md in the java_tools zip archives has provenance info. Looks like v13.14 was built at 7d2509e

That's convenient. I can now tell that it doesn't include the required a37d288.

@hvadehra I filed bazelbuild/rules_java#265 as a replacement for bazelbuild/rules_java#250.

@fmeum fmeum mentioned this pull request Jan 29, 2025
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@fmeum @tjgq @hvadehra