Skip to content

Support escaped runfile paths - #3910

Merged
illicitonion merged 2 commits into
bazelbuild:mainfrom
meroton:escaped-manifests
Mar 23, 2026
Merged

Support escaped runfile paths#3910
illicitonion merged 2 commits into
bazelbuild:mainfrom
meroton:escaped-manifests

Conversation

@moroten

@moroten moroten commented Mar 20, 2026

Copy link
Copy Markdown
Contributor

Add support for spaces, newlines and backslashes in paths written to the runfiles MANIFEST file.

@moroten

moroten commented Mar 23, 2026

Copy link
Copy Markdown
Contributor Author

Rebased: 54b3d78a9 -> 14e2faeb9

@moroten

moroten commented Mar 23, 2026

Copy link
Copy Markdown
Contributor Author

CI fails for //rust/runfiles:runfiles_test on Bazel 7.4.1 but passes on my local machine. Is there a way to look at the test output?

@UebelAndre

Copy link
Copy Markdown
Collaborator

Seems like you unfortunately hit bazelbuild/continuous-integration#1838

If you run bazel coverage //rust/runfiles:runfiles_test can you see the test failures?

@moroten

moroten commented Mar 23, 2026

Copy link
Copy Markdown
Contributor Author

Coverage succeeds locally.

@UebelAndre

Copy link
Copy Markdown
Collaborator

I opened bazelbuild/continuous-integration#2508 but in the mean time you could update the presubmit file for this job to add --test_output=errors to coverage_flags to force information to start appearing.

Add support for spaces, newlines and backslashes in paths written to the
runfiles MANIFEST file.

@UebelAndre UebelAndre left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks! Just one request!

Comment thread rust/runfiles/runfiles.rs Outdated
.ok_or(RunfilesError::RunfilesManifestInvalidFormat)?;
Ok::<(PathBuf, PathBuf), RunfilesError>((pair.0.into(), pair.1.into()))
let pair = if line.starts_with(' ') {
// Unescape according to src/main/java/com/google/devtools/build/lib/analysis/SourceManifestAction.java.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Can you leave a link with a commit hash in it so the context isn't invalidated by upstream changes when folks go to look this up?

Suggested change
// Unescape according to src/main/java/com/google/devtools/build/lib/analysis/SourceManifestAction.java.
// Unescape according to SourceManifestAction.java
// https://github.com/bazelbuild/bazel/blob/5b3e143adf7ccf40b05f73b0711f60f1ceade180/src/main/java/com/google/devtools/build/lib/analysis/SourceManifestAction.java

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I choose a link with line numbers as well. Thank you for the review!

@UebelAndre

Copy link
Copy Markdown
Collaborator

Looks like CI failed with a network flake 😞

If you push a new commit (e.g. git commit --amend --date=now) then you'll probably get lucky enough for a passing build

@illicitonion
illicitonion enabled auto-merge March 23, 2026 19:54
@illicitonion
illicitonion added this pull request to the merge queue Mar 23, 2026
Merged via the queue into bazelbuild:main with commit 487ea0e Mar 23, 2026
3 checks passed
@moroten
moroten deleted the escaped-manifests branch March 23, 2026 21:08
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.

3 participants