Skip to content

Commit 27aad1b

Browse files
authored
fix!: Fix ShellOptions::with_cargo_path() (#273)
## What? Changes `ShellOptions::with_cargo_path()` to support custom build / target directory paths and the upcoming changes in the default build directory structure. ## Why? The current approach doesn't work with custom build / target directory paths. fixes #269
1 parent b834b3f commit 27aad1b

6 files changed

Lines changed: 72 additions & 16 deletions

File tree

crates/term-transcript-cli/tests/e2e.rs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ fn test_config() -> (TestConfig<StdShell>, TempDir) {
3434
// Switch off logging if `RUST_LOG` is set in the surrounding env
3535
.with_env("RUST_LOG", "off")
3636
.with_current_dir(temp_dir.path())
37-
.with_cargo_path()
37+
.with_cargo_path_for("term-transcript")
3838
.with_additional_path(rainbow_dir)
3939
.with_io_timeout(Duration::from_secs(2));
4040
let config = TestConfig::new(shell_options).with_match_kind(MatchKind::Precise);
@@ -58,7 +58,8 @@ fn scrolled_template() -> Template {
5858
fn help_example() {
5959
use term_transcript::PtyCommand;
6060

61-
let shell_options = ShellOptions::new(PtyCommand::default()).with_cargo_path();
61+
let shell_options =
62+
ShellOptions::new(PtyCommand::default()).with_cargo_path_for("term-transcript");
6263
TestConfig::new(shell_options).test(svg_snapshot("help"), ["term-transcript --help"]);
6364
}
6465

crates/term-transcript/CHANGELOG.md

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,10 @@ The project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html)
99

1010
- Bump minimum supported Rust version to 1.86.
1111

12+
### Fixed
13+
14+
- Rework `ShellOptions::with_cargo_path()` to work with custom target directories.
15+
1216
## 0.5.0-beta.1 - 2026-02-04
1317

1418
### Added
@@ -104,7 +108,7 @@ The project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html)
104108
As an example, this can be used to import fonts using `@import` or `@font-face`.
105109
- Add a fallback error message to the default template if HTML-in-SVG embedding
106110
is not supported.
107-
- Add [FAQ](../FAQ.md) with some tips and troubleshooting advice.
111+
- Add a FAQ with some tips and troubleshooting advice.
108112
- Allow hiding `UserInput`s during transcript rendering by calling the `hide()` method.
109113
Hidden inputs are supported by the default and pure SVG templates.
110114

crates/term-transcript/src/shell/mod.rs

Lines changed: 60 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -263,25 +263,77 @@ impl<Cmd: ConfigureCommand> ShellOptions<Cmd> {
263263
path
264264
}
265265

266-
/// Adds paths to cargo binaries (including examples) to the `PATH` env variable
267-
/// for the shell described by these options.
268-
/// This allows to call them by the corresponding filename, without specifying a path
266+
#[cfg_attr(feature = "tracing", tracing::instrument(level = "debug", ret))]
267+
fn legacy_cargo_path(binary_name: &str) -> Option<PathBuf> {
268+
let target_path = Self::target_path();
269+
let binary_path = target_path.join(format!("{binary_name}{}", env::consts::EXE_SUFFIX));
270+
let exists = binary_path.try_exists();
271+
272+
#[cfg(feature = "tracing")]
273+
tracing::debug!(?binary_path, ?exists, "checked binary path");
274+
exists.ok()?.then_some(binary_path)
275+
}
276+
277+
fn panic_on_missing_cargo_path(binary_name: &str) -> ! {
278+
let binaries: Vec<_> = env::vars_os()
279+
.filter_map(|(name, _)| {
280+
let name = name.into_string().ok()?;
281+
Some(name.strip_prefix("CARGO_BIN_EXE_")?.to_owned())
282+
})
283+
.collect();
284+
if binaries.is_empty() {
285+
panic!(
286+
"`CARGO_BIN_EXE_{binary_name}` env variable is unset, and {binary_name} is not in the default cargo target dir.\n\
287+
help: If this is run in a unit test, move it to an integration test to gain access to `CARGO_BIN_EXE_` vars (requires Rust 1.94+)"
288+
);
289+
} else {
290+
panic!(
291+
"`{binary_name}` does not look like a valid cargo binary in the workspace.\n\
292+
help: Available binaries: {binaries:?}"
293+
);
294+
}
295+
}
296+
297+
/// Adds paths to a cargo binary to the `PATH` env variable for the shell described by these options.
298+
/// This allows to call the binary by the corresponding filename, without specifying a path
269299
/// or doing complex preparations (e.g., calling `cargo install`).
270300
///
271301
/// # Limitations
272302
///
273303
/// - The caller must be a unit or integration test; the method will work improperly otherwise.
304+
/// - Does not work in Rust 1.91, 1.92, 1.93 with a non-default `build.build-dir`.
305+
#[cfg_attr(feature = "tracing", tracing::instrument(level = "debug", skip(self)))]
274306
#[must_use]
275-
pub fn with_cargo_path(mut self) -> Self {
276-
let target_path = Self::target_path();
277-
self.path_additions.push(target_path.join("examples"));
278-
self.path_additions.push(target_path);
307+
#[allow(clippy::missing_panics_doc)] // should never be triggered
308+
pub fn with_cargo_path_for(mut self, binary_name: &str) -> Self {
309+
let env_var_name = format!("CARGO_BIN_EXE_{binary_name}");
310+
let binary_path = env::var_os(&env_var_name).map(PathBuf::from);
311+
312+
#[cfg(feature = "tracing")]
313+
tracing::debug!(?binary_path, "got Rust 1.94+ path to binary");
314+
315+
let binary_path = binary_path
316+
.or_else(|| Self::legacy_cargo_path(binary_name))
317+
.unwrap_or_else(|| Self::panic_on_missing_cargo_path(binary_name));
318+
319+
#[cfg(feature = "tracing")]
320+
tracing::debug!(?binary_path, "got path to binary");
321+
322+
let parent_path = binary_path
323+
.parent()
324+
.expect("invalid binary path")
325+
.to_owned();
326+
// The check is inefficient, but we shouldn't have many additional paths.
327+
if !self.path_additions.contains(&parent_path) {
328+
self.path_additions.push(parent_path);
329+
}
330+
279331
self
280332
}
281333

282334
/// Adds a specified path to the `PATH` env variable for the shell described by these options.
283335
/// This method can be called multiple times to add multiple paths and is composable
284-
/// with [`Self::with_cargo_path()`].
336+
/// with [`Self::with_cargo_path_for()`].
285337
#[must_use]
286338
pub fn with_additional_path(mut self, path: impl Into<PathBuf>) -> Self {
287339
let path = path.into();

crates/term-transcript/src/shell/standard.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,7 @@ impl ShellOptions<StdShell> {
9494
/// Creates an alias for the binary at `path_to_bin`, which should be an absolute path.
9595
/// This allows to call the binary using this alias without complex preparations (such as
9696
/// installing it globally via `cargo install`), and is more flexible than
97-
/// [`Self::with_cargo_path()`].
97+
/// [`Self::with_cargo_path_for()`].
9898
///
9999
/// In integration tests, you may use [`env!("CARGO_BIN_EXE_<name>")`] to get a path
100100
/// to binary targets.

crates/term-transcript/src/test/mod.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,8 @@
1313
//!
1414
//! // Test configuration that can be shared across tests.
1515
//! fn config() -> TestConfig {
16-
//! let shell_options = ShellOptions::default().with_cargo_path();
16+
//! let shell_options = ShellOptions::default()
17+
//! .with_cargo_path_for("my-command");
1718
//! TestConfig::new(shell_options)
1819
//! .with_match_kind(MatchKind::Precise)
1920
//! .with_output(TestOutputConfig::Verbose)

crates/term-transcript/tests/integration.rs

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -162,9 +162,7 @@ fn transcript_with_empty_output(mute_outputs: &[bool], pure_svg: bool) -> anyhow
162162
}
163163
});
164164

165-
let mut shell_options = ShellOptions::default()
166-
.with_cargo_path()
167-
.with_io_timeout(Duration::from_millis(200));
165+
let mut shell_options = ShellOptions::default().with_io_timeout(Duration::from_millis(200));
168166
let transcript = Transcript::from_inputs(&mut shell_options, inputs)?;
169167
assert_tracing_for_transcript_from_inputs(&tracing_storage.lock());
170168

0 commit comments

Comments
 (0)