From 8b8822d965af0b9fb9fba4a022b40ec4ef843537 Mon Sep 17 00:00:00 2001 From: Samuel Moelius Date: Tue, 11 Feb 2025 01:04:22 +0000 Subject: [PATCH] Have `cargo metadata` and `cargo fetch` use the same `cargo` when building library packages --- Cargo.lock | 3 +++ .../tests/integration/library_packages.rs | 21 +++++++++++++++++++ dylint/src/library_packages/cargo_cli/mod.rs | 3 ++- .../Cargo.toml | 10 +++++++++ .../rust-toolchain | 2 ++ .../src/lib.rs | 1 + internal/Cargo.toml | 3 +++ internal/src/cargo.rs | 9 ++++++++ internal/src/env.rs | 2 +- 9 files changed, 52 insertions(+), 2 deletions(-) create mode 100644 fixtures/library_packages_with_rust_toolchain/Cargo.toml create mode 100644 fixtures/library_packages_with_rust_toolchain/rust-toolchain create mode 100644 fixtures/library_packages_with_rust_toolchain/src/lib.rs diff --git a/Cargo.lock b/Cargo.lock index 323d78a87..4f25c6ed4 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -985,6 +985,7 @@ version = "3.5.0" dependencies = [ "ansi_term", "anyhow", + "assert_cmd", "bitflags 2.8.0", "cargo-util", "cargo_metadata", @@ -995,11 +996,13 @@ dependencies = [ "if_chain", "log", "once_cell", + "predicates", "regex", "rust-embed", "rustversion", "semver", "serde", + "tempfile", "thiserror 2.0.11", "toml", "toml_edit", diff --git a/cargo-dylint/tests/integration/library_packages.rs b/cargo-dylint/tests/integration/library_packages.rs index c2d29dd86..2ae9267c1 100644 --- a/cargo-dylint/tests/integration/library_packages.rs +++ b/cargo-dylint/tests/integration/library_packages.rs @@ -88,6 +88,27 @@ fn library_packages_in_dylint_toml() { )); } +#[test] +fn library_packages_with_rust_toolchain() { + let assert = std::process::Command::cargo_bin("cargo-dylint") + .unwrap() + .current_dir("../fixtures/library_packages_with_rust_toolchain") + .env(env::RUST_LOG, "debug") + .args(["dylint", "--all"]) + .assert() + .success(); + + if cfg!(all( + feature = "cargo-cli", + target_arch = "x86_64", + not(target_os = "windows") + )) { + assert.stderr(predicate::str::contains( + r#"/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/bin/cargo" "fetch""#, + )); + } +} + #[test] fn list() { let tempdir = tempdir().unwrap(); diff --git a/dylint/src/library_packages/cargo_cli/mod.rs b/dylint/src/library_packages/cargo_cli/mod.rs index bc6645775..eedde9236 100644 --- a/dylint/src/library_packages/cargo_cli/mod.rs +++ b/dylint/src/library_packages/cargo_cli/mod.rs @@ -19,7 +19,7 @@ use crate::opts; use anyhow::{anyhow, bail, ensure, Context, Result}; use cargo_metadata::{Metadata, MetadataCommand}; use cargo_util_schemas::manifest::TomlDetailedDependency; -use dylint_internal::{home::cargo_home, packaging::isolate, CommandExt}; +use dylint_internal::{cargo::stable_cargo_path, home::cargo_home, packaging::isolate, CommandExt}; use semver::Version; use serde::Serialize; use std::{ @@ -288,6 +288,7 @@ fn cargo_fetch(path: &Path) -> Result { fn cargo_metadata(path: &Path) -> Result { MetadataCommand::new() + .cargo_path(stable_cargo_path()) .current_dir(path) .exec() .map_err(Into::into) diff --git a/fixtures/library_packages_with_rust_toolchain/Cargo.toml b/fixtures/library_packages_with_rust_toolchain/Cargo.toml new file mode 100644 index 000000000..5f9feec41 --- /dev/null +++ b/fixtures/library_packages_with_rust_toolchain/Cargo.toml @@ -0,0 +1,10 @@ +[package] +name = "library_packages_with_rust_toolchain" +version = "0.1.0" +edition = "2021" +publish = false + +[workspace.metadata.dylint] +libraries = [ + { git = "https://github.com/trailofbits/dylint", pattern = "examples/general" }, +] diff --git a/fixtures/library_packages_with_rust_toolchain/rust-toolchain b/fixtures/library_packages_with_rust_toolchain/rust-toolchain new file mode 100644 index 000000000..5d56faf9a --- /dev/null +++ b/fixtures/library_packages_with_rust_toolchain/rust-toolchain @@ -0,0 +1,2 @@ +[toolchain] +channel = "nightly" diff --git a/fixtures/library_packages_with_rust_toolchain/src/lib.rs b/fixtures/library_packages_with_rust_toolchain/src/lib.rs new file mode 100644 index 000000000..8b1378917 --- /dev/null +++ b/fixtures/library_packages_with_rust_toolchain/src/lib.rs @@ -0,0 +1 @@ + diff --git a/internal/Cargo.toml b/internal/Cargo.toml index 0cc4bdc8a..7f1c69cb0 100644 --- a/internal/Cargo.toml +++ b/internal/Cargo.toml @@ -34,6 +34,9 @@ toml_edit = { workspace = true, optional = true } walkdir = { workspace = true, optional = true } [dev-dependencies] +assert_cmd = { workspace = true } +predicates = { workspace = true } +tempfile = { workspace = true } toml_edit = { workspace = true } [features] diff --git a/internal/src/cargo.rs b/internal/src/cargo.rs index 43223a311..8532d68c2 100644 --- a/internal/src/cargo.rs +++ b/internal/src/cargo.rs @@ -15,6 +15,10 @@ pub use home::cargo_home; static STABLE_CARGO: Lazy = Lazy::new(|| { let mut command = Command::new("rustup"); + // smoelius: Rustup 1.27.1 doesn't properly handle the case where the toolchain is specified via + // both the `RUSTUP_TOOLCHAIN` environment variable and the command line (e.g., `+stable`). This + // bug is fixed in Rustup's `master` branch, though. + command.env_remove("RUSTUP_TOOLCHAIN"); command.args(["+stable", "which", "cargo"]); let output = command.logged_output(true).unwrap(); assert!(output.status.success()); @@ -202,3 +206,8 @@ pub fn package(metadata: &Metadata, package_id: &PackageId) -> Result { .cloned() .ok_or_else(|| anyhow!("Could not find package")) } + +#[must_use] +pub fn stable_cargo_path() -> &'static Path { + &STABLE_CARGO +} diff --git a/internal/src/env.rs b/internal/src/env.rs index 9abac8bdc..bcb257567 100644 --- a/internal/src/env.rs +++ b/internal/src/env.rs @@ -34,7 +34,7 @@ declare_const!(RUSTC_WORKSPACE_WRAPPER); declare_const!(RUSTFLAGS); declare_const!(RUSTUP_HOME); declare_const!(RUSTUP_TOOLCHAIN); -declare_const!(RUST_BACKTRACE); +declare_const!(RUST_LOG); declare_const!(TARGET); /// Returns true if the environment variable `key` is set to a non-zero value.