Skip to content

fix(crate_universe): fix OUTPUT_BASE env var value format in crates_vendor - #4102

Merged
UebelAndre merged 2 commits into
bazelbuild:mainfrom
georgesfarah:fix/output-base-env-value-format
Jul 30, 2026
Merged

fix(crate_universe): fix OUTPUT_BASE env var value format in crates_vendor#4102
UebelAndre merged 2 commits into
bazelbuild:mainfrom
georgesfarah:fix/output-base-env-value-format

Conversation

@georgesfarah

@georgesfarah georgesfarah commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

Summary

Fix the OUTPUT_BASE environment variable override in crate_universe/src/cli/vendor.rs — it has never worked correctly.

Bug

The bazel info output parser splits each line on : and stores only the part after the colon as the HashMap value:

let (k, v) = line.split_at(line.find(':')?);
Ok((k.to_string(), (v[1..]).trim().to_string()))

So for output like output_base: /data/output, the HashMap contains {"output_base" => "/data/output"}.

However, the OUTPUT_BASE env var override stores the full formatted string as the value:

bazel_info.insert("output_base".to_owned(), format!("output_base: {}", path));

This produces {"output_base" => "output_base: /data/output"}. When TryFrom later converts this to a PathBuf via .map(Into::into), it produces PathBuf("output_base: /data/output") — an invalid path.

Fix

Store the raw path string without the prefix:

bazel_info.insert("output_base".to_owned(), path);

Also extract parse_bazel_info from try_new so the parsing and env var override logic can be unit-tested without spawning a bazel info subprocess.

Test plan

Added test_parse_bazel_info_output_base_env_override which verifies:

  • Without OUTPUT_BASE set, the parsed value from bazel info output is used
  • With OUTPUT_BASE set, it overrides the parsed value with the correct path

Note

The test uses env::set_var/env::remove_var, which are unsafe in Rust 1.83+ (and required to be in unsafe blocks starting with edition 2024). This matches the existing test style in this file (see test_bazel_info). A follow-up could migrate both tests to use unsafe {} blocks or #[serial_test::serial] if the project upgrades its Rust edition.

@google-cla

google-cla Bot commented Jun 25, 2026

Copy link
Copy Markdown

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@georgesfarah
georgesfarah force-pushed the fix/output-base-env-value-format branch 3 times, most recently from 52e7ce9 to f6bf0fc Compare June 25, 2026 09:19
…endor

The OUTPUT_BASE env var override inserted
`format!("output_base: {}", path)` as the HashMap value. However, the
`bazel info` output parser splits each line on `:` and stores only the
part after the colon as the value:

    let (k, v) = line.split_at(line.find(':')?);
    Ok((k.to_string(), (v[1..]).trim().to_string()))

So for `bazel info` output like "output_base: /data/output", the HashMap
contains {"output_base" => "/data/output"}.

But the OUTPUT_BASE override stored the FULL formatted string as the
value: {"output_base" => "output_base: /data/output"}. When TryFrom
later converts this to a PathBuf via `.map(Into::into)`, it produces
PathBuf("output_base: /data/output") — an invalid path.

This means the OUTPUT_BASE env var has never worked correctly. Fix by
storing the raw path string without the prefix.

Also extract `parse_bazel_info` from `try_new` so the parsing and env
var override logic can be unit-tested without spawning a subprocess.
@georgesfarah
georgesfarah force-pushed the fix/output-base-env-value-format branch from f6bf0fc to 60036d4 Compare June 25, 2026 09:23
abrisco

This comment was marked as outdated.

@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.

Thank you!

@UebelAndre
UebelAndre enabled auto-merge July 30, 2026 14:50
@UebelAndre
UebelAndre added this pull request to the merge queue Jul 30, 2026
Merged via the queue into bazelbuild:main with commit b88f87a Jul 30, 2026
3 checks passed
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