Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update imports.lock less frequently #296

Merged
merged 2 commits into from
Aug 22, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -57,3 +57,4 @@ features = [

[dev-dependencies]
insta = "1.15.0"
similar = "2.1.0"
65 changes: 39 additions & 26 deletions src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand All @@ -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
////////////////////////////////////////////////////////////
Expand Down Expand Up @@ -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,
}

//////////////////////////////////////////////////////////
Expand Down
8 changes: 8 additions & 0 deletions src/format.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
106 changes: 51 additions & 55 deletions src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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(())
}
Expand Down Expand Up @@ -707,7 +711,7 @@ fn do_cmd_certify(

let criteria_mapper = CriteriaMapper::new(
&store.audits.criteria,
&store.imports,
store.imported_audits(),
&store.config.imports,
);

Expand Down Expand Up @@ -914,6 +918,7 @@ fn do_cmd_certify(
.collect(),
who,
notes,
is_fresh_import: false,
};

store
Expand Down Expand Up @@ -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(),
Expand Down Expand Up @@ -1134,6 +1139,7 @@ fn cmd_record_violation(
criteria,
who,
notes,
is_fresh_import: false,
};

store
Expand All @@ -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?
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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(())
}

Expand All @@ -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)?;

Expand Down Expand Up @@ -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())?;

Expand Down Expand Up @@ -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(())
}

Expand All @@ -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 {
Expand Down Expand Up @@ -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)?;
}
Expand Down Expand Up @@ -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()?;
}

Expand All @@ -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(())
}
Expand All @@ -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(())
}
Expand Down
Loading