Skip to content

Commit

Permalink
Prune non-importable audits.
Browse files Browse the repository at this point in the history
This addresses part of #580.
  • Loading branch information
afranchuk committed Dec 8, 2023
1 parent 03ae094 commit e78e696
Show file tree
Hide file tree
Showing 10 changed files with 354 additions and 47 deletions.
3 changes: 3 additions & 0 deletions src/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -682,6 +682,9 @@ pub struct PruneArgs {
/// Don't prune unused exemptions
#[clap(long, action)]
pub no_exemptions: bool,
/// Don't prune unused non-importable audits.
#[clap(long, action)]
pub no_audits: bool,
}

#[derive(clap::Args)]
Expand Down
53 changes: 32 additions & 21 deletions src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -541,6 +541,7 @@ fn cmd_init(_out: &Arc<dyn Out>, cfg: &Config, _sub_args: &InitArgs) -> Result<(
resolver::update_store(cfg, &mut store, |_| resolver::UpdateMode {
search_mode: resolver::SearchMode::RegenerateExemptions,
prune_exemptions: true,
prune_non_importable_audits: true,
prune_imports: true,
});

Expand Down Expand Up @@ -982,17 +983,18 @@ fn do_cmd_certify(
.validate(cfg.today(), false)
.expect("the new audit entry made the store invalid?");

// Minimize exemptions after adding the new audit. This will be used to
// potentially update imports, and remove now-unnecessary exemptions for the
// target package. We only prefer fresh imports and prune exemptions for the
// package we certified, to avoid unrelated changes.
// Minimize exemptions after adding the new audit. This will be used to potentially update
// imports, and remove now-unnecessary exemptions and audits for the target package. We only
// prefer fresh imports and prune exemptions for the package we certified, to avoid unrelated
// changes.
resolver::update_store(cfg, store, |name| resolver::UpdateMode {
search_mode: if name == &package[..] {
resolver::SearchMode::PreferFreshImports
} else {
resolver::SearchMode::PreferExemptions
},
prune_exemptions: name == &package[..],
prune_non_importable_audits: name == &package[..],
prune_imports: false,
});

Expand Down Expand Up @@ -1237,11 +1239,11 @@ fn cmd_import(
let cache = Cache::acquire(cfg)?;
tokio::runtime::Handle::current().block_on(store.go_online(cfg, &network, &cache, false))?;

// Update the store state, pruning unnecessary exemptions, and cleaning out
// unnecessary old imports.
// Update the store state, pruning unnecessary exemptions, audits, and imports.
resolver::update_store(cfg, &mut store, |_| resolver::UpdateMode {
search_mode: resolver::SearchMode::PreferFreshImports,
prune_exemptions: true,
prune_non_importable_audits: true,
prune_imports: true,
});

Expand Down Expand Up @@ -1493,17 +1495,18 @@ fn apply_cmd_trust(
.validate(cfg.today(), false)
.expect("the new trusted entry made the store invalid?");

// Minimize exemptions after adding the new trust entry. This will be used
// to potentially update imports, and remove now-unnecessary exemptions for
// the target package. We only prefer fresh imports and prune exemptions for
// the package we trusted, to avoid unrelated changes.
// Minimize exemptions and audits after adding the new trust entry. This will be used to
// potentially update imports, and remove now-unnecessary exemptions for the target package. We
// only prefer fresh imports and prune exemptions for the package we trusted, to avoid
// unrelated changes.
resolver::update_store(cfg, store, |name| resolver::UpdateMode {
search_mode: if name == package {
resolver::SearchMode::PreferFreshImports
} else {
resolver::SearchMode::PreferExemptions
},
prune_exemptions: name == package,
prune_non_importable_audits: name == package,
prune_imports: false,
});
Ok(())
Expand Down Expand Up @@ -1689,11 +1692,11 @@ fn cmd_regenerate_imports(
let network = Network::acquire(cfg);
let mut store = Store::acquire(cfg, network.as_ref(), true)?;

// Update the store state, pruning unnecessary exemptions, and cleaning out
// unnecessary old imports.
// Update the store state, pruning unnecessary exemptions, audits, and imports.
resolver::update_store(cfg, &mut store, |_| resolver::UpdateMode {
search_mode: resolver::SearchMode::PreferFreshImports,
prune_exemptions: true,
prune_non_importable_audits: true,
prune_imports: true,
});

Expand Down Expand Up @@ -1753,6 +1756,7 @@ fn cmd_regenerate_unpublished(
resolver::update_store(cfg, &mut store, |_| resolver::UpdateMode {
search_mode: resolver::SearchMode::PreferExemptions,
prune_exemptions: false,
prune_non_importable_audits: false,
prune_imports: false,
});

Expand Down Expand Up @@ -1954,6 +1958,7 @@ fn cmd_regenerate_exemptions(
resolver::update_store(cfg, &mut store, |_| resolver::UpdateMode {
search_mode: resolver::SearchMode::RegenerateExemptions,
prune_exemptions: true,
prune_non_importable_audits: true,
prune_imports: true,
});

Expand Down Expand Up @@ -2130,33 +2135,38 @@ fn cmd_check(
if !cfg.cli.locked {
// Simulate a full `fetch-imports` run, and record the potential
// pruned imports and exemptions.
let (pruned_imports, pruned_exemptions) =
resolver::get_store_updates(cfg, &store, |_| resolver::UpdateMode {
search_mode: resolver::SearchMode::PreferFreshImports,
prune_exemptions: true,
prune_imports: true,
});
let updates = resolver::get_store_updates(cfg, &store, |_| resolver::UpdateMode {
search_mode: resolver::SearchMode::PreferFreshImports,
prune_exemptions: true,
prune_non_importable_audits: true,
prune_imports: true,
});

// Perform a minimal store update to pull in necessary imports,
// while avoiding any other changes to exemptions or imports.
resolver::update_store(cfg, &mut store, |_| resolver::UpdateMode {
search_mode: resolver::SearchMode::PreferExemptions,
prune_exemptions: false,
prune_non_importable_audits: false,
prune_imports: false,
});

// XXX: Consider trying to be more precise here? Would require some
// more clever comparisons.
if store.config.exemptions != pruned_exemptions {
if store.config.exemptions != updates.exemptions {
warn!("Your supply-chain has unnecessary exemptions which could be relaxed or pruned.");
warn!(" Consider running `cargo vet prune` to prune unnecessary exemptions and imports.");
} else if store.imports != pruned_imports {
} else if store.imports != updates.imports {
warn!("Your supply-chain has unnecessary imports which could be pruned.");
warn!(" Consider running `cargo vet prune` to prune unnecessary imports.");
} else if store.audits.audits != updates.audits {
warn!("Your supply-chain has unnecessary audits which could be pruned.");
warn!(" Consider running `cargo vet prune` to prune unnecessary imports.");
}

// Check if we have `unpublished` entries for crates which have since been published.
let since_published: Vec<_> = pruned_imports
let since_published: Vec<_> = updates
.imports
.unpublished
.iter()
.filter(|(_, unpublished)| unpublished.iter().any(|u| !u.still_unpublished))
Expand Down Expand Up @@ -2293,6 +2303,7 @@ fn cmd_prune(
resolver::SearchMode::PreferFreshImports
},
prune_exemptions: !sub_args.no_exemptions,
prune_non_importable_audits: !sub_args.no_audits,
prune_imports: !sub_args.no_imports,
});

Expand Down
85 changes: 68 additions & 17 deletions src/resolver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -282,7 +282,10 @@ pub struct AuditGraph<'a> {
#[derive(Debug, Clone, Eq, PartialEq, Ord, PartialOrd)]
pub enum DeltaEdgeOrigin {
/// This edge represents an audit from the local audits.toml.
StoredLocalAudit { audit_index: usize },
StoredLocalAudit {
audit_index: usize,
importable: bool,
},
/// This edge represents an audit imported from a peer, potentially stored
/// in the local imports.lock.
ImportedAudit {
Expand All @@ -307,10 +310,13 @@ pub enum DeltaEdgeOrigin {
FreshExemption { version: VetVersion },
}

/// An indication of a required imported entry or exemption. Used to compute the
/// minimal set of possible imports for imports.lock.
/// An indication of a required local audit, imported entry, or exemption. Used to compute the
/// minimal set of possible imports for imports.lock and for pruning unused audits and exemptions.
#[derive(Debug, Clone, Eq, PartialEq, Ord, PartialOrd)]
pub enum RequiredEntry {
LocalAudit {
audit_index: usize,
},
Audit {
import_index: usize,
audit_index: usize,
Expand Down Expand Up @@ -1070,7 +1076,10 @@ impl<'a> AuditGraph<'a> {
import_index,
audit_index,
},
None => DeltaEdgeOrigin::StoredLocalAudit { audit_index },
None => DeltaEdgeOrigin::StoredLocalAudit {
audit_index,
importable: audit.importable,
},
},
audit,
)
Expand Down Expand Up @@ -1437,6 +1446,7 @@ fn search_for_path(
#[derive(Debug, Copy, Clone, Eq, PartialEq, Ord, PartialOrd)]
enum CaveatLevel {
None,
NonImportableAudit,
PreferredExemption,
PreferredUnpublished,
FreshImport,
Expand Down Expand Up @@ -1558,6 +1568,9 @@ fn search_for_path(

// Compute the level of caveats which are being added by the current edge
let edge_caveat_level = match &edge.origin {
DeltaEdgeOrigin::StoredLocalAudit { importable, .. } if !importable => {
CaveatLevel::NonImportableAudit
}
DeltaEdgeOrigin::Exemption { .. } if mode == SearchMode::PreferExemptions => {
CaveatLevel::PreferredExemption
}
Expand Down Expand Up @@ -2670,17 +2683,15 @@ fn resolve_package_required_entries(
) -> Option<SortedMap<RequiredEntry, CriteriaSet>> {
assert_eq!(graph.nodes.len(), requirements.len());

// Collect the list of third-party packages with the given name, along with
// their requirements.
// Collect the list of third-party packages with the given name, along with their requirements.
let packages: Vec<_> = graph
.nodes
.iter()
.zip(requirements)
.filter(|(package, _)| package.name == package_name && package.is_third_party)
.collect();

// If there are no third-party packages with the name, we definitely don't
// need any entries.
// If there are no third-party packages with the name, we definitely don't need any entries.
if packages.is_empty() {
return Some(SortedMap::new());
}
Expand Down Expand Up @@ -2744,7 +2755,9 @@ fn resolve_package_required_entries(
DeltaEdgeOrigin::Unpublished { unpublished_index } => {
add_entry(RequiredEntry::Unpublished { unpublished_index })
}
DeltaEdgeOrigin::StoredLocalAudit { .. } => {}
DeltaEdgeOrigin::StoredLocalAudit { audit_index, .. } => {
add_entry(RequiredEntry::LocalAudit { audit_index })
}
}
}
}
Expand All @@ -2759,29 +2772,40 @@ fn resolve_package_required_entries(
pub struct UpdateMode {
pub search_mode: SearchMode,
pub prune_exemptions: bool,
pub prune_non_importable_audits: bool,
pub prune_imports: bool,
}

pub(crate) struct StoreUpdates {
pub audits: SortedMap<String, Vec<AuditEntry>>,
pub imports: ImportsFile,
pub exemptions: SortedMap<PackageName, Vec<ExemptedDependency>>,
}

impl StoreUpdates {
pub fn apply(self, store: &mut Store) {
store.audits.audits = self.audits;
store.imports = self.imports;
store.config.exemptions = self.exemptions;
}
}

/// Refresh the state of the store, importing required audits, and optionally
/// pruning unnecessary exemptions and/or imports.
/// pruning unnecessary exemptions, audits, and/or imports.
pub fn update_store(
cfg: &Config,
store: &mut Store,
mode: impl FnMut(PackageStr<'_>) -> UpdateMode,
) {
let (new_imports, new_exemptions) = get_store_updates(cfg, store, mode);

// Update the store to reflect the new imports and exemptions files.
store.imports = new_imports;
store.config.exemptions = new_exemptions;
get_store_updates(cfg, store, mode).apply(store);
}

/// The non-mutating core of `update_store` for use in non-mutating situations.
pub(crate) fn get_store_updates(
cfg: &Config,
store: &Store,
mut mode: impl FnMut(PackageStr<'_>) -> UpdateMode,
) -> (ImportsFile, SortedMap<PackageName, Vec<ExemptedDependency>>) {
) -> StoreUpdates {
// Compute the set of required entries from the store for all packages in
// the dependency graph.
let graph = DepGraph::new(
Expand All @@ -2806,6 +2830,29 @@ pub(crate) fn get_store_updates(
});
}

// Remove unused non-importable audits.
let mut new_audits = store.audits.audits.clone();
for (pkg, entries) in &required_entries {
if !mode(pkg).prune_non_importable_audits {
continue;
}
let Some(entries) = entries else { continue };

if let Some(audit_entries) = new_audits.get_mut(*pkg) {
*audit_entries = std::mem::take(audit_entries)
.into_iter()
.enumerate()
.filter(|&(audit_index, ref entry)| {
// Keep the entry if it's importable (i.e. it could be used externally) or it's
// used locally.
entry.importable
|| entries.contains_key(&RequiredEntry::LocalAudit { audit_index })
})
.map(|(_, entry)| entry)
.collect();
}
}

// Dummy value to use if a package isn't found in `required_entries` - no
// edges will be required.
let no_required_entries = Some(SortedMap::new());
Expand Down Expand Up @@ -3096,5 +3143,9 @@ pub(crate) fn get_store_updates(
exemptions.sort();
}

(new_imports, all_new_exemptions)
StoreUpdates {
audits: new_audits,
imports: new_imports,
exemptions: all_new_exemptions,
}
}
Loading

0 comments on commit e78e696

Please sign in to comment.