Repository: mozilla/cargo-vet
Pull request #633 (2024-09-18 19:20:27)

Remember user --mode choices, and support diff.rs for inspect
This change adds support to remember user's --mode choices, so that they do not need to be used every time. In addition, it adds a basic support for diff.rs to inspect (by loading a diff from a version to itself) such that this mode memory can be shared between diff and inspect.

If xfbs/diff.rs#24 is merged, we should change the diff.rs inspect behaviour to use the new browse endpoints instead.

Finally, this patch changes the default mode from sourcegraph to diff.rs. As noted in #611, sourcegraph has been quite unreliable lately, and diff.rs is likely to provide a better experience until that is resolved.


Patch:
diff --git a/src/cli.rs b/src/cli.rs
index 83ea068..a12b3fe 100644
--- a/src/cli.rs
+++ b/src/cli.rs
@@ -1,6 +1,7 @@
 use std::{path::PathBuf, str::FromStr};
 
 use clap::{Parser, Subcommand, ValueEnum};
+use serde::{Deserialize, Serialize};
 use tracing::level_filters::LevelFilter;
 
 use crate::format::{CriteriaName, ImportName, PackageName, VersionReq, VetVersion};
@@ -455,7 +456,7 @@ pub struct CheckArgs {}
 #[derive(clap::Args)]
 pub struct InitArgs {}
 
