Skip to content

Commit

Permalink
Add test to prevent circular dependencies from being added (apache#9292)
Browse files Browse the repository at this point in the history
  • Loading branch information
andygrove authored Feb 25, 2024
1 parent 148b4d2 commit 10cbb05
Show file tree
Hide file tree
Showing 4 changed files with 524 additions and 180 deletions.
1 change: 1 addition & 0 deletions datafusion/core/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,7 @@ zstd = { version = "0.13", optional = true, default-features = false }
[dev-dependencies]
async-trait = { workspace = true }
bigdecimal = { workspace = true }
cargo = "0.77.0"
criterion = { version = "0.5", features = ["async_tokio"] }
csv = "1.1.6"
ctor = { workspace = true }
Expand Down
78 changes: 78 additions & 0 deletions datafusion/core/tests/depcheck.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
// Licensed to the Apache Software Foundation (ASF) under one
// or more contributor license agreements. See the NOTICE file
// distributed with this work for additional information
// regarding copyright ownership. The ASF licenses this file
// to you under the Apache License, Version 2.0 (the
// "License"); you may not use this file except in compliance
// with the License. You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing,
// software distributed under the License is distributed on an
// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
// KIND, either express or implied. See the License for the
// specific language governing permissions and limitations
// under the License.

/// Check for circular dependencies between DataFusion crates
use std::collections::{HashMap, HashSet};
use std::env;
use std::path::Path;

use cargo::util::config::Config;
#[test]
fn test_deps() -> Result<(), Box<dyn std::error::Error>> {
let config = Config::default()?;
let path = env::var("CARGO_MANIFEST_DIR").unwrap();
let dir = Path::new(&path);
let root_cargo_toml = dir.join("Cargo.toml");
let workspace = cargo::core::Workspace::new(&root_cargo_toml, &config)?;
let (_, resolve) = cargo::ops::resolve_ws(&workspace)?;

let mut package_deps = HashMap::new();
for package_id in resolve
.iter()
.filter(|id| id.name().starts_with("datafusion"))
{
let deps: Vec<String> = resolve
.deps(package_id)
.filter(|(package_id, _)| package_id.name().starts_with("datafusion"))
.map(|(package_id, _)| package_id.name().to_string())
.collect();
package_deps.insert(package_id.name().to_string(), deps);
}

// check for circular dependencies
for (root_package, deps) in &package_deps {
let mut seen = HashSet::new();
for dep in deps {
check_circular_deps(root_package, dep, &package_deps, &mut seen);
}
}

Ok(())
}

fn check_circular_deps(
root_package: &str,
current_dep: &str,
package_deps: &HashMap<String, Vec<String>>,
seen: &mut HashSet<String>,
) {
if root_package == current_dep {
panic!(
"circular dependency detected from {root_package} to self via one of {:?}",
seen
);
}
if seen.contains(current_dep) {
return;
}
seen.insert(current_dep.to_string());
if let Some(deps) = package_deps.get(current_dep) {
for dep in deps {
check_circular_deps(root_package, dep, package_deps, seen);
}
}
}
101 changes: 69 additions & 32 deletions dev/release/crate-deps.dot
Original file line number Diff line number Diff line change
Expand Up @@ -16,39 +16,76 @@
// under the License.

digraph G {

datafusion_common

datafusion_expr -> datafusion_common

datafusion_sql -> datafusion_common
datafusion_sql -> datafusion_expr

datafusion_optimizer -> datafusion_common
datafusion_optimizer -> datafusion_expr

datafusion_physical_expr -> datafusion_common
datafusion_physical_expr -> datafusion_expr

datafusion_execution -> datafusion_common
datafusion_execution -> datafusion_expr

datafusion_examples
datafusion_examples -> datafusion
datafusion_examples -> datafusion_common
datafusion_examples -> datafusion_expr
datafusion_examples -> datafusion_optimizer
datafusion_examples -> datafusion_physical_expr
datafusion_examples -> datafusion_sql
datafusion_expr
datafusion_expr -> datafusion_common
datafusion_functions
datafusion_functions -> datafusion_common
datafusion_functions -> datafusion_execution
datafusion_functions -> datafusion_expr
datafusion_wasmtest
datafusion_wasmtest -> datafusion
datafusion_wasmtest -> datafusion_common
datafusion_wasmtest -> datafusion_execution
datafusion_wasmtest -> datafusion_expr
datafusion_wasmtest -> datafusion_optimizer
datafusion_wasmtest -> datafusion_physical_expr
datafusion_wasmtest -> datafusion_physical_plan
datafusion_wasmtest -> datafusion_sql
datafusion_common
datafusion_sql
datafusion_sql -> datafusion_common
datafusion_sql -> datafusion_expr
datafusion_physical_plan
datafusion_physical_plan -> datafusion_common
datafusion_physical_plan -> datafusion_execution
datafusion_physical_plan -> datafusion_expr
datafusion_physical_plan -> datafusion_physical_expr

datafusion -> datafusion_common
datafusion -> datafusion_execution
datafusion -> datafusion_expr
datafusion -> datafusion_optimizer
datafusion -> datafusion_physical_expr
datafusion -> datafusion_physical_plan
datafusion -> datafusion_sql

datafusion_proto -> datafusion

datafusion_substrait -> datafusion

datafusion_cli -> datafusion
}
datafusion_benchmarks
datafusion_benchmarks -> datafusion
datafusion_benchmarks -> datafusion_common
datafusion_benchmarks -> datafusion_proto
datafusion_docs_tests
datafusion_docs_tests -> datafusion
datafusion_optimizer
datafusion_optimizer -> datafusion_common
datafusion_optimizer -> datafusion_expr
datafusion_optimizer -> datafusion_physical_expr
datafusion_optimizer -> datafusion_sql
datafusion_proto
datafusion_proto -> datafusion
datafusion_proto -> datafusion_common
datafusion_proto -> datafusion_expr
datafusion_physical_expr
datafusion_physical_expr -> datafusion_common
datafusion_physical_expr -> datafusion_execution
datafusion_physical_expr -> datafusion_expr
datafusion_sqllogictest
datafusion_sqllogictest -> datafusion
datafusion_sqllogictest -> datafusion_common
datafusion
datafusion -> datafusion_common
datafusion -> datafusion_execution
datafusion -> datafusion_expr
datafusion -> datafusion_functions
datafusion -> datafusion_functions_array
datafusion -> datafusion_optimizer
datafusion -> datafusion_physical_expr
datafusion -> datafusion_physical_plan
datafusion -> datafusion_sql
datafusion_functions_array
datafusion_functions_array -> datafusion_common
datafusion_functions_array -> datafusion_execution
datafusion_functions_array -> datafusion_expr
datafusion_execution
datafusion_execution -> datafusion_common
datafusion_execution -> datafusion_expr
datafusion_substrait
datafusion_substrait -> datafusion
}
Loading

0 comments on commit 10cbb05

Please sign in to comment.