Repository: mozilla/cargo-vet (language: python)

Issue:
Consider pruning publisher entries even if `prune_imports` is disabled when a new publisher entry is being added for a crate
In order to avoid unnecessary changes to files such as `imports.lock` cargo-vet makes some effort to avoid removing entries from the file unless requested. The main way this happens in normal operation is when `cargo vet certify` is used to add a new audit for a crate, which can lead to other exemptions, imports, etc. for the crate to be removed.

Unfortunately this pattern doesn't work super well for crates with wildcard audits or trusted entries. For these crates, generally adding a dependency does not require the user running any command other than `cargo vet`, which does not prune unused entries, which means that the previous `publisher` entries are left behind and not cleaned up.

We could consider automatically pruning `publisher` entries for crates even when `prune_imports` is disabled so long as a new publisher entry is being added for that crate. This still avoids unnecessary updates, but means we can automatically clean up publisher entries when new entries are being added anyway.

We could also consider doing this for other forms of imports such as normal and wildcard audits, though removing those from `imports.lock` due to them becoming unnecessary is less frequent during normal dependency updates.


Discussion / hints:

Fix patch:
diff --git a/src/resolver.rs b/src/resolver.rs
index 1c79cb57..daf0ba1f 100644
--- a/src/resolver.rs
+++ b/src/resolver.rs
@@ -2858,6 +2858,39 @@ pub fn update_store(
     get_store_updates(cfg, store, mode).apply(store);
 }
 
+/// Helper function to determine if we should be pruning imports.
+///
+/// We always prune if requested, but will also prune imports if a new audit or
+/// publisher entry is being added for the crate.
+fn should_prune_imports(
+    store: &Store,
+    required_entries: &Option<SortedMap<RequiredEntry, CriteriaSet>>,
+    mode: UpdateMode,
+    pkgname: PackageStr<'_>,
+) -> bool {
+    if mode.prune_imports {
+        true
+    } else if let Some(required_entries) = required_entries {
+        let import = |idx| store.imported_audits().values().nth(idx).unwrap();
+        required_entries.keys().any(|entry| match entry {
+            RequiredEntry::Audit {
+                import_index,
+                audit_index,
+            } => import(*import_index).audits[pkgname][*audit_index].is_fresh_import,
+            RequiredEntry::WildcardAudit {
+                import_index,
+                audit_index,
+            } => import(*import_index).wildcard_audits[pkgname][*audit_index].is_fresh_import,
+            RequiredEntry::Publisher { publisher_index } => {
+                store.publishers()[pkgname][*publisher_index].is_fresh_import
+            }
+            _ => false,
+        })
+    } else {
+        false
+    }
+}
+
 /// The non-mutating core of `update_store` for use in non-mutating situations.
 pub(crate) fn get_store_updates(
     cfg: &Config,
@@ -2932,10 +2965,11 @@ pub(crate) fn get_store_updates(
                 .wildcard_audits
                 .iter()
                 .map(|(pkgname, wildcard_audits)| {
-                    let prune_imports = mode(&pkgname[..]).prune_imports;
                     let required_entries = required_entries
                         .get(&pkgname[..])
                         .unwrap_or(&no_required_entries);
+                    let prune_imports =
+                        should_prune_imports(store, required_entries, mode(&pkgname[..]), pkgname);
                     (
                         pkgname,
                         wildcard_audits
@@ -2974,12 +3008,13 @@ pub(crate) fn get_store_updates(
                 .audits
                 .iter()
                 .map(|(pkgname, audits)| {
-                    let prune_imports = mode(&pkgname[..]).prune_imports;
                     let (uses_package, required_entries) = match required_entries.get(&pkgname[..])
                     {
                         Some(e) => (true, e),
                         None => (false, &no_required_entries),
                     };
+                    let prune_imports =
+                        should_prune_imports(store, required_entries, mode(&pkgname[..]), pkgname);
                     (
                         pkgname,
                         audits
@@ -3029,10 +3064,11 @@ pub(crate) fn get_store_updates(
 
     // Determine which live publisher information to keep in the imports.lock file.
     for (pkgname, publishers) in store.publishers() {
-        let prune_imports = mode(&pkgname[..]).prune_imports;
         let required_entries = required_entries
             .get(&pkgname[..])
             .unwrap_or(&no_required_entries);
+        let prune_imports =
+            should_prune_imports(store, required_entries, mode(&pkgname[..]), pkgname);
         let mut publishers: Vec<_> = publishers
             .iter()
             .enumerate()


Test patch:
diff --git a/src/tests/snapshots/cargo_vet__tests__unpublished__audit_as_crates_io_unpublished_wildcard_nounpublished_prefer_exemptions.snap b/src/tests/snapshots/cargo_vet__tests__unpublished__audit_as_crates_io_unpublished_wildcard_nounpublished_prefer_exemptions.snap
index 145db011..d5ad564a 100644
--- a/src/tests/snapshots/cargo_vet__tests__unpublished__audit_as_crates_io_unpublished_wildcard_nounpublished_prefer_exemptions.snap
+++ b/src/tests/snapshots/cargo_vet__tests__unpublished__audit_as_crates_io_unpublished_wildcard_nounpublished_prefer_exemptions.snap
@@ -13,17 +13,11 @@ imports.lock:
 +audited_as = "9.0.0"
 +
  [[publisher.descriptive]]
- version = "8.0.0"
+-version = "8.0.0"
++version = "9.0.0"
  when = "2022-12-15"
  user-id = 1
  user-login = "user1"
  user-name = "User One"
-+
-+[[publisher.descriptive]]
-+version = "9.0.0"
-+when = "2022-12-15"
-+user-id = 1
-+user-login = "user1"
-+user-name = "User One"