-/// Fetches the crate to a temp location and pushd's to it
+/// Inspect a crate at a specific version
 #[derive(clap::Args)]
 pub struct InspectArgs {
     /// The package to inspect
@@ -465,11 +466,16 @@ pub struct InspectArgs {
     #[clap(action)]
     pub version: VetVersion,
     /// How to inspect the source
-    #[clap(long, action, default_value = "sourcegraph")]
-    pub mode: InspectFetchMode,
+    ///
+    /// Defaults to the most recently used --mode argument, or diff.rs if no
+    /// mode argument has been used.
+    ///
+    /// This option is ignored if a git version is passed.
+    #[clap(long, action)]
+    pub mode: Option<FetchMode>,
 }
 
-/// Emits a diff of the two versions
+/// View a diff between two versions of the given crate
 #[derive(clap::Args)]
 pub struct DiffArgs {
     /// The package to diff
@@ -481,9 +487,14 @@ pub struct DiffArgs {
     /// The target version to diff
     #[clap(action)]
     pub version2: VetVersion,
-    /// How to inspect the source
-    #[clap(long, action, default_value = "sourcegraph")]
-    pub mode: DiffFetchMode,
+    /// How to inspect the diff
+    ///
+    /// Defaults to the most recently used --mode argument, or diff.rs if no
+    /// mode argument has been used.
+    ///
+    /// This option is ignored if a git version is passed.
+    #[clap(long, action)]
+    pub mode: Option<FetchMode>,
 }
 
 /// Certifies a package as audited
@@ -772,14 +783,8 @@ pub enum Verbose {
     Trace,
 }
 
-#[derive(Copy, Clone, PartialEq, Eq, PartialOrd, Ord, ValueEnum)]
-pub enum InspectFetchMode {
-    Local,
-    Sourcegraph,
-}
-
-#[derive(Copy, Clone, PartialEq, Eq, PartialOrd, Ord, ValueEnum)]
-pub enum DiffFetchMode {
+#[derive(Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Debug, ValueEnum, Serialize, Deserialize)]
+pub enum FetchMode {
     Local,
     Sourcegraph,
     #[clap(name = "diff.rs")]
diff --git a/src/format.rs b/src/format.rs
index f28dbb6..5b0f3bb 100644
--- a/src/format.rs
+++ b/src/format.rs
@@ -1,5 +1,6 @@
 //! Details of the file formats used by cargo vet
 
+use crate::cli::FetchMode;
 use crate::errors::{StoreVersionParseError, VersionParseError};
 use crate::resolver::{DiffRecommendation, ViolationConflict};
 use crate::serialization::{spanned::Spanned, Tidyable};
@@ -1182,6 +1183,7 @@ impl FetchCommand {
 pub struct CommandHistory {
     #[serde(flatten)]
     pub last_fetch: Option<FetchCommand>,
+    pub last_fetch_mode: Option<FetchMode>,
 }
 
 ////////////////////////////////////////////////////////////////////////////////////
diff --git a/src/main.rs b/src/main.rs
index ffcf114..f793b16 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -570,8 +570,20 @@ fn cmd_inspect(
             version: version.clone(),
         });
 
-        if sub_args.mode == InspectFetchMode::Sourcegraph && version.git_rev.is_none() {
-            let url = format!("https://sourcegraph.com/crates/{package}@v{version}");
+        // Determine the fetch mode to use. We'll need to do a local diff if the
+        // selected version has a git revision.
+        let mode = cache.select_fetch_mode(sub_args.mode, version.git_rev.is_some());
+
+        if mode != FetchMode::Local {
+            let url = match mode {
+                FetchMode::Sourcegraph => {
+                    format!("https://sourcegraph.com/crates/{package}@v{version}")
+                }
+                FetchMode::DiffRs => {
+                    format!("https://diff.rs/{package}/{version}/{version}/")
+                }
+                FetchMode::Local => unreachable!(),
+            };
             tokio::runtime::Handle::current()
                 .block_on(prompt_criteria_eulas(
                     out,
@@ -2050,20 +2062,24 @@ fn cmd_diff(out: &Arc<dyn Out>, cfg: &Config, sub_args: &DiffArgs) -> Result<(),
             version2: version2.clone(),
         });
 
-        if sub_args.mode != DiffFetchMode::Local
-            && version1.git_rev.is_none()
-            && version2.git_rev.is_none()
-        {
-            let url = match sub_args.mode {
-                DiffFetchMode::Sourcegraph => {
+        // Determine the fetch mode to use. We'll need to do a local diff if the
+        // selected version has a git revision.
+        let mode = cache.select_fetch_mode(
+            sub_args.mode,
+            version1.git_rev.is_some() || version2.git_rev.is_some(),
+        );
+
+        if mode != FetchMode::Local {
+            let url = match mode {
+                FetchMode::Sourcegraph => {
                     format!(
                         "https://sourcegraph.com/crates/{package}/-/compare/v{version1}...v{version2}?visible=7000"
                     )
                 }
-                DiffFetchMode::DiffRs => {
+                FetchMode::DiffRs => {
                     format!("https://diff.rs/{package}/{version1}/{version2}/")
                 }
-                DiffFetchMode::Local => unreachable!(),
+                FetchMode::Local => unreachable!(),
             };
             tokio::runtime::Handle::current()
                 .block_on(prompt_criteria_eulas(
diff --git a/src/storage.rs b/src/storage.rs
index 775e33e..84723af 100644
--- a/src/storage.rs
+++ b/src/storage.rs
@@ -19,6 +19,7 @@ use tar::Archive;
 use tracing::{error, info, log::warn, trace};
 
 use crate::{
+    cli::FetchMode,
     criteria::CriteriaMapper,
     errors::{
         AggregateError, BadFormatError, BadWildcardEndDateError, CacheAcquireError,
@@ -2291,6 +2292,35 @@ impl Cache {
         guard.command_history.last_fetch = Some(last_fetch);
     }
 
+    pub fn select_fetch_mode(
+        &self,
+        chosen_mode: Option<FetchMode>,
+        force_local: bool,
+    ) -> FetchMode {
+        // If we're going to be forced to use a local mode, return it.
+        //
+        // We don't update the cache here so that folks used to manually typing
+        // out `--local` when doing git diffs don't clobber their last fetch
+        // mode settings.
+        if force_local {
+            return FetchMode::Local;
+        }
+
+        // If an explicit mode was selected on the command line, update the last
+        // fetch mode to reflect it.
+        let mut guard = self.state.lock().unwrap();
+        if let Some(mode) = chosen_mode {
+            guard.command_history.last_fetch_mode = Some(mode);
+        }
+
+        // Return either the most-recently selected fetch mode, or `diff.rs` if
+        // no fetch mode has been explicitly selected.
+        guard
+            .command_history
+            .last_fetch_mode
+            .unwrap_or(FetchMode::DiffRs)
+    }
+
     /// For a given package, fetch the list of versions published on crates.io,
     /// along with the corresponding index entry.
     ///


Test patch:
diff --git a/tests/snapshots/test_cli__markdown-help.snap b/tests/snapshots/test_cli__markdown-help.snap
index 8e4dfeb..f3465fb 100644
--- a/tests/snapshots/test_cli__markdown-help.snap
+++ b/tests/snapshots/test_cli__markdown-help.snap
@@ -277,8 +277,12 @@ The version to inspect
 #### `--mode <MODE>`
 How to inspect the source
 
-\[default: sourcegraph]  
-\[possible values: local, sourcegraph]  
+Defaults to the most recently used --mode argument, or diff.rs if no mode argument has
+been used.
+
+This option is ignored if a git version is passed.
+
+\[possible values: local, sourcegraph, diff.rs]  
 
 #### `-h, --help`
 Print help information
@@ -310,9 +314,13 @@ The target version to diff
 
 ### OPTIONS
 #### `--mode <MODE>`
-How to inspect the source
+How to inspect the diff
+
+Defaults to the most recently used --mode argument, or diff.rs if no mode argument has
+been used.
+
+This option is ignored if a git version is passed.
 
-\[default: sourcegraph]  
 \[possible values: local, sourcegraph, diff.rs]  
 
 #### `-h, --help`