Skip to content

Commit

Permalink
Merge pull request #752 from CycloneDX/revert-736-strip-build-deps
Browse files Browse the repository at this point in the history 
Revert "more explicit build dependency handling"
  • Loading branch information
@Shnatsel
Shnatsel authored Jul 17, 2024
2 parents 0f30ba3 + 057aa22 commit 51f8c79
Show file tree
Hide file tree
Showing 12 changed files with 33 additions and 267 deletions.
15 changes: 4 additions & 11 deletions cargo-cyclonedx/src/cli.rs
View file
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
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
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

0 comments on commit 51f8c79

Please sign in to comment.