Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: --print-all-dependencies should handle unknown-deriver #70

Merged
merged 12 commits into from
Jul 10, 2024
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ crate-type = ["cdylib", "rlib"]

[dependencies]
anyhow = "1.0"
thiserror = "1.0"
serde = { version = "1.0", features = ["derive"] }
serde_json = "1.0"
tokio = { version = "1", features = ["full"] }
Expand Down
97 changes: 79 additions & 18 deletions src/nix/nix_store.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,12 @@
use std::{fmt, path::PathBuf};
use std::{
fmt::{self, Display},
path::PathBuf,
};

use anyhow::{bail, Result};
use anyhow::Result;
use serde::{Deserialize, Serialize};
use thiserror::Error;
use tokio::process::Command;

/// Nix derivation output path
#[derive(Debug, Ord, PartialOrd, Eq, PartialEq, Clone, Hash)]
pub struct DrvOut(pub PathBuf);
Expand Down Expand Up @@ -66,7 +69,10 @@ impl NixStoreCmd {
/// This is done by querying the deriver of each output path from [devour_flake::DrvOut] using [nix_store_query_deriver] and
/// then querying all dependencies of each deriver using [nix_store_query_requisites_with_outputs].
/// Finally, all dependencies of each deriver are collected and returned as [Vec<StorePath>].
pub async fn fetch_all_deps(&self, out_paths: Vec<DrvOut>) -> Result<Vec<StorePath>> {
pub async fn fetch_all_deps(
&self,
out_paths: Vec<DrvOut>,
) -> Result<Vec<StorePath>, NixCmdError> {
let mut all_drvs = Vec::new();
for out in out_paths.iter() {
let DrvOut(out_path) = out;
Expand All @@ -84,20 +90,24 @@ impl NixStoreCmd {
}

/// Return the derivation used to build the given build output.
async fn nix_store_query_deriver(&self, out_path: PathBuf) -> Result<DrvOut> {
async fn nix_store_query_deriver(&self, out_path: PathBuf) -> Result<DrvOut, NixCmdError> {
let mut cmd = self.command();
cmd.args(["--query", "--deriver", &out_path.to_string_lossy().as_ref()]);
nix_rs::command::trace_cmd(&cmd);
let out = cmd.output().await?;
let out = cmd
.output()
.await
.map_err(CommandError::ChildProcessError)?;
srid marked this conversation as resolved.
Show resolved Hide resolved
if out.status.success() {
let drv_path = String::from_utf8(out.stdout)?.trim().to_string();
if drv_path == "unknown-deriver" {
return Err(NixCmdError::UnknownDeriverError);
}
Ok(DrvOut(PathBuf::from(drv_path)))
} else {
let exit_code = out.status.code().unwrap_or(1);
bail!(
"nix-store --query --deriver failed to run (exited: {})",
exit_code
);
let stderr = String::from_utf8(out.stderr).ok();
let exit_code = out.status.code();
Err(CommandError::ProcessFailed { stderr, exit_code }.into())
Copy link
Owner

Choose a reason for hiding this comment

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

There's a bug there. If UTF8 decoding fails, this sets stderr to None thus creating a false impression that no stderr is missing. Just use lossy decoding.

}
}

Expand All @@ -106,7 +116,7 @@ impl NixStoreCmd {
async fn nix_store_query_requisites_with_outputs(
&self,
drv_path: DrvOut,
) -> Result<Vec<StorePath>> {
) -> Result<Vec<StorePath>, NixCmdError> {
let mut cmd = self.command();
cmd.args([
"--query",
Expand All @@ -115,7 +125,10 @@ impl NixStoreCmd {
&drv_path.0.to_string_lossy().as_ref(),
]);
nix_rs::command::trace_cmd(&cmd);
let out = cmd.output().await?;
let out = cmd
.output()
.await
.map_err(CommandError::ChildProcessError)?;
if out.status.success() {
let out = String::from_utf8(out.stdout)?;
let out = out
Expand All @@ -127,11 +140,59 @@ impl NixStoreCmd {
.collect();
Ok(out)
} else {
let exit_code = out.status.code().unwrap_or(1);
bail!(
"nix-store --query --requisites --include-outputs failed to run (exited: {})",
exit_code
);
let stderr = String::from_utf8(out.stderr).ok();
let exit_code = out.status.code();
Err(CommandError::ProcessFailed { stderr, exit_code }.into())
Copy link
Owner

Choose a reason for hiding this comment

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

Same

}
}
}

/// Errors when running and interpreting the output of a nix command
#[derive(Error, Debug)]
pub enum NixCmdError {
Copy link
Owner

Choose a reason for hiding this comment

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

You shouldn't use duplicate names for types in order to avoid confusion. NixCmdError already exists: https://github.com/juspay/nix-rs/blob/2dbba678a9e67925d964dbde24fbd8951ecbd2de/src/command.rs#L187

Suggested change
pub enum NixCmdError {
pub enum NixStoreCmdError {

#[error("Command error: {0}")]
CmdError(#[from] CommandError),

#[error("Failed to decode command stdout (utf8 error): {0}")]
DecodeErrorUtf8(#[from] std::string::FromUtf8Error),

#[error("Failed to decode command stdout (from_str error): {0}")]
DecodeErrorFromStr(#[from] FromStrError),

#[error("Failed to decode command stdout (json error): {0}")]
DecodeErrorJson(#[from] serde_json::Error),
Copy link
Owner

@srid srid Jul 10, 2024

Choose a reason for hiding this comment

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

Why copy the fields rather than re-use NixCmdError directly here?

Copy link
Owner

Choose a reason for hiding this comment

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


#[error("Unknown deriver in drv_path")]
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
#[error("Unknown deriver in drv_path")]
#[error("Unknown deriver")]

UnknownDeriverError,
}

#[derive(Debug)]
pub struct FromStrError(String);

impl Display for FromStrError {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
write!(f, "Failed to parse string: {}", self.0)
}
}

impl std::error::Error for FromStrError {}
Copy link
Owner

Choose a reason for hiding this comment

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

What's this for?


#[derive(Error, Debug)]
pub enum CommandError {
#[error("Child process error: {0}")]
ChildProcessError(#[from] std::io::Error),
#[error(
"Process exited unsuccessfully. exit_code={:?}{}",
exit_code,
match stderr {
Some(s) => format!(" stderr={}", s),
None => "".to_string()
},
)]
ProcessFailed {
stderr: Option<String>,
exit_code: Option<i32>,
},
#[error("Failed to decode command stderr: {0}")]
Decode(#[from] std::string::FromUtf8Error),
}
Copy link
Owner

Choose a reason for hiding this comment

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