Skip to content

ensure callers of BuildDirectory::run can determine if the source of the error is in Prepare::prepare #103

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

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all 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
2 changes: 1 addition & 1 deletion build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ fn main() -> Result<(), Box<dyn Error>> {
let target = std::env::var("TARGET")?;

let output = std::env::var("OUT_DIR")?;
::std::fs::write(format!("{}/target", output), target.as_bytes())?;
::std::fs::write(format!("{output}/target"), target.as_bytes())?;

println!("cargo:rerun-if-env-changed=DOCS_RS");
if std::env::var_os("DOCS_RS").as_deref() == Some(std::ffi::OsStr::new("1")) {
Expand Down
10 changes: 8 additions & 2 deletions src/build.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use crate::cmd::{Command, MountKind, Runnable, SandboxBuilder};
use crate::prepare::Prepare;
use crate::{Crate, Toolchain, Workspace};
use crate::{Crate, PrepareError, Toolchain, Workspace};
use std::path::PathBuf;
use std::vec::Vec;

Expand Down Expand Up @@ -194,7 +194,13 @@ impl BuildDirectory {
}

let mut prepare = Prepare::new(&self.workspace, toolchain, krate, &source_dir, patches);
prepare.prepare()?;
prepare.prepare().map_err(|err| {
if err.downcast_ref::<PrepareError>().is_none() {
err.context(PrepareError::Uncategorized)
} else {
err
}
})?;

std::fs::create_dir_all(self.target_dir())?;
let res = f(&Build {
Expand Down
6 changes: 3 additions & 3 deletions src/cmd/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -501,14 +501,14 @@ impl<'w, 'pl> Command<'w, 'pl> {
cmd.env(k, v);
}

let cmdstr = format!("{:?}", cmd);
let cmdstr = format!("{cmd:?}");

if let Some(ref cd) = self.cd {
cmd.current_dir(cd);
}

if self.log_command {
info!("running `{}`", cmdstr);
info!("running `{cmdstr}`");
}

let out = RUNTIME
Expand All @@ -521,7 +521,7 @@ impl<'w, 'pl> Command<'w, 'pl> {
self.log_output,
))
.map_err(|e| {
error!("error running command: {}", e);
error!("error running command: {e}");
e
})?;

Expand Down
10 changes: 5 additions & 5 deletions src/cmd/sandbox.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ impl SandboxImage {
/// error will be returned instead.
pub fn remote(name: &str) -> Result<Self, CommandError> {
let mut image = SandboxImage { name: name.into() };
info!("pulling image {} from Docker Hub", name);
info!("pulling image {name} from Docker Hub");
Command::new_workspaceless("docker")
.args(&["pull", name])
.run()
Expand Down Expand Up @@ -219,7 +219,7 @@ impl SandboxBuilder {
}

pub(super) fn user(mut self, user: u32, group: u32) -> Self {
self.user = Some(format!("{}:{}", user, group));
self.user = Some(format!("{user}:{group}"));
self
}

Expand All @@ -242,7 +242,7 @@ impl SandboxBuilder {

for (var, value) in &self.env {
args.push("-e".into());
args.push(format! {"{}={}", var, value})
args.push(format! {"{var}={value}"})
}

if let Some(workdir) = self.workdir {
Expand Down Expand Up @@ -308,10 +308,10 @@ impl SandboxBuilder {
scopeguard::defer! {{
if let Err(err) = container.delete() {
error!("failed to delete container {}", container.id);
error!("caused by: {}", err);
error!("caused by: {err}");
let mut err: &dyn Error = &err;
while let Some(cause) = err.source() {
error!("caused by: {}", cause);
error!("caused by: {cause}");
err = cause;
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/crates/registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ impl RegistryCrate {
git2::build::RepoBuilder::new()
.fetch_options(fo)
.clone(url, &index_path)
.with_context(|| format!("unable to update_index at {}", url))?;
.with_context(|| format!("unable to update_index at {url}"))?;
info!("cloned registry index");
}
let config = std::fs::read_to_string(index_path.join("config.json"))?;
Expand Down
4 changes: 2 additions & 2 deletions src/inside_docker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ pub(crate) fn probe_container_id(workspace: &Workspace) -> anyhow::Result<Option
.log_command(false)
.run_capture()?;
for id in out.stdout_lines() {
info!("probing container id {}", id);
info!("probing container id {id}");

let res = Command::new(workspace, "docker")
.args(&["exec", id, "cat", probe_path_str])
Expand All @@ -71,7 +71,7 @@ pub(crate) fn probe_container_id(workspace: &Workspace) -> anyhow::Result<Option
.run_capture();
if let Ok([probed]) = res.as_ref().map(|out| out.stdout_lines()) {
if *probed == probe_content {
info!("probe successful, this is container ID {}", id);
info!("probe successful, this is container ID {id}");
return Ok(Some(id.clone()));
}
}
Expand Down
10 changes: 7 additions & 3 deletions src/prepare.rs
Original file line number Diff line number Diff line change
Expand Up @@ -268,7 +268,7 @@ impl<'a> TomlTweaker<'a> {
// Eliminate parent workspaces
if let Some(&mut Value::Table(ref mut package)) = self.table.get_mut("package") {
if package.remove("workspace").is_some() {
info!("removed parent workspace from {}", krate);
info!("removed parent workspace from {krate}");
}
}
}
Expand All @@ -295,15 +295,15 @@ impl<'a> TomlTweaker<'a> {

// Strip the 'publish-lockfile' key from [package]
if has_publish_lockfile {
info!("disabled cargo feature 'publish-lockfile' from {}", krate);
info!("disabled cargo feature 'publish-lockfile' from {krate}");
if let Some(&mut Value::Table(ref mut package)) = self.table.get_mut("package") {
package.remove("publish-lockfile");
}
}

// Strip the 'default-run' key from [package]
if has_default_run {
info!("disabled cargo feature 'default-run' from {}", krate);
info!("disabled cargo feature 'default-run' from {krate}");
if let Some(&mut Value::Table(ref mut package)) = self.table.get_mut("package") {
package.remove("default-run");
}
Expand Down Expand Up @@ -390,6 +390,10 @@ pub enum PrepareError {
/// Some of the dependencies do not exist anymore.
#[error("the crate depends on missing dependencies: \n\n{0}")]
MissingDependencies(String),
/// Uncategorized error
#[doc(hidden)]
#[error("uncategorized prepare error")]
Uncategorized,
}

#[cfg(test)]
Expand Down
5 changes: 2 additions & 3 deletions src/toolchain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -407,8 +407,7 @@ impl Toolchain {
.collect()),
Err(_) if not_installed => Err(ToolchainError::NotInstalled.into()),
Err(err) => Err(anyhow!(err).context(format!(
"failed to read the list of installed {}s for {} with rustup",
thing, name
"failed to read the list of installed {thing}s for {name} with rustup"
))),
}
}
Expand All @@ -419,7 +418,7 @@ impl Toolchain {
Command::new(workspace, &RUSTUP)
.args(&["toolchain", "uninstall", &name])
.run()
.with_context(|| format!("unable to uninstall toolchain {} via rustup", name))?;
.with_context(|| format!("unable to uninstall toolchain {name} via rustup"))?;
Ok(())
}

Expand Down
4 changes: 2 additions & 2 deletions src/tools/rustup.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ impl Tool for Rustup {
.error_for_status()?;

let tempdir = tempdir()?;
let installer = &tempdir.path().join(format!("rustup-init{}", EXE_SUFFIX));
let installer = &tempdir.path().join(format!("rustup-init{EXE_SUFFIX}"));
{
let mut file = File::create(installer)?;
io::copy(&mut resp, &mut file)?;
Expand Down Expand Up @@ -81,7 +81,7 @@ impl Tool for Rustup {
Command::new(workspace, &RUSTUP)
.args(&["update", MAIN_TOOLCHAIN_NAME])
.run()
.with_context(|| format!("failed to update main toolchain {}", MAIN_TOOLCHAIN_NAME))?;
.with_context(|| format!("failed to update main toolchain {MAIN_TOOLCHAIN_NAME}"))?;
Ok(())
}
}
2 changes: 1 addition & 1 deletion src/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ pub(crate) fn file_lock<T>(
let mut message_displayed = false;
while let Err(err) = file.try_lock_exclusive() {
if !message_displayed && err.kind() == fs2::lock_contended_error().kind() {
warn!("blocking on other processes finishing to {}", msg);
warn!("blocking on other processes finishing to {msg}");
message_displayed = true;
}
file.lock_exclusive()?;
Expand Down

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

Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
[package]
name = "invalid-cargotoml-conflicting-links"
version = "0.1.0"
authors = ["Pietro Albini <pietro@pietroalbini.org>"]
edition = "2018"

# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html

[dependencies]
ring_13 = { version = "=0.13.5", package = "ring" }
ring_14 = { version = "=0.14.6", package = "ring" }


[replace]
"ring:0.13.5" = { git = "https://github.com/briansmith/ring.git", rev = "704e4216a397bd830479bcd6d7dd67fc62cdbe67" }
"ring:0.14.6" = { git = "https://github.com/briansmith/ring.git", rev = "ef85df478152aa3fe06c811309379efa08f8a529" }
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
fn main() {
println!("Hello, world!");
}
12 changes: 12 additions & 0 deletions tests/buildtest/crates/invalid-cargotoml-content-deps/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
[package]
name = "invalid-cargotoml-content-deps"
version = "0.1.0"
authors = ["Pietro Albini <pietro@pietroalbini.org>"]
edition = "2018"

# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html

[dependencies]
parity-db = "=0.2.3"
# invalid package name in invalid-cargotoml-content is too invalid
# invalid-cargotoml-content = { git = "https://github.com/rust-lang/rustwide.git", rev = "ee102383cbe40aafdfce7bf04a120226c16a8983" }
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
fn main() {
println!("Hello, world!");
}

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

Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
[package]
name = "invalid-cargotoml-content-type-in-deps"
version = "0.1.0"
authors = []
edition = "2018"

# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html

[dependencies]
libressl-pnacl-sys = "=2.1.0"
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
fn main() {
println!("Hello, world!");
}
12 changes: 12 additions & 0 deletions tests/buildtest/crates/invalid-cargotoml-cyclic-feature/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
[package]
name = "invalid-cargotoml-cyclic-feature"
version = "0.1.0"
authors = ["Pietro Albini <pietro@pietroalbini.org>"]
edition = "2018"

# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html

[dependencies]

[features]
cyclic = ["cyclic"]
Loading