diff --git a/src/errors.rs b/src/errors.rs index ae5f366c..464d9cbc 100644 --- a/src/errors.rs +++ b/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>, } -#[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})"))] diff --git a/src/format.rs b/src/format.rs index 40ccb34a..d6721424 100644 --- a/src/format.rs +++ b/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, } diff --git a/src/storage.rs b/src/storage.rs index ed407ea9..649e0cca 100644 --- a/src/storage.rs +++ b/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 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() {