Skip to content

Add support for experimental_output_paths#4011

Merged
UebelAndre merged 2 commits into
bazelbuild:mainfrom
UebelAndre:path_mapping
May 7, 2026
Merged

Add support for experimental_output_paths#4011
UebelAndre merged 2 commits into
bazelbuild:mainfrom
UebelAndre:path_mapping

Conversation

@UebelAndre
Copy link
Copy Markdown
Collaborator

@UebelAndre UebelAndre commented May 2, 2026

For details on path mapping see bazelbuild/bazel#22658

Changes:

  • The process wrapper now explicitly handles out_dir to avoid location expanded environment variables.
  • construct_arguments is updated to support rustc_flags as an Args object. List is still supported though

@UebelAndre UebelAndre force-pushed the path_mapping branch 8 times, most recently from a2c755c to 68242ac Compare May 4, 2026 15:38
@UebelAndre UebelAndre marked this pull request as ready for review May 6, 2026 13:06
Copy link
Copy Markdown
Collaborator

@slackito slackito left a comment

Choose a reason for hiding this comment

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

Looks good overall. Thank you for the thorough comments, they made the changes much easier to follow :)

Just a couple of comments.

Comment thread .bazelci/presubmit.yml
rust_analyzer_tests_linux:
name: Rust-Analyzer Linux Tests
ide_integration_tests_linux:
name: IDE VSCode Tests
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.

Are the Rust-Analyzer -> IDE VSCode config changes related to the rest of the PR?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I needed to trim down the number of jobs since we're hitting the 128 limit

Comment thread rust/private/rustc.bzl
rustc_flags.add('--cfg=feature="no_std"')

# Add target specific flags last, so they can override previous flags
authored_rustc_flags = getattr(attr, "rustc_flags", [])
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.

What does "authored" mean in this context?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Flags users added to their targets. I can use whatever name you'd prefer. I know I'm bad at naming 😅

@UebelAndre UebelAndre requested a review from slackito May 7, 2026 15:56
@UebelAndre UebelAndre added this pull request to the merge queue May 7, 2026
Merged via the queue into bazelbuild:main with commit 175bf94 May 7, 2026
3 checks passed
@bdolgov
Copy link
Copy Markdown
Contributor

bdolgov commented May 15, 2026

I think this has broken a pattern somewhat common in embedded Rust where one crate puts a file into build script outputs, and another create references it from its build script in linker flags. I bisected the problem to 175bf94, but didn't yet understand whether the problem is in this commit or somewhere on my side.

@UebelAndre
Copy link
Copy Markdown
Collaborator Author

I think this has broken a pattern somewhat common in embedded Rust where one crate puts a file into build script outputs, and another create references it from its build script in linker flags. I bisected the problem to 175bf94, but didn't yet understand whether the problem is in this commit or somewhere on my side.

Interesting, I would be surprised this has an impact assuming you're not using --experimental_output_paths=strip. I assume you're not?

@bdolgov
Copy link
Copy Markdown
Contributor

bdolgov commented May 15, 2026

No, I don't.

@UebelAndre
Copy link
Copy Markdown
Collaborator Author

No, I don't.

Can you make a small repro in a pull-request somewhere in //cargo/tests/cargo_build_script ?

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