Skip to content

Replace current_dir().unwrap() in init() with env::var_os("CARGO_MANIFEST_DIR") - #34

Merged
leighmcculloch merged 3 commits into
mainfrom
comments-expect
May 27, 2026
Merged

Replace current_dir().unwrap() in init() with env::var_os("CARGO_MANIFEST_DIR")#34
leighmcculloch merged 3 commits into
mainfrom
comments-expect

Conversation

@leighmcculloch

@leighmcculloch leighmcculloch commented May 26, 2026

Copy link
Copy Markdown
Member

What

Replace current_dir().unwrap() in init() with env::var_os("CARGO_MANIFEST_DIR"), and document why this is the correct source of the crate path in a build script.

Why

current_dir() happens to equal the manifest directory in the common case, but package.build in Cargo.toml can point to a build script located outside the package source tree. CARGO_MANIFEST_DIR is the env var Cargo explicitly sets to the package's manifest directory when executing the build script, so it's the correct source for the crate path regardless of where the build script lives.

Using expect instead of unwrap also surfaces a clear message at the panic site. Under normal operations there is no reason for CARGO_MANIFEST_DIR to be unset — Cargo always sets it for build scripts. If it is unset, the build system is already broken in other, larger, disastrous ways, and graceful handling here would not meaningfully recover the build.

Some discussion:

@leighmcculloch
leighmcculloch marked this pull request as ready for review May 26, 2026 04:45
Copilot AI review requested due to automatic review settings May 26, 2026 04:45

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR improves the clarity of failures in crate_git_revision::init() by replacing a bare unwrap() on std::env::current_dir() with an expect(...) message, and adds documentation explaining why relying on the current working directory is correct when invoked from a Cargo build script.

Changes:

  • Replace current_dir().unwrap() with current_dir().expect("getting current directory") in init().
  • Add an in-function comment (with a Cargo reference link) describing Cargo’s working-directory guarantee for build scripts.
  • Update the public rustdoc for init() to describe the working-directory requirement (but it currently describes it imprecisely).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/lib.rs Outdated
@leighmcculloch leighmcculloch changed the title Use expect with message in init() Replace current_dir().unwrap() in init() with env::var_os("CARGO_MANIFEST_DIR") May 26, 2026
@leighmcculloch
leighmcculloch merged commit 96f8378 into main May 27, 2026
11 checks passed
@leighmcculloch
leighmcculloch deleted the comments-expect branch May 27, 2026 03:49
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.

4 participants