Skip to content

fix: Filter out unused cargo features from config #14910

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

Merged
merged 1 commit into from
May 26, 2023
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
1 change: 0 additions & 1 deletion .github/workflows/ci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,6 @@ jobs:
uses: actions/checkout@v3
with:
ref: ${{ github.event.pull_request.head.sha }}
fetch-depth: 20
Copy link
Member Author

Choose a reason for hiding this comment

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

Noticed that we still had this as well, we can drop that since we no longer have the relevant CI check running that needed it


- name: Install Rust toolchain
run: |
Expand Down
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 crates/project-model/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ serde.workspace = true
triomphe.workspace = true
anyhow = "1.0.62"
la-arena = { version = "0.3.0", path = "../../lib/la-arena" }
itertools = "0.10.5"

# local deps
base-db.workspace = true
Expand Down
28 changes: 22 additions & 6 deletions crates/project-model/src/build_scripts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,10 @@ use std::{
};

use cargo_metadata::{camino::Utf8Path, Message};
use itertools::Itertools;
use la_arena::ArenaMap;
use paths::{AbsPath, AbsPathBuf};
use rustc_hash::FxHashMap;
use rustc_hash::{FxHashMap, FxHashSet};
use semver::Version;
use serde::Deserialize;

Expand Down Expand Up @@ -56,7 +57,10 @@ impl BuildScriptOutput {
}

impl WorkspaceBuildScripts {
fn build_command(config: &CargoConfig) -> io::Result<Command> {
fn build_command(
config: &CargoConfig,
allowed_features: &FxHashSet<String>,
) -> io::Result<Command> {
let mut cmd = match config.run_build_script_command.as_deref() {
Some([program, args @ ..]) => {
let mut cmd = Command::new(program);
Expand Down Expand Up @@ -88,7 +92,12 @@ impl WorkspaceBuildScripts {
}
if !features.is_empty() {
cmd.arg("--features");
cmd.arg(features.join(" "));
cmd.arg(
features
.iter()
.filter(|&feat| allowed_features.contains(feat))
.join(","),
);
}
}
}
Expand Down Expand Up @@ -127,13 +136,20 @@ impl WorkspaceBuildScripts {
}
.as_ref();

match Self::run_per_ws(Self::build_command(config)?, workspace, current_dir, progress) {
let allowed_features = workspace.workspace_features();

match Self::run_per_ws(
Self::build_command(config, &allowed_features)?,
workspace,
current_dir,
progress,
) {
Ok(WorkspaceBuildScripts { error: Some(error), .. })
if toolchain.as_ref().map_or(false, |it| *it >= RUST_1_62) =>
{
// building build scripts failed, attempt to build with --keep-going so
// that we potentially get more build data
let mut cmd = Self::build_command(config)?;
let mut cmd = Self::build_command(config, &allowed_features)?;
cmd.args(["-Z", "unstable-options", "--keep-going"]).env("RUSTC_BOOTSTRAP", "1");
let mut res = Self::run_per_ws(cmd, workspace, current_dir, progress)?;
res.error = Some(error);
Expand Down Expand Up @@ -161,7 +177,7 @@ impl WorkspaceBuildScripts {
))
}
};
let cmd = Self::build_command(config)?;
let cmd = Self::build_command(config, &Default::default())?;
// NB: Cargo.toml could have been modified between `cargo metadata` and
// `cargo check`. We shouldn't assume that package ids we see here are
// exactly those from `config`.
Expand Down
17 changes: 16 additions & 1 deletion crates/project-model/src/cargo_workspace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use base_db::Edition;
use cargo_metadata::{CargoOpt, MetadataCommand};
use la_arena::{Arena, Idx};
use paths::{AbsPath, AbsPathBuf};
use rustc_hash::FxHashMap;
use rustc_hash::{FxHashMap, FxHashSet};
use serde::Deserialize;
use serde_json::from_value;

Expand Down Expand Up @@ -491,6 +491,21 @@ impl CargoWorkspace {
None
}

/// Returns the union of the features of all member crates in this workspace.
pub fn workspace_features(&self) -> FxHashSet<String> {
self.packages()
.filter_map(|package| {
let package = &self[package];
if package.is_member {
Some(package.features.keys().cloned())
} else {
None
}
})
.flatten()
.collect()
}

fn is_unique(&self, name: &str) -> bool {
self.packages.iter().filter(|(_, v)| v.name == name).count() == 1
}
Expand Down
14 changes: 10 additions & 4 deletions crates/rust-analyzer/src/cargo_target_spec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use std::mem;
use cfg::{CfgAtom, CfgExpr};
use ide::{Cancellable, FileId, RunnableKind, TestId};
use project_model::{self, CargoFeatures, ManifestPath, TargetKind};
use rustc_hash::FxHashSet;
use vfs::AbsPathBuf;

use crate::global_state::GlobalStateSnapshot;
Expand All @@ -21,6 +22,7 @@ pub(crate) struct CargoTargetSpec {
pub(crate) target: String,
pub(crate) target_kind: TargetKind,
pub(crate) required_features: Vec<String>,
pub(crate) features: FxHashSet<String>,
}

impl CargoTargetSpec {
Expand Down Expand Up @@ -73,12 +75,13 @@ impl CargoTargetSpec {
}
}

let target_required_features = if let Some(mut spec) = spec {
let (allowed_features, target_required_features) = if let Some(mut spec) = spec {
let allowed_features = mem::take(&mut spec.features);
let required_features = mem::take(&mut spec.required_features);
spec.push_to(&mut args, kind);
required_features
(allowed_features, required_features)
} else {
Vec::new()
(Default::default(), Default::default())
};

let cargo_config = snap.config.cargo();
Expand All @@ -97,7 +100,9 @@ impl CargoTargetSpec {
required_features(cfg, &mut feats);
}

feats.extend(features.iter().cloned());
feats.extend(
features.iter().filter(|&feat| allowed_features.contains(feat)).cloned(),
);
feats.extend(target_required_features);

feats.dedup();
Expand Down Expand Up @@ -136,6 +141,7 @@ impl CargoTargetSpec {
target: target_data.name.clone(),
target_kind: target_data.kind,
required_features: target_data.required_features.clone(),
features: package_data.features.keys().cloned().collect(),
};

Ok(Some(res))
Expand Down