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

Parse Starlark files as raw bytes for Bzlmod #24217

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

fmeum
Copy link
Collaborator

@fmeum fmeum commented Nov 5, 2024

As long as Bazel internally represents strings as raw bytes "encoded" in Latin-1, the same must be true for all Starlark files that may contain file system paths.

Also includes changes to the Python test setup:

  • ScratchFile now always writes files as UTF-8
  • RunProgram encodes and decodes stdin/stderr/stdout as UTF-8
  • download no longer leaks a file

@github-actions github-actions bot added team-ExternalDeps External dependency handling, remote repositiories, WORKSPACE file. awaiting-review PR is awaiting review from an assigned reviewer labels Nov 5, 2024
@fmeum
Copy link
Collaborator Author

fmeum commented Nov 5, 2024

This needs #24010 to get its test to pass.

@fmeum
Copy link
Collaborator Author

fmeum commented Nov 5, 2024

@bazel-io fork 8.0.0

@fmeum fmeum marked this pull request as draft November 7, 2024 16:26
@fmeum fmeum marked this pull request as ready for review November 7, 2024 18:22
@fmeum
Copy link
Collaborator Author

fmeum commented Nov 7, 2024

Ready for review now.

@tjgq FYI, this is the other Unicode-related PR I am planning to get into 8.0.0. If you see anything else that looks incompatible, please let me know.

@fmeum
Copy link
Collaborator Author

fmeum commented Nov 7, 2024

Turns out that there is an interesting failure, switching back to draft.

@fmeum fmeum marked this pull request as draft November 7, 2024 19:01
@fmeum fmeum force-pushed the 23859-unicode-bzlmod branch 7 times, most recently from e8c1ed0 to ae182a1 Compare November 7, 2024 21:48
As long as Bazel internally represents strings as raw bytes "encoded" in Latin-1, the same must be true for all Starlark files that may contain e.g. file system paths.
@fmeum fmeum marked this pull request as ready for review November 7, 2024 22:47
@fmeum
Copy link
Collaborator Author

fmeum commented Nov 7, 2024

@Wyverald CI should be green now, I just had to fix some issues in the Python test setup code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting-review PR is awaiting review from an assigned reviewer team-ExternalDeps External dependency handling, remote repositiories, WORKSPACE file.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant