Skip to content

Commit

Permalink
Do not propagate dev deps
Browse files Browse the repository at this point in the history
commit-id:910fac15
  • Loading branch information
maciektr committed Dec 27, 2023
1 parent ce19e9a commit 47619d9
Show file tree
Hide file tree
Showing 4 changed files with 51 additions and 14 deletions.
13 changes: 13 additions & 0 deletions scarb/src/core/manifest/dependency.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,19 @@ pub enum DepKind {
Target(TargetKind),
}

impl DepKind {
pub fn is_propagated(&self) -> bool {
!self.is_test()
}

pub fn is_test(&self) -> bool {
match self {
DepKind::Target(kind) => kind.is_test(),
_ => false,
}
}
}

impl Deref for ManifestDependency {
type Target = ManifestDependencyInner;

Expand Down
23 changes: 23 additions & 0 deletions scarb/src/core/manifest/summary.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,14 @@ impl Summary {
self.dependencies.iter().chain(self.implicit_dependencies())
}

pub fn filtered_full_dependencies(
&self,
dep_filter: DependencyFilter,
) -> impl Iterator<Item = &ManifestDependency> {
self.full_dependencies()
.filter(move |dep| dep_filter.filter(dep))
}

pub fn implicit_dependencies(&self) -> impl Iterator<Item = &ManifestDependency> {
static CORE_DEPENDENCY: Lazy<ManifestDependency> = Lazy::new(|| {
// NOTE: Pin `core` to exact version, because we know that's the only one we have.
Expand Down Expand Up @@ -103,3 +111,18 @@ impl Summary {
.filter(|dep| dep.kind == DepKind::Normal)
}
}

#[derive(Default)]
pub struct DependencyFilter {
pub do_propagate: bool,
}

impl DependencyFilter {
pub fn propagation(do_propagate: bool) -> Self {
Self { do_propagate }
}

pub fn filter(&self, dep: &ManifestDependency) -> bool {
self.do_propagate || dep.kind.is_propagated()
}
}
16 changes: 13 additions & 3 deletions scarb/src/resolver/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,14 @@ use anyhow::{bail, Result};
use indoc::{formatdoc, indoc};
use petgraph::graphmap::DiGraphMap;
use scarb_ui::Ui;
use std::collections::HashSet;

use crate::core::lockfile::Lockfile;
use crate::core::registry::Registry;
use crate::core::resolver::{DependencyEdge, Resolve};
use crate::core::{
DepKind, DependencyVersionReq, ManifestDependency, PackageId, Summary, TargetKind,
DepKind, DependencyFilter, DependencyVersionReq, ManifestDependency, PackageId, Summary,
TargetKind,
};

/// Builds the list of all packages required to build the first argument.
Expand Down Expand Up @@ -41,6 +43,10 @@ pub async fn resolve(
// TODO(#2): This is very bad, use PubGrub here.
let mut graph = DiGraphMap::<PackageId, DependencyEdge>::new();

let main_packages = summaries
.iter()
.map(|sum| sum.package_id)
.collect::<HashSet<PackageId>>();
let mut packages: HashMap<_, _> = HashMap::from_iter(
summaries
.iter()
Expand All @@ -59,7 +65,10 @@ pub async fn resolve(
for package_id in queue {
graph.add_node(package_id);

for dep in summaries[&package_id].clone().full_dependencies() {
let summary = summaries[&package_id].clone();
let dep_filter =
DependencyFilter::propagation(main_packages.contains(&summary.package_id));
for dep in summary.filtered_full_dependencies(dep_filter) {
let dep = rewrite_dependency_source_id(registry, &package_id, dep).await?;

let locked_package_id = lockfile.packages_matching(dep.clone());
Expand Down Expand Up @@ -129,7 +138,8 @@ pub async fn resolve(
// Detect incompatibilities and bail in case ones are found.
let mut incompatibilities = Vec::new();
for from_package in graph.nodes() {
for manifest_dependency in summaries[&from_package].full_dependencies() {
let dep_filter = DependencyFilter::propagation(main_packages.contains(&from_package));
for manifest_dependency in summaries[&from_package].filtered_full_dependencies(dep_filter) {
let to_package = packages[&manifest_dependency.name];
if !manifest_dependency.matches_package_id(to_package) {
let message = format!(
Expand Down
13 changes: 2 additions & 11 deletions scarb/tests/metadata.rs
Original file line number Diff line number Diff line change
Expand Up @@ -273,9 +273,7 @@ fn dev_dependencies() {
}

#[test]
#[ignore = "not implemented yet"]
fn dev_deps_are_not_propagated() {
// TODO(maciektr): Make sure dev-deps are not propagated.
let t = assert_fs::TempDir::new().unwrap();

let dep1 = t.child("dep1");
Expand All @@ -290,7 +288,7 @@ fn dev_deps_are_not_propagated() {
let pkg = t.child("pkg");
ProjectBuilder::start()
.name("x")
.dep("dep2", &dep2)
.dev_dep("dep2", &dep2)
.build(&pkg);

let metadata = Scarb::quick_snapbox()
Expand All @@ -314,10 +312,6 @@ fn dev_deps_are_not_propagated() {
"test_plugin".to_string()
]
),
(
"dep1".to_string(),
vec!["core".to_string(), "test_plugin".to_string()]
),
(
"dep2".to_string(),
vec![
Expand All @@ -332,10 +326,7 @@ fn dev_deps_are_not_propagated() {
assert_eq!(
units_and_components(metadata),
BTreeMap::from_iter(vec![
(
"x".to_string(),
vec!["core".to_string(), "dep2".to_string(), "x".to_string()]
),
("x".to_string(), vec!["core".to_string(), "x".to_string()]),
(
"x_unittest".to_string(),
vec!["core".to_string(), "dep2".to_string(), "x".to_string()]
Expand Down

0 comments on commit 47619d9

Please sign in to comment.