Skip to content

Revert "more explicit build dependency handling" #752

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.

Sign up for GitHub

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
Shnatsel merged 1 commit into from
Jul 17, 2024
Merged

Revert "more explicit build dependency handling" #752

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
15 changes: 4 additions & 11 deletions cargo-cyclonedx/src/cli.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,10 +100,6 @@ Defaults to the host target, as printed by 'rustc -vV'"
/// The CycloneDX specification version to output: `1.3`, `1.4` or `1.5`. Defaults to 1.3
#[clap(long = "spec-version")]
pub spec_version: Option<SpecVersion>,

/// List only dependencies of kind normal (no build deps, no dev deps)
#[clap(name = "only-normal-deps", long = "only-normal-deps")]
pub only_normal_deps: bool,
}

impl Args {
Expand Down Expand Up @@ -174,7 +170,6 @@ impl Args {

let describe = self.describe;
let spec_version = self.spec_version;
let only_normal_deps = Some(self.only_normal_deps);

Ok(SbomConfig {
format: self.format,
Expand All @@ -185,7 +180,6 @@ impl Args {
license_parser,
describe,
spec_version,
only_normal_deps,
})
}
}
Expand All @@ -196,11 +190,6 @@ pub enum ArgsError {
FilenameOverrideError(#[from] FilenameOverrideError),
}

#[cfg(test)]
pub fn parse_to_config(args: &[&str]) -> SbomConfig {
Args::parse_from(args.iter()).as_config().unwrap()
}

#[cfg(test)]
mod tests {
use super::*;
Expand Down Expand Up @@ -233,6 +222,10 @@ mod tests {
assert!(!contains_feature(&config, ""));
}

fn parse_to_config(args: &[&str]) -> SbomConfig {
Args::parse_from(args.iter()).as_config().unwrap()
}

fn contains_feature(config: &SbomConfig, feature: &str) -> bool {
config
.features
Expand Down
2 changes: 0 additions & 2 deletions cargo-cyclonedx/src/config.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ pub struct SbomConfig {
pub license_parser: Option<LicenseParserOptions>,
pub describe: Option<Describe>,
pub spec_version: Option<SpecVersion>,
pub only_normal_deps: Option<bool>,
}

impl SbomConfig {
Expand All @@ -58,7 +57,6 @@ impl SbomConfig {
.or_else(|| self.license_parser.clone()),
describe: other.describe.or(self.describe),
spec_version: other.spec_version.or(self.spec_version),
only_normal_deps: other.only_normal_deps.or(self.only_normal_deps),
}
}

Expand Down
141 changes: 17 additions & 124 deletions cargo-cyclonedx/src/generator.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
use crate::config::Describe;
use std::cmp::min;
/*
* This file is part of CycloneDX Rust Cargo.
*
Expand Down Expand Up @@ -55,8 +54,8 @@ use once_cell::sync::Lazy;
use regex::Regex;

use log::Level;
use std::collections::BTreeMap;
use std::collections::HashMap;
use std::collections::{BTreeMap, LinkedList};
use std::convert::TryFrom;
use std::fs::File;
use std::io::BufWriter;
Expand All @@ -69,42 +68,6 @@ use validator::validate_email;
// Maps from PackageId to Package for efficiency - faster lookups than in a Vec
type PackageMap = BTreeMap<PackageId, Package>;
type ResolveMap = BTreeMap<PackageId, Node>;
type DependencyKindMap = BTreeMap<PackageId, DependencyKind>;

/// The values are ordered from weakest to strongest so that casting to integer would make sense
#[derive(Debug, PartialEq, Eq, PartialOrd, Ord, Copy, Clone)]
enum PrivateDepKind {
Development,
Build,
Runtime,
}
impl From<PrivateDepKind> for DependencyKind {
fn from(priv_kind: PrivateDepKind) -> Self {
match priv_kind {
PrivateDepKind::Development => DependencyKind::Development,
PrivateDepKind::Build => DependencyKind::Build,
PrivateDepKind::Runtime => DependencyKind::Normal,
}
}
}

impl From<&DependencyKind> for PrivateDepKind {
fn from(kind: &DependencyKind) -> Self {
match kind {
DependencyKind::Normal => PrivateDepKind::Runtime,
DependencyKind::Development => PrivateDepKind::Development,
DependencyKind::Build => PrivateDepKind::Build,
_ => panic!("Unknown dependency kind"),
}
}
}

fn strongest_dep_kind(deps: &[cargo_metadata::DepKindInfo]) -> PrivateDepKind {
deps.iter()
.map(|d| PrivateDepKind::from(&d.kind))
.max()
.unwrap_or(PrivateDepKind::Runtime) // for compatibility with Rust earlier than 1.41
}

pub struct SbomGenerator {
config: SbomConfig,
Expand Down Expand Up @@ -135,13 +98,11 @@ impl SbomGenerator {
for member in members.iter() {
log::trace!("Processing the package {}", member);

let dep_kinds = index_dep_kinds(member, &resolve);

let (dependencies, pruned_resolve) =
if config.included_dependencies() == IncludedDependencies::AllDependencies {
all_dependencies(member, &packages, &resolve, config)
all_dependencies(member, &packages, &resolve)
} else {
top_level_dependencies(member, &packages, &resolve, config)
top_level_dependencies(member, &packages, &resolve)
};

let manifest_path = packages[member].manifest_path.clone().into_std_path_buf();
Expand All @@ -167,7 +128,7 @@ impl SbomGenerator {
crate_hashes,
};
let (bom, target_kinds) =
generator.create_bom(member, &dependencies, &pruned_resolve, &dep_kinds)?;
generator.create_bom(member, &dependencies, &pruned_resolve)?;

let generated = GeneratedSbom {
bom,
Expand All @@ -188,15 +149,14 @@ impl SbomGenerator {
package: &PackageId,
packages: &PackageMap,
resolve: &ResolveMap,
dep_kinds: &DependencyKindMap,
) -> Result<(Bom, TargetKinds), GeneratorError> {
let mut bom = Bom::default();
let root_package = &packages[package];

let components: Vec<_> = packages
.values()
.filter(|p| &p.id != package)
.map(|component| self.create_component(component, root_package, dep_kinds))
.map(|component| self.create_component(component, root_package))
.collect();

bom.components = Some(Components(components));
Expand All @@ -210,12 +170,7 @@ impl SbomGenerator {
Ok((bom, target_kinds))
}

fn create_component(
&self,
package: &Package,
root_package: &Package,
dep_kinds: &DependencyKindMap,
) -> Component {
fn create_component(&self, package: &Package, root_package: &Package) -> Component {
let name = package.name.to_owned().trim().to_string();
let version = package.version.to_string();

Expand All @@ -235,13 +190,7 @@ impl SbomGenerator {
);

component.purl = purl;
component.scope = match dep_kinds
.get(&package.id)
.unwrap_or(&DependencyKind::Normal)
{
DependencyKind::Normal => Some(Scope::Required),
_ => Some(Scope::Excluded),
};
component.scope = Some(Scope::Required);
component.external_references = Self::get_external_references(package);
component.licenses = self.get_licenses(package);
component.hashes = self.get_hashes(package);
Expand All @@ -257,7 +206,7 @@ impl SbomGenerator {
/// Same as [Self::create_component] but also includes information
/// on binaries and libraries comprising it as subcomponents
fn create_toplevel_component(&self, package: &Package) -> (Component, TargetKinds) {
let mut top_component = self.create_component(package, package, &DependencyKindMap::new());
let mut top_component = self.create_component(package, package);
let mut subcomponents: Vec<Component> = Vec::new();
let mut target_kinds = HashMap::new();
for tgt in filter_targets(&package.targets) {
Expand Down Expand Up @@ -593,49 +542,6 @@ fn index_resolve(packages: Vec<Node>) -> ResolveMap {
.collect()
}

fn index_dep_kinds(root: &PackageId, resolve: &ResolveMap) -> DependencyKindMap {
// cache strongest found dependency kind for every node
let mut id_to_dep_kind: HashMap<&PackageId, PrivateDepKind> = HashMap::new();
id_to_dep_kind.insert(root, PrivateDepKind::Runtime);

let root_node = &resolve[root];
let mut nodes_to_visit = LinkedList::<&Node>::new();
nodes_to_visit.push_front(root_node);

// perform a simple iterative DFS over the dependencies,
// mark child deps with the minimum of parent kind and their own strongest value
// therefore e.g. mark decendants of build dependencies as build dependencies,
// as long as they never occur as normal dependency.
while !nodes_to_visit.is_empty() {
let node = nodes_to_visit.pop_front().unwrap();
let parent_node_kind = id_to_dep_kind[&node.id];

for child_dep in &node.deps {
let child_node = &resolve[&child_dep.pkg];
let dep_kind = strongest_dep_kind(child_dep.dep_kinds.as_slice());
let child_node_kind = min(parent_node_kind, dep_kind);

let dep_kind_on_previous_visit = id_to_dep_kind.get(&child_node.id);
// insert/update a nodes dependency kind, when its new or stronger than the previous value
if dep_kind_on_previous_visit.is_none()
|| child_node_kind > *dep_kind_on_previous_visit.unwrap()
{
let _ = id_to_dep_kind.insert(&child_node.id, child_node_kind);
}

// cargo metadata already return a DAG, so assume this will terminate
// and don't mark already visited notes. It's ok and necessary to visit
// deps as often as they occur.
nodes_to_visit.push_front(child_node);
}
}

id_to_dep_kind
.iter()
.map(|(x, y)| ((*x).clone(), DependencyKind::from(*y)))
.collect()
}

#[derive(Error, Debug)]
pub enum GeneratorError {
#[error("Expected a root package in the cargo config: {config_filepath}")]
Expand Down Expand Up @@ -678,15 +584,13 @@ fn top_level_dependencies(
root: &PackageId,
packages: &PackageMap,
resolve: &ResolveMap,
config: &SbomConfig,
) -> (PackageMap, ResolveMap) {
log::trace!("Adding top-level dependencies to SBOM");

// Only include packages that have dependency kinds other than "Development"
let root_node = add_filtered_dependencies(&resolve[root], config);
let root_node = strip_dev_dependencies(&resolve[root]);

let mut pkg_result = PackageMap::new();

// Record the root package, then its direct non-dev dependencies
pkg_result.insert(root.to_owned(), packages[root].to_owned());
for id in &root_node.dependencies {
Expand All @@ -711,7 +615,6 @@ fn all_dependencies(
root: &PackageId,
packages: &PackageMap,
resolve: &ResolveMap,
config: &SbomConfig,
) -> (PackageMap, ResolveMap) {
log::trace!("Adding all dependencies to SBOM");

Expand All @@ -732,11 +635,9 @@ fn all_dependencies(
// If we haven't processed this node yet...
if !out_resolve.contains_key(&node.id) {
// Add the node to the output
out_resolve.insert(node.id.to_owned(), add_filtered_dependencies(node, config));
out_resolve.insert(node.id.to_owned(), strip_dev_dependencies(node));
// Queue its dependencies for the next BFS loop iteration
next_queue.extend(
filtered_dependencies(&node.deps, config).map(|dep| &resolve[&dep.pkg]),
);
next_queue.extend(non_dev_dependencies(&node.deps).map(|dep| &resolve[&dep.pkg]));
}
}
std::mem::swap(&mut current_queue, &mut next_queue);
Expand All @@ -752,27 +653,20 @@ fn all_dependencies(
(out_packages, out_resolve)
}

fn add_filtered_dependencies(node: &Node, config: &SbomConfig) -> Node {
fn strip_dev_dependencies(node: &Node) -> Node {
let mut node = node.clone();
node.deps = filtered_dependencies(&node.deps, config).cloned().collect();
node.deps = non_dev_dependencies(&node.deps).cloned().collect();
node.dependencies = node.deps.iter().map(|d| d.pkg.to_owned()).collect();
node
}

/// Filters out dependencies only used for development, and not affecting the final binary.
/// These are specified under `[dev-dependencies]` in Cargo.toml.
fn filtered_dependencies<'a>(
input: &'a [NodeDep],
config: &'a SbomConfig,
) -> impl Iterator<Item = &'a NodeDep> {
fn non_dev_dependencies(input: &[NodeDep]) -> impl Iterator<Item = &NodeDep> {
input.iter().filter(|p| {
p.dep_kinds.iter().any(|dep| {
if let Some(true) = config.only_normal_deps {
dep.kind == DependencyKind::Normal
} else {
dep.kind != DependencyKind::Development
}
})
p.dep_kinds
.iter()
.any(|dep| dep.kind != DependencyKind::Development)
})
}

Expand All @@ -783,7 +677,6 @@ fn filtered_dependencies<'a>(
/// * `package_name` - Package from which this SBOM was generated
/// * `sbom_config` - Configuration options used during generation
/// * `target_kinds` - Detailed information on the kinds of targets in `sbom`
#[derive(Debug)]
pub struct GeneratedSbom {
pub bom: Bom,
pub manifest_path: PathBuf,
Expand Down
Loading