Avoid writing config.toml and other files if no significant changes.

src/errors.rs

@@ -448,9 +448,6 @@ pub enum StoreValidateError {
     InvalidCriteria(InvalidCriteriaError),
     #[diagnostic(transparent)]
     #[error(transparent)]
-    BadFormat(BadFormatError),
-    #[diagnostic(transparent)]
-    #[error(transparent)]
     BadWildcardEndDate(BadWildcardEndDateError),
     #[error("imports.lock is out-of-date with respect to configuration")]
     #[diagnostic(help("run `cargo vet` without --locked to update imports"))]
@@ -469,13 +466,6 @@ pub struct InvalidCriteriaError {
     pub valid_names: Arc<Vec<String>>,
 }
 
-#[derive(Debug, Error, Diagnostic)]
-#[error("A file in the store is not correctly formatted:\n\n{unified_diff}")]
-#[diagnostic(help("run `cargo vet` without --locked to reformat files in the store"))]
-pub struct BadFormatError {
-    pub unified_diff: String,
-}
-
 #[derive(Debug, Error, Diagnostic)]
 #[error("'{date}' is more than a year in the future")]
 #[diagnostic(help("wildcard audits must end at most a year in the future ({max})"))]

src/format.rs

@@ -551,7 +551,7 @@ pub struct TrustEntry {
 ////////////////////////////////////////////////////////////////////////////////////
 
 /// config.toml
-#[derive(Debug, serde::Serialize, serde::Deserialize, Clone)]
+#[derive(Debug, serde::Serialize, serde::Deserialize, Clone, PartialEq, Eq)]
 pub struct ConfigFile {
     #[serde(rename = "cargo-vet")]
     #[serde(default = "CargoVetConfig::missing")]
@@ -595,7 +595,7 @@ fn is_default_criteria(val: &CriteriaName) -> bool {
 }
 
 /// The table of crate policies.
-#[derive(Debug, serde::Serialize, serde::Deserialize, Clone, Default)]
+#[derive(Debug, serde::Serialize, serde::Deserialize, Clone, Default, PartialEq, Eq)]
 #[serde(try_from = "serialization::policy::AllPolicies")]
 #[serde(into = "serialization::policy::AllPolicies")]
 pub struct Policy {
@@ -734,7 +734,7 @@ impl<'a> IntoIterator for &'a Policy {
 /// If the crate exists as a third-party crate anywhere in the dependency tree, crate versions for
 /// _all_ and _only_ the versions present in the dependency tree must be provided to set policies.
 /// Otherwise, versions may be omitted.
-#[derive(Debug, Clone)]
+#[derive(Debug, Clone, PartialEq, Eq)]
 // We have to use a slightly different serialization than than `serde(untagged)`, because toml only
 // parses `Spanned` elements (as contained in `PolicyEntry`) through their own Deseralizer, and
 // `serde(untagged)` deserializes everything into a buffer first to try different deserialization
@@ -759,7 +759,7 @@ pub enum PackagePolicyEntry {
 /// If this sounds overwhelming, don't worry, everything defaults to "nothing special"
 /// and an empty PolicyTable basically just means "everything should satisfy the
 /// default criteria in audits.toml".
-#[derive(Debug, serde::Serialize, serde::Deserialize, Clone, Default)]
+#[derive(Debug, serde::Serialize, serde::Deserialize, Clone, Default, PartialEq, Eq)]
 pub struct PolicyEntry {
     /// Whether this nominally-first-party crate should actually be subject to audits
     /// as-if it was third-party, based on matches to crates.io packages with the same
@@ -831,7 +831,7 @@ pub static DEFAULT_POLICY_CRITERIA: CriteriaStr = SAFE_TO_DEPLOY;
 pub static DEFAULT_POLICY_DEV_CRITERIA: CriteriaStr = SAFE_TO_RUN;
 
 /// A remote audits.toml that we trust the contents of (by virtue of trusting the maintainer).
-#[derive(Debug, serde::Serialize, serde::Deserialize, Clone, Default)]
+#[derive(Debug, serde::Serialize, serde::Deserialize, Clone, Default, PartialEq, Eq)]
 pub struct RemoteImport {
     /// URL(s) of the foreign audits.toml
     #[serde(with = "serialization::string_or_vec")]
@@ -965,7 +965,7 @@ impl<'de> Deserialize<'de> for StoreVersion {
 }
 
 /// Cargo vet config metadata field for the store's config file.
-#[derive(Debug, Serialize, Deserialize, Clone)]
+#[derive(Debug, Serialize, Deserialize, Clone, PartialEq, Eq)]
 pub struct CargoVetConfig {
     pub version: StoreVersion,
 }

src/storage.rs

@@ -21,13 +21,13 @@ use tracing::{error, info, log::warn, trace};
 use crate::{
     criteria::CriteriaMapper,
     errors::{
-        AggregateError, BadFormatError, BadWildcardEndDateError, CacheAcquireError,
-        CacheCommitError, CertifyError, CommandError, CrateInfoError, CriteriaChangeError,
-        CriteriaChangeErrors, DiffError, DownloadError, FetchAndDiffError,
-        FetchAuditAggregateError, FetchAuditError, FetchError, FetchRegistryError, FlockError,
-        InvalidCriteriaError, JsonParseError, LoadJsonError, LoadTomlError, SourceFile,
-        StoreAcquireError, StoreCommitError, StoreCreateError, StoreJsonError, StoreTomlError,
-        StoreValidateError, StoreValidateErrors, TomlParseError, UnpackCheckoutError, UnpackError,
+        AggregateError, BadWildcardEndDateError, CacheAcquireError, CacheCommitError, CertifyError,
+        CommandError, CrateInfoError, CriteriaChangeError, CriteriaChangeErrors, DiffError,
+        DownloadError, FetchAndDiffError, FetchAuditAggregateError, FetchAuditError, FetchError,
+        FetchRegistryError, FlockError, InvalidCriteriaError, JsonParseError, LoadJsonError,
+        LoadTomlError, SourceFile, StoreAcquireError, StoreCommitError, StoreCreateError,
+        StoreJsonError, StoreTomlError, StoreValidateError, StoreValidateErrors, TomlParseError,
+        UnpackCheckoutError, UnpackError,
     },
     flock::{FileLock, Filesystem},
     format::{
@@ -496,16 +496,43 @@ impl Store {
 
     /// Commit the store's contents back to disk
     pub fn commit(self) -> Result<(), StoreCommitError> {
+        fn has_diffs<T: for<'a> Deserialize<'a> + Eq>(
+            old_source: &SourceFile,
+            new_content: &T,
+        ) -> bool {
+            toml::from_str(old_source.source())
+                .ok()
+                .map(|old_content: T| old_content != *new_content)
+                .unwrap_or(true)
+        }
         // TODO: make this truly transactional?
         // (With a dir rename? Does that work with the lock? Fine because it's already closed?)
         if let Some(lock) = self.lock {
-            let mut audits = lock.write_audits()?;
-            let mut config = lock.write_config()?;
-            let mut imports = lock.write_imports()?;
+            let audits = if has_diffs(&self.audits_src, &self.audits) {
+                Some(lock.write_audits()?)
+            } else {
+                None
+            };
+            let config = if has_diffs(&self.config_src, &self.config) {
+                Some(lock.write_config()?)
+            } else {
+                None
+            };
+            let imports = if has_diffs(&self.imports_src, &self.imports) {
+                Some(lock.write_imports()?)
+            } else {
+                None
+            };
             let user_info = user_info_map(&self.imports);
-            audits.write_all(store_audits(self.audits, &user_info)?.as_bytes())?;
-            config.write_all(store_config(self.config)?.as_bytes())?;
-            imports.write_all(store_imports(self.imports, &user_info)?.as_bytes())?;
+            if let Some(mut audits) = audits {
+                audits.write_all(store_audits(self.audits, &user_info)?.as_bytes())?;
+            }
+            if let Some(mut config) = config {
+                config.write_all(store_config(self.config)?.as_bytes())?;
+            }
+            if let Some(mut imports) = imports {
+                imports.write_all(store_imports(self.imports, &user_info)?.as_bytes())?;
+            }
         }
         Ok(())
     }
@@ -661,46 +688,6 @@ impl Store {
             }
         }
 
-        // If requested, verify that files in the store are correctly formatted
-        // and have no unrecognized fields. We don't want to be reformatting
-        // them or dropping unused fields while in CI, as those changes will be
-        // ignored.
-        if check_file_formatting {
-            let user_info = user_info_map(&self.imports);
-            for (name, old, new) in [
-                (
-                    CONFIG_TOML,
-                    self.config_src.source(),
-                    store_config(self.config.clone())
-                        .unwrap_or_else(|_| self.config_src.source().to_owned()),
-                ),
-                (
-                    AUDITS_TOML,
-                    self.audits_src.source(),
-                    store_audits(self.audits.clone(), &user_info)
-                        .unwrap_or_else(|_| self.audits_src.source().to_owned()),
-                ),
-                (
-                    IMPORTS_LOCK,
-                    self.imports_src.source(),
-                    store_imports(self.imports.clone(), &user_info)
-                        .unwrap_or_else(|_| self.imports_src.source().to_owned()),
-                ),
-            ] {
-                if old.trim_end() != new.trim_end() {
-                    errors.push(StoreValidateError::BadFormat(BadFormatError {
-                        unified_diff: unified_diff(
-                            Algorithm::Myers,
-                            old,
-                            &new,
-                            3,
-                            Some((&format!("old/{name}"), &format!("new/{name}"))),
-                        ),
-                    }));
-                }
-            }
-        }
-
         // If we're locked, and therefore not fetching new live imports,
         // validate that our imports.lock is in sync with config.toml.
         if check_file_formatting && self.imports_lock_outdated() {

src/tests/snapshots/cargo_vet__tests__store_parsing__invalid_formatting.snap

@@ -1,50 +1,5 @@
 ---
 source: src/tests/store_parsing.rs
-expression: acquire_errors
+expression: no_errors
 ---
-  × Your cargo-vet store (supply-chain) has consistency errors
-
-Error:   × A file in the store is not correctly formatted:
-  │ 
-  │ --- old/config.toml
-  │ +++ new/config.toml
-  │ @@ -11,14 +11,14 @@
-  │  [imports.peer2]
-  │  url = "https://peer1.com"
-  │ 
-  │ -[[exemptions.zzz]]
-  │ +[[exemptions.aaa]]
-  │  version = "1.0.0"
-  │  criteria = "safe-to-deploy"
-  │ 
-  │  [[exemptions.bbb]]
-  │ +version = "1.0.0"
-  │  criteria = "safe-to-deploy"
-  │ -version = "1.0.0"
-  │ 
-  │ -[[exemptions.aaa]]
-  │ +[[exemptions.zzz]]
-  │  version = "1.0.0"
-  │  criteria = "safe-to-deploy"
-  │ 
-  help: run `cargo vet` without --locked to reformat files in the store
-Error:   × A file in the store is not correctly formatted:
-  │ 
-  │ --- old/audits.toml
-  │ +++ new/audits.toml
-  │ @@ -7,9 +7,9 @@
-  │ 
-  │  [[audits.serde]]
-  │  criteria = ["safe-to-deploy", "good"]
-  │ -version = "2.0.0"
-  │ +version = "1.0.0"
-  │ +notes = "valid field"
-  │ 
-  │  [[audits.serde]]
-  │  criteria = ["safe-to-deploy", "good"]
-  │ -version = "1.0.0"
-  │ -notes = "valid field"
-  │ +version = "2.0.0"
-  │ 
-  help: run `cargo vet` without --locked to reformat files in the store
 

src/tests/snapshots/cargo_vet__tests__store_parsing__unknown_field_config.snap

@@ -1,23 +1,5 @@
 ---
 source: src/tests/store_parsing.rs
-expression: acquire_errors
+expression: no_errors
 ---
-  × Your cargo-vet store (supply-chain) has consistency errors
-
-Error:   × A file in the store is not correctly formatted:
-  │ 
-  │ --- old/config.toml
-  │ +++ new/config.toml
-  │ @@ -7,9 +7,7 @@
-  │  [imports.peer1]
-  │  url = "https://peer1.com"
-  │  exclude = ["zzz", "aaa"]
-  │ -unknown-field = "hi"
-  │ 
-  │  [[exemptions.zzz]]
-  │  version = "1.0.0"
-  │  criteria = "safe-to-deploy"
-  │ -unknown-field = "hi"
-  │ 
-  help: run `cargo vet` without --locked to reformat files in the store
 

src/tests/store_parsing.rs

@@ -269,8 +269,8 @@ criteria = "safe-to-deploy"
 version = "10.0.0"
 "#;
 
-    let acquire_errors = get_valid_store(config, EMPTY_AUDITS, imports);
-    insta::assert_snapshot!(acquire_errors);
+    let no_errors = get_valid_store(config, EMPTY_AUDITS, imports);
+    assert_eq!("", no_errors);
 }
 
 #[test]
@@ -407,8 +407,8 @@ criteria = "safe-to-deploy"
 version = "10.0.0"
 "#;
 
-    let acquire_errors = get_valid_store(config, audits, imports);
-    insta::assert_snapshot!(acquire_errors);
+    let no_errors = get_valid_store(config, audits, imports);
+    insta::assert_snapshot!(no_errors);
 }
 
 #[test]