Update imports.lock less frequently

This patch reduces the frequency that imports.lock updates by doing a
few things:

 * Imports are only updated when necessary for the vet to pass. If vet
   would successfully pass without the new audits, only changes to
   notes, criteria descriptions, revoked audits, and violations will be
   pulled.

 * Some commands, such as suggest, will always run against the live
   copy when not --locked, providing the most accurate results possible.

 * Only audits for crates in your crate graph will be imported.

 * When an update is required, all new audits for the given crate will
   be fetched and locked, to batch updates as much as possible.

This required doing some changes to how imports are handled under the
hood, as well as a number of changes to the resolver in order to track
whether fresh imports are required with sufficient accuracy. Imports are
also now fetched in more APIs in order to have the most up-to-date
information.

The changes to the resolver involved piping through "caveats" which are
determined when solving paths. This is an expansion on the existing
"needs_exemption"/"fully_audited" flags which were being used to
generate stats after a successful vet to also handle freshly imported
audits which should only be used when necessary.

Special handling was also needed to make sure that audits for unused
criteria are not counted by tracking caveats on a per-criteria basis and
bubbling them out.

src/errors.rs

@@ -185,6 +185,12 @@ pub enum StoreAcquireError {
     #[error(transparent)]
     #[diagnostic(transparent)]
     Validate(#[from] StoreValidateErrors),
+    #[error(transparent)]
+    #[diagnostic(transparent)]
+    FetchAuditError(#[from] FetchAuditError),
+    #[diagnostic(transparent)]
+    #[error(transparent)]
+    CriteriaChange(#[from] CriteriaChangeErrors),
 }
 
 #[derive(Debug, Error, Diagnostic)]
@@ -203,6 +209,26 @@ pub enum StoreCommitError {
     ),
 }
 
+#[derive(Debug, Error, Diagnostic)]
+#[error("Some of your imported audits changed their criteria descriptions")]
+#[diagnostic()]
+pub struct CriteriaChangeErrors {
+    #[related]
+    pub errors: Vec<CriteriaChangeError>,
+}
+#[derive(Debug, Error, Diagnostic)]
+// FIXME: it would be rad if this was a diff!
+#[error(
+    "{import_name}'s '{criteria_name}' criteria changed from\n\n{old_desc}\n\nto\n\n{new_desc}\n"
+)]
+#[diagnostic(help("Run `cargo vet regenerate imports` to accept this new definition"))]
+pub struct CriteriaChangeError {
+    pub import_name: ImportName,
+    pub criteria_name: ForeignCriteriaName,
+    pub old_desc: String,
+    pub new_desc: String,
+}
+
 ////////////////////////////////////////////////////////////
 // StoreValidateErrors
 ////////////////////////////////////////////////////////////
@@ -436,38 +462,25 @@ pub enum FetchAuditError {
         #[source]
         error: url::ParseError,
     },
