[bazel] Improve runfiles handling#10098
Conversation
zbarsky-openai
force-pushed
the
zbarsky/rust-up
branch
2 times, most recently
from
4c176aa to
cae3169
Compare
codex-rs/utils/cargo-bin/src/lib.rs
Outdated
| @@ -72,6 +58,27 @@ fn cargo_bin_env_keys(name: &str) -> Vec<String> { | |||
| keys | |||
| } | |||
|
|
|||
| fn resolve_bin_from_env(key: &str, value: OsString) -> Result<PathBuf, CargoBinError> { | |||
| let raw = PathBuf::from(&value); | |||
| if std::env::var_os("RUN_UNDER_RUNFILES").is_some() { | |||
There was a problem hiding this comment.
Let's make this a constant with a docstring?
There was a problem hiding this comment.
ok will do, but that can only be used in this method, not in the macro (inlined to callers)
zbarsky-openai
force-pushed
the
zbarsky/rust-up
branch
from
00438a1 to
634aa8f
Compare
codex-rs/utils/cargo-bin/src/lib.rs
Outdated
|
|
||
| if let Some(root) = | ||
| buck_project_root().map_err(|source| CargoBinError::CurrentExe { source })? | ||
| #[doc(hidden)] |
There was a problem hiding this comment.
#[doc(hidden)] is unnecessary for our purposes, I believe.
| regex-lite = "0.1.8" | ||
| reqwest = "0.12" | ||
| rmcp = { version = "0.12.0", default-features = false } | ||
| runfiles = { git = "https://github.com/dzbarsky/rules_rust", rev = "b56cbaa8465e74127f1ea216f813cd377295ad81" } |
There was a problem hiding this comment.
What is our long-term plan for this dep?
There was a problem hiding this comment.
there's a few potential options:
- We figure out who owns the
runfilescrate on crates.io and get it bumped - We upstream this fix back to rules_rust so it's consumable from the "official" github
- We make it part of
rules_rsand that becomes "official" as it continues to get traction
By the way, this is only necessary for cargo test and non-bazel clippy, so if there's a world where those flows are both bazelized, this can be removed (I will teach rules_rs to alias @crates//:runfiles to @rules_rust//rust/runfiles instead of the crate from crates.io)
.bazelrc
Outdated
| common --experimental_platform_in_output_dir | ||
|
|
||
| # Runfile paths are too long on Windows, so we can't enable them there. Disable directory strategy everywhere for consistency. | ||
| # Instead we use the runfiles manifest strategy. |
There was a problem hiding this comment.
Can you cite a path that is likely to be stable: maybe utils/cargo-bin/README.md (which would need to be created?)
zbarsky-openai
force-pushed
the
zbarsky/rust-up
branch
2 times, most recently
from
41affed to
89a88b5
Compare
zbarsky-openai
force-pushed
the
zbarsky/rust-up
branch
from
126ee2d to
7c04836
Compare
zbarsky-openai
force-pushed
the
zbarsky/rust-up
branch
5 times, most recently
from
d6b86ad to
6a091d6
Compare
zbarsky-openai
force-pushed
the
zbarsky/rust-up
branch
from
6a091d6 to
702c156
Compare
3 participants
we can't use runfiles directory on Windows due to path lengths, so swap to manifest strategy. Parsing the manifest is a bit complex and the format is changing in Bazel upstream, so pull in the official Rust library (via a small hack to make it importable...) and cleanup all the associated logic to work cleanly in both bazel and cargo without extra confusion