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

Do not propagate dev deps #1050

Merged
merged 1 commit into from
Jan 15, 2024
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
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
6 changes: 0 additions & 6 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 Down Expand Up @@ -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 Down