+    #[error("{import_name}'s '{criteria_name}' criteria is missing a description")]
+    MissingCriteriaDescription {
+        import_name: ImportName,
+        criteria_name: ForeignCriteriaName,
+    },
+    #[error("{import_name}'s '{criteria_name}' criteria description URI is invalid: '{url}'")]
+    InvalidCriteriaDescriptionUrl {
+        import_name: ImportName,
+        criteria_name: ForeignCriteriaName,
+        url: String,
+        #[source]
+        error: url::ParseError,
+    },
     #[diagnostic(transparent)]
     #[error(transparent)]
     Download(#[from] DownloadError),
     #[diagnostic(transparent)]
     #[error(transparent)]
     Toml(#[from] LoadTomlError),
-    #[diagnostic(transparent)]
-    #[error(transparent)]
-    Validate(#[from] StoreValidateErrors),
-    #[diagnostic(transparent)]
-    #[error(transparent)]
-    CriteriaChange(#[from] CriteriaChangeErrors),
-}
-
-#[derive(Debug, Error, Diagnostic)]
-#[error("Some of your imported audits changed their criteria descriptions")]
-#[diagnostic()]
-pub struct CriteriaChangeErrors {
-    #[related]
-    pub errors: Vec<CriteriaChangeError>,
-}
-#[derive(Debug, Error, Diagnostic)]
-// FIXME: it would be rad if this was a diff!
-#[error(
-    "{import_name}'s '{criteria_name}' criteria changed from\n\n{old_desc}\n\nto\n\n{new_desc}\n"
-)]
-#[diagnostic(help("Run `cargo vet regenerate imports` to accept this new definition"))]
-pub struct CriteriaChangeError {
-    pub import_name: ImportName,
-    pub criteria_name: ForeignCriteriaName,
-    pub old_desc: String,
-    pub new_desc: String,
 }
 
 //////////////////////////////////////////////////////////

src/format.rs

@@ -162,6 +162,14 @@ pub struct AuditEntry {
     pub criteria: Vec<Spanned<CriteriaName>>,
     pub kind: AuditKind,
     pub notes: Option<String>,
+    /// A non-serialized member which indicates whether this audit is a "fresh"
+    /// audit. This will be set for all audits imported found in the remote
+    /// audits file which aren't also found in the local `imports.lock` cache.
+    ///
+    /// This should almost always be `false`, and only set to `true` by the
+    /// import handling code.
+    #[serde(skip)]
+    pub is_fresh_import: bool,
 }
 
 /// Implement PartialOrd manually because the order we want for sorting is

src/main.rs

@@ -524,9 +524,9 @@ fn cmd_inspect(
     let package = &*sub_args.package;
 
     let fetched = {
-        let store = Store::acquire(cfg)?;
-        let cache = Cache::acquire(cfg)?;
         let network = Network::acquire(cfg);
+        let store = Store::acquire(cfg, network.as_ref(), false)?;
+        let cache = Cache::acquire(cfg)?;
 
         // Record this command for magic in `vet certify`
         cache.set_last_fetch(FetchCommand::Inspect {
@@ -607,14 +607,18 @@ fn cmd_certify(
     sub_args: &CertifyArgs,
 ) -> Result<(), miette::Report> {
     // Certify that you have reviewed a crate's source for some version / delta
-    let mut store = Store::acquire(cfg)?;
     let network = Network::acquire(cfg);
+    let mut store = Store::acquire(cfg, network.as_ref(), false)?;
 
     // Grab the last fetch and immediately drop the cache
     let last_fetch = Cache::acquire(cfg)?.get_last_fetch();
 
     do_cmd_certify(out, cfg, sub_args, &mut store, network.as_ref(), last_fetch)?;
 
+    // Re-run the resolver after adding the new `certify`. This will be used to
+    // potentially update imports.
+    store.maybe_update_imports_file(cfg, false);
+
     store.commit()?;
     Ok(())
 }
@@ -707,7 +711,7 @@ fn do_cmd_certify(
 
     let criteria_mapper = CriteriaMapper::new(
         &store.audits.criteria,
-        &store.imports,
+        store.imported_audits(),
         &store.config.imports,
     );
 
@@ -914,6 +918,7 @@ fn do_cmd_certify(
             .collect(),
         who,
         notes,
+        is_fresh_import: false,
     };
 
     store
@@ -1087,7 +1092,7 @@ fn cmd_record_violation(
     sub_args: &RecordViolationArgs,
 ) -> Result<(), miette::Report> {
     // Mark a package as a violation
-    let mut store = Store::acquire(cfg)?;
+    let mut store = Store::acquire_offline(cfg)?;
 
     let kind = AuditKind::Violation {
         violation: sub_args.versions.clone(),
@@ -1134,6 +1139,7 @@ fn cmd_record_violation(
         criteria,
         who,
         notes,
+        is_fresh_import: false,
     };
 
     store
@@ -1156,7 +1162,7 @@ fn cmd_add_exemption(
     sub_args: &AddExemptionArgs,
 ) -> Result<(), miette::Report> {
     // Add an exemption entry
-    let mut store = Store::acquire(cfg)?;
+    let mut store = Store::acquire_offline(cfg)?;
 
     let dependency_criteria = if sub_args.dependency_criteria.is_empty() {
         // TODO: look at the current audits to infer this? prompt?
@@ -1229,8 +1235,8 @@ fn cmd_suggest(
 ) -> Result<(), miette::Report> {
     // Run the checker to validate that the current set of deps is covered by the current cargo vet store
     trace!("suggesting...");
-    let suggest_store = Store::acquire(cfg)?.clone_for_suggest();
     let network = Network::acquire(cfg);
+    let suggest_store = Store::acquire(cfg, network.as_ref(), false)?.clone_for_suggest();
 
     // DO THE THING!!!!
     let report = resolver::resolve(
@@ -1261,28 +1267,21 @@ fn cmd_regenerate_imports(
 ) -> Result<(), miette::Report> {
     trace!("regenerating imports...");
 
-    let mut store = Store::acquire(cfg)?;
-    let network = Network::acquire(cfg);
-
-    if let Some(network) = &network {
-        if !cfg.cli.locked {
-            // Literally the only difference between this command and fetch-imports
-            // is that we pass `accept_changes = true`
-            tokio::runtime::Handle::current()
-                .block_on(store.fetch_foreign_audits(network, true))?;
-            store.commit()?;
-            return Ok(());
-        }
+    if cfg.cli.locked {
+        // ERRORS: just a warning that you're holding it wrong, unclear if immediate or buffered,
+        // or if this should be a hard error, or if we should ignore the --locked flag and
+        // just do it anyway
+        writeln!(
+            out,
+            "warning: ran `regenerate imports` with --locked, this won't do anything!"
+        );
+        return Ok(());
     }
 
-    // ERRORS: just a warning that you're holding it wrong, unclear if immediate or buffered,
-    // or if this should be a hard error, or if we should ignore the --locked flag and
-    // just do it anyway
-    writeln!(
-        out,
-        "warning: ran `regenerate imports` with --locked, this won't do anything!"
-    );
-
+    let network = Network::acquire(cfg);
+    let mut store = Store::acquire(cfg, network.as_ref(), true)?;
+    store.maybe_update_imports_file(cfg, true);
+    store.commit()?;
     Ok(())
 }
 
@@ -1292,7 +1291,7 @@ fn cmd_regenerate_audit_as(
     _sub_args: &RegenerateAuditAsCratesIoArgs,
 ) -> Result<(), miette::Report> {
     trace!("regenerating audit-as-crates-io...");
-    let mut store = Store::acquire(cfg)?;
+    let mut store = Store::acquire_offline(cfg)?;
 
     fix_audit_as(cfg, &mut store)?;
 
@@ -1347,8 +1346,8 @@ fn cmd_regenerate_exemptions(
     _sub_args: &RegenerateExemptionsArgs,
 ) -> Result<(), miette::Report> {
     trace!("regenerating exemptions...");
-    let mut store = Store::acquire(cfg)?;
     let network = Network::acquire(cfg);
+    let mut store = Store::acquire(cfg, network.as_ref(), false)?;
 
     minimize_exemptions(cfg, &mut store, network.as_ref())?;
 
@@ -1457,6 +1456,9 @@ pub fn minimize_exemptions(
     // Alright there's the new exemptions
     store.config.exemptions = new_exemptions;
 
+    // Re-vet and ensure that imports are updated after changing exemptions.
+    store.maybe_update_imports_file(cfg, false);
+
     Ok(())
 }
 
@@ -1466,9 +1468,9 @@ fn cmd_diff(out: &Arc<dyn Out>, cfg: &Config, sub_args: &DiffArgs) -> Result<(),
     let package = &*sub_args.package;
 
     let (fetched1, fetched2) = {
-        let store = Store::acquire(cfg)?;
-        let cache = Cache::acquire(cfg)?;
         let network = Network::acquire(cfg);
+        let store = Store::acquire(cfg, network.as_ref(), false)?;
+        let cache = Cache::acquire(cfg)?;
 
         // Record this command for magic in `vet certify`
         cache.set_last_fetch(FetchCommand::Diff {
@@ -1552,16 +1554,10 @@ fn cmd_check(out: &Arc<dyn Out>, cfg: &Config, sub_args: &CheckArgs) -> Result<(
     // Run the checker to validate that the current set of deps is covered by the current cargo vet store
     trace!("vetting...");
 
-    let mut store = Store::acquire(cfg)?;
     let network = Network::acquire(cfg);
+    let mut store = Store::acquire(cfg, network.as_ref(), false)?;
 
     if !cfg.cli.locked {
-        // Try to update the foreign audits (imports)
-        if let Some(network) = &network {
-            tokio::runtime::Handle::current()
-                .block_on(store.fetch_foreign_audits(network, false))?;
-        }
-
         // Check if any of our first-parties are in the crates.io registry
         check_audit_as_crates_io(cfg, &store)?;
     }
@@ -1598,6 +1594,7 @@ fn cmd_check(out: &Arc<dyn Out>, cfg: &Config, sub_args: &CheckArgs) -> Result<(
         // Err(eyre!("report contains errors"))?;
         panic_any(ExitPanic(-1));
     } else {
+        store.imports = store.get_updated_imports_file(&report, false);
         store.commit()?;
     }
 
@@ -1611,25 +1608,23 @@ fn cmd_fetch_imports(
 ) -> Result<(), miette::Report> {
     trace!("fetching imports...");
 
-    let mut store = Store::acquire(cfg)?;
-    let network = Network::acquire(cfg);
+    if cfg.cli.locked {
+        // ERRORS: just a warning that you're holding it wrong, unclear if immediate or buffered,
+        // or if this should be a hard error, or if we should ignore the --locked flag and
+        // just do it anyway
+        writeln!(
+            out,
+            "warning: ran fetch-imports with --locked, this won't do anything!"
+        );
 
-    if let Some(network) = &network {
-        if !cfg.cli.locked {
-            tokio::runtime::Handle::current()
-                .block_on(store.fetch_foreign_audits(network, false))?;
-            store.commit()?;
-            return Ok(());
-        }
+        return Ok(());
     }
 
-    // ERRORS: just a warning that you're holding it wrong, unclear if immediate or buffered,
-    // or if this should be a hard error, or if we should ignore the --locked flag and
-    // just do it anyway
-    writeln!(
-        out,
-        "warning: ran fetch-imports with --locked, this won't do anything!"
-    );
+    let network = Network::acquire(cfg);
+    let mut store = Store::acquire(cfg, network.as_ref(), false)?;
+
+    store.maybe_update_imports_file(cfg, true);
+    store.commit()?;
 
     Ok(())
 }
@@ -1656,7 +1651,8 @@ fn cmd_dump_graph(
 fn cmd_fmt(_out: &Arc<dyn Out>, cfg: &Config, _sub_args: &FmtArgs) -> Result<(), miette::Report> {
     // Reformat all the files (just load and store them, formatting is implicit).
     trace!("formatting...");
-    let store = Store::acquire(cfg)?;
+    // We don't need to fetch foreign audits to format files
+    let store = Store::acquire_offline(cfg)?;
     store.commit()?;
     Ok(())
 }