fix: use CARGO_BIN_EXE_usage if set by kybe236 · Pull Request #568 · jdx/usage

kybe236 · 2026-03-29T20:00:58Z

Use CARGO_BIN_EXE_usage if set

This ensures the tests can locate the binary even if the build directory includes a target triple.
It requires the binary to be built beforehand, which is already the case in Nixpkgs (hence this PR/patch).
Not sure how to determine the correct path if the binary hasn't been built yet.

Mainly made this commit to make the tests pass in Nixpkgs:
NixOS/nixpkgs#504792

See:

codecov · 2026-03-29T20:01:58Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 79.04%. Comparing base (2eee02e) to head (e4768f5).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #568   +/-   ##
=======================================
  Coverage   79.04%   79.04%           
=======================================
  Files          48       48           
  Lines        7235     7235           
  Branches     7235     7235           
=======================================
  Hits         5719     5719           
  Misses       1141     1141           
  Partials      375      375

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

gemini-code-assist

Code Review

This pull request updates the build_usage_binary function in the shell completion integration tests to check for the CARGO_BIN_EXE_usage environment variable. A review comment suggests enhancing this check by filtering out empty strings to avoid potential panics when the path is used later.

greptile-apps · 2026-03-29T20:02:53Z

Greptile Summary

This PR adds a small but meaningful fix to build_usage_binary() in the shell-completions integration test: before falling back to running cargo build, it checks the CARGO_BIN_EXE_usage environment variable and, if set and non-empty, returns that path directly.

Why this matters:

Cargo already sets CARGO_BIN_EXE_<name> at runtime for integration tests, so in normal cargo test runs this also eliminates a redundant rebuild.
In Nixpkgs (and similar sandboxed build systems), the binary is pre-built and lives at a path that may include a target triple; CARGO_BIN_EXE_usage is the standard Cargo mechanism to expose that path to tests.

Key observations:

The filter(|s| !s.is_empty()) guard is a good defensive touch.
All four shell-specific test functions call usage_bin.parent().unwrap() to build a PATH prefix. This is fine for absolute paths (which both Cargo and Nixpkgs produce), but would silently add "" (i.e., the CWD) to PATH for a bare binary name — a low-risk but easy-to-tighten edge case.

Confidence Score: 5/5

Safe to merge — the change is minimal, well-scoped, and the only remaining feedback is a low-priority style suggestion.

No P0 or P1 issues. The logic is correct: runtime env-var lookup with an empty-string guard, followed by an early return. The single P2 note about parent() on a bare filename is an edge case that won't occur with either Cargo or Nixpkgs paths.

No files require special attention.

Important Files Changed

Filename	Overview
cli/tests/shell_completions_integration.rs	Added early return in `build_usage_binary()` to honour `CARGO_BIN_EXE_usage` env var when set, allowing Nixpkgs (and normal Cargo test runs) to skip the redundant `cargo build` step and correctly locate binaries built with a target-triple in the path.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[build_usage_binary called] --> B{CARGO_BIN_EXE_usage set\nand non-empty?}
    B -- Yes --> C[Return PathBuf from env var\ne.g. /nix/store/.../bin/usage]
    B -- No --> D[Locate CARGO_MANIFEST_DIR\nfind workspace root]
    D --> E[Run: cargo build --bin usage]
    E --> F{Build successful?}
    F -- No --> G[panic! with stderr]
    F -- Yes --> H[Return workspace_root/target/debug/usage]
    C --> I[Tests use binary path\nfor shell completion tests]
    H --> I

_{Reviews (2): Last reviewed commit: "[autofix.ci] apply automated fixes" | Re-trigger Greptile}

this is needed incase the build directory includes a target tripplet. see: - https://github.com/NixOS/nixpkgs/blob/75e64f8ebde8e90df2643e5b82677b93ba33d922/doc/languages-frameworks/rust.section.md#tests-relying-on-the-structure-of-the-target-directory-tests-relying-on-the-structure-of-the-target-directory - https://doc.rust-lang.org/cargo/reference/build-cache.html

@david-hamilton-glean

gemini-code-assist Bot reviewed Mar 29, 2026

View reviewed changes

Comment thread cli/tests/shell_completions_integration.rs Outdated

kybe236 changed the title ~~use CARGO_BIN_EXE_usage if set~~ fix: use CARGO_BIN_EXE_usage if set Mar 29, 2026

greptile-apps Bot reviewed Mar 29, 2026

View reviewed changes

Comment thread cli/tests/shell_completions_integration.rs Outdated

kybe236 force-pushed the main branch from ab13008 to 67e5397 Compare March 29, 2026 20:05

[autofix.ci] apply automated fixes

e4768f5

jdx merged commit 62936a2 into jdx:main Mar 30, 2026
8 checks passed

mise-en-dev mentioned this pull request Mar 30, 2026

chore: release v3.2.1 #564

Merged

BrewTestBot mentioned this pull request Apr 22, 2026

usage 3.2.1 Homebrew/homebrew-core#278772

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix: use CARGO_BIN_EXE_usage if set#568

fix: use CARGO_BIN_EXE_usage if set#568
jdx merged 2 commits intojdx:mainfrom
kybe236:main

kybe236 commented Mar 29, 2026

Uh oh!

codecov Bot commented Mar 29, 2026 •

edited

Loading

Uh oh!

gemini-code-assist Bot left a comment

Uh oh!

Uh oh!

greptile-apps Bot commented Mar 29, 2026 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

kybe236 commented Mar 29, 2026

Uh oh!

codecov Bot commented Mar 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

greptile-apps Bot commented Mar 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Greptile Summary

Confidence Score: 5/5

Important Files Changed

Flowchart

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

codecov Bot commented Mar 29, 2026 •

edited

Loading

greptile-apps Bot commented Mar 29, 2026 •

edited

Loading