Skip to content

Commit

Permalink
fix: check for git presence before calling
Browse files Browse the repository at this point in the history
This follows up #647 by also ensuring we don't call `git` if it's
not present on the system. If it's missing, we skip trying to
generate a source tarball altogether.
  • Loading branch information
mistydemeo committed Dec 13, 2023
1 parent 45d7d17 commit af785a8
Show file tree
Hide file tree
Showing 4 changed files with 38 additions and 4 deletions.
8 changes: 8 additions & 0 deletions cargo-dist/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -266,6 +266,14 @@ pub enum DistError {
#[error("We failed to generate a source tarball for your project")]
#[diagnostic(help("This is probably not your fault, please file an issue!"))]
GitArchiveError {},

/// A required tool is missing
#[error("{tool}, required to run this task, is missing")]
#[diagnostic(help("Ensure {tool} is installed"))]
ToolMissing {
/// the name of the missing tool
tool: String,
},
}

impl From<minijinja::Error> for DistError {
Expand Down
21 changes: 18 additions & 3 deletions cargo-dist/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,9 @@ fn run_build_step(
committish,
prefix,
target,
}) => Ok(generate_source_tarball(committish, prefix, target)?),
}) => Ok(generate_source_tarball(
dist_graph, committish, prefix, target,
)?),
BuildStep::Extra(target) => run_extra_artifacts_build(dist_graph, target),
}
}
Expand Down Expand Up @@ -185,8 +187,21 @@ fn generate_checksum(checksum: &ChecksumStyle, src_path: &Utf8Path) -> DistResul
/// Creates a source code tarball from the git archive from
/// tag/ref/commit `committish`, with the directory prefix `prefix`,
/// at the output file `target`.
fn generate_source_tarball(committish: &str, prefix: &str, target: &Utf8Path) -> DistResult<()> {
let status = Command::new("git")
fn generate_source_tarball(
graph: &DistGraph,
committish: &str,
prefix: &str,
target: &Utf8Path,
) -> DistResult<()> {
let git = if let Some(tool) = &graph.tools.git {
tool.cmd.to_owned()
} else {
return Err(DistError::ToolMissing {
tool: "git".to_owned(),
});
};

let status = Command::new(git)
.arg("archive")
.arg(committish)
.arg("--format=tar.gz")
Expand Down
12 changes: 11 additions & 1 deletion cargo-dist/src/tasks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -235,6 +235,8 @@ pub struct Tools {
pub rustup: Option<Tool>,
/// homebrew, only available on macOS
pub brew: Option<Tool>,
/// git, used if the repository is a git repo
pub git: Option<Tool>,
}

/// Info about the cargo toolchain we're using
Expand Down Expand Up @@ -1202,13 +1204,20 @@ impl<'pkg_graph> DistGraphBuilder<'pkg_graph> {
return;
}

let git = if let Some(tool) = &self.inner.tools.git {
tool.cmd.to_owned()
} else {
warn!("skipping source tarball; git not installed");
return;
};

// It's possible to run cargo-dist in something that's not a git
// repo, including a brand-new repo that hasn't been `git init`ted yet;
// we can't act on those.
//
// Note we don't need the output of --show-toplevel,
// just the exit status.
let status = Command::new("git")
let status = Command::new(git)
.arg("rev-parse")
.arg("--show-toplevel")
.stdout(std::process::Stdio::piped())
Expand Down Expand Up @@ -2679,6 +2688,7 @@ fn tool_info() -> Result<Tools> {
cargo,
rustup: find_tool("rustup", "-V"),
brew: find_tool("brew", "--version"),
git: find_tool("git", "--version"),
})
}

Expand Down
1 change: 1 addition & 0 deletions cargo-dist/src/tests/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ pub fn mock_tools() -> Tools {
},
rustup: None,
brew: None,
git: None,
}
}

Expand Down

0 comments on commit af785a8

Please sign in to comment.