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

Have cargo metadata and cargo fetch use the same cargo when building library packages #1519

Merged
merged 1 commit into from
Feb 11, 2025
Merged
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
3 changes: 3 additions & 0 deletions Cargo.lock

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

21 changes: 21 additions & 0 deletions cargo-dylint/tests/integration/library_packages.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
3 changes: 2 additions & 1 deletion dylint/src/library_packages/cargo_cli/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::{
Expand Down Expand Up @@ -288,6 +288,7 @@ fn cargo_fetch(path: &Path) -> Result<std::process::Output> {

fn cargo_metadata(path: &Path) -> Result<Metadata> {
MetadataCommand::new()
.cargo_path(stable_cargo_path())
.current_dir(path)
.exec()
.map_err(Into::into)
Expand Down
10 changes: 10 additions & 0 deletions fixtures/library_packages_with_rust_toolchain/Cargo.toml
Original file line number Diff line number Diff line change
@@ -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" },
]
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
[toolchain]
channel = "nightly"
1 change: 1 addition & 0 deletions fixtures/library_packages_with_rust_toolchain/src/lib.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@

3 changes: 3 additions & 0 deletions internal/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
9 changes: 9 additions & 0 deletions internal/src/cargo.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,10 @@ pub use home::cargo_home;

static STABLE_CARGO: Lazy<PathBuf> = 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());
Expand Down Expand Up @@ -202,3 +206,8 @@ pub fn package(metadata: &Metadata, package_id: &PackageId) -> Result<Package> {
.cloned()
.ok_or_else(|| anyhow!("Could not find package"))
}

#[must_use]
pub fn stable_cargo_path() -> &'static Path {
&STABLE_CARGO
}
2 changes: 1 addition & 1 deletion internal/src/env.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down