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

Improve error message when a library is not found #158

Merged
merged 4 commits into from
Dec 20, 2023
Merged
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
162 changes: 154 additions & 8 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,12 +67,23 @@ use std::env;
use std::error;
use std::ffi::{OsStr, OsString};
use std::fmt;
use std::fmt::Display;
use std::io;
use std::ops::{Bound, RangeBounds};
use std::path::PathBuf;
use std::process::{Command, Output};
use std::str;

/// Wrapper struct to polyfill methods introduced in 1.57 (`get_envs`, `get_args` etc).
/// This is needed to reconstruct the pkg-config command for output in a copy-
/// paste friendly format via `Display`.
struct WrappedCommand {
inner: Command,
program: OsString,
env_vars: Vec<(OsString, OsString)>,
args: Vec<OsString>,
}

#[derive(Clone, Debug)]
pub struct Config {
statik: Option<bool>,
Expand Down Expand Up @@ -148,6 +159,81 @@ pub enum Error {
__Nonexhaustive,
}

impl WrappedCommand {
fn new<S: AsRef<OsStr>>(program: S) -> Self {
Self {
inner: Command::new(program.as_ref()),
program: program.as_ref().to_os_string(),
env_vars: Vec::new(),
args: Vec::new(),
}
}

fn args<I, S>(&mut self, args: I) -> &mut Self
where
I: IntoIterator<Item = S> + Clone,
S: AsRef<OsStr>,
{
self.inner.args(args.clone());
self.args
.extend(args.into_iter().map(|arg| arg.as_ref().to_os_string()));

self
}

fn arg<S: AsRef<OsStr>>(&mut self, arg: S) -> &mut Self {
self.inner.arg(arg.as_ref());
self.args.push(arg.as_ref().to_os_string());

self
}

fn env<K, V>(&mut self, key: K, value: V) -> &mut Self
where
K: AsRef<OsStr>,
V: AsRef<OsStr>,
{
self.inner.env(key.as_ref(), value.as_ref());
self.env_vars
.push((key.as_ref().to_os_string(), value.as_ref().to_os_string()));

self
}

fn output(&mut self) -> io::Result<Output> {
self.inner.output()
}
}

/// Output a command invocation that can be copy-pasted into the terminal.
/// `Command`'s existing debug implementation is not used for that reason,
/// as it can sometimes lead to output such as:
/// `PKG_CONFIG_ALLOW_SYSTEM_CFLAGS="1" PKG_CONFIG_ALLOW_SYSTEM_LIBS="1" "pkg-config" "--libs" "--cflags" "mylibrary"`
/// Which cannot be copy-pasted into terminals such as nushell, and is a bit noisy.
/// This will look something like:
/// `PKG_CONFIG_ALLOW_SYSTEM_CFLAGS=1 PKG_CONFIG_ALLOW_SYSTEM_LIBS=1 pkg-config --libs --cflags mylibrary`
impl Display for WrappedCommand {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
// Format all explicitly defined environment variables
let envs = self
.env_vars
.iter()
.map(|(env, arg)| format!("{}={}", env.to_string_lossy(), arg.to_string_lossy()))
.collect::<Vec<String>>()
.join(" ");

// Format all pkg-config arguments
let args = self
.args
.iter()
.map(|arg| arg.to_string_lossy().to_string())
.collect::<Vec<String>>()
.join(" ");

write!(f, "{} {} {}", envs, self.program.to_string_lossy(), args)
}
}

impl error::Error for Error {}

impl fmt::Debug for Error {
Expand Down Expand Up @@ -208,12 +294,72 @@ impl fmt::Display for Error {
ref command,
ref output,
} => {
write!(
let crate_name =
env::var("CARGO_PKG_NAME").unwrap_or(String::from("<NO CRATE NAME>"));

writeln!(f, "")?;

// Give a short explanation of what the error is
writeln!(
f,
"`{}` did not exit successfully: {}\nerror: could not find system library '{}' required by the '{}' crate\n",
command, output.status, name, env::var("CARGO_PKG_NAME").unwrap_or_default(),
"pkg-config {}",
match output.status.code() {
Some(code) => format!("exited with status code {}", code),
None => "was terminated by signal".to_string(),
}
)?;
format_output(output, f)

// Give the command run so users can reproduce the error
writeln!(f, "> {}\n", command)?;

// Explain how it was caused
writeln!(
f,
"The system library `{}` required by crate `{}` was not found.",
name, crate_name
)?;
writeln!(
f,
"The file `{}.pc` needs to be installed and the PKG_CONFIG_PATH environment variable must contain its parent directory.",
name
)?;

// There will be no status code if terminated by signal
if let Some(_code) = output.status.code() {
// NixOS uses a wrapper script for pkg-config that sets the custom
// environment variable PKG_CONFIG_PATH_FOR_TARGET
let search_path =
if let Ok(path_for_target) = env::var("PKG_CONFIG_PATH_FOR_TARGET") {
Some(path_for_target)
} else if let Ok(path) = env::var("PKG_CONFIG_PATH") {
Some(path)
} else {
None
};

// Guess the most reasonable course of action
let hint = if let Some(search_path) = search_path {
writeln!(
f,
"PKG_CONFIG_PATH contains the following:\n{}",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Isn't this confusing for NixOS users? Maybe remember from which variable the search path comes and print that here.

Looks good to me otherwise, thanks a lot :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, good catch! Will write a quick fix.

search_path
.split(':')
.map(|path| format!(" - {}", path))
.collect::<Vec<String>>()
.join("\n"),
)?;

format!("you may need to install a package such as {name}, {name}-dev or {name}-devel.", name=name)
} else {
writeln!(f, "PKG_CONFIG_PATH environment variable is not set")?;
format!("If you have installed the library, try adding its parent directory to your PATH.")
};

// Try and nudge the user in the right direction so they don't get stuck
writeln!(f, "\nHINT: {}", hint)?;
}

Ok(())
}
Error::Failure {
ref command,
Expand Down Expand Up @@ -499,20 +645,20 @@ impl Config {
Ok(output.stdout)
} else {
Err(Error::Failure {
command: format!("{:?}", cmd),
command: format!("{}", cmd),
output,
})
}
}
Err(cause) => Err(Error::Command {
command: format!("{:?}", cmd),
command: format!("{}", cmd),
cause,
}),
}
}

fn command(&self, exe: OsString, name: &str, args: &[&str]) -> Command {
let mut cmd = Command::new(exe);
fn command(&self, exe: OsString, name: &str, args: &[&str]) -> WrappedCommand {
let mut cmd = WrappedCommand::new(exe);
if self.is_static(name) {
cmd.arg("--static");
}
Expand Down