Prelease tidy and add is_ci to Diagnostics (#271)

* Some pre-release tidy and add is_ci to diagnostics * Bump locks * Fixup cargo-audit raised issues * Remove a spare 'I' which we were saving for later * rename a field * failure_variant, not error_variant
DeterminateSystems · Feb 24, 2023 · db329ea · db329ea
1 parent 59abd1b
commit db329ea
Show file tree

Hide file tree

Showing 11 changed files with 159 additions and 169 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/Cargo.toml b/Cargo.toml
@@ -17,7 +17,7 @@ build-inputs = ["darwin.apple_sdk.frameworks.Security"]
 [features]
 default = ["cli", "diagnostics"]
 cli = ["eyre", "color-eyre", "clap", "tracing-subscriber", "tracing-error", "atty"]
-diagnostics = ["os-release"]
+diagnostics = ["os-release", "is_ci"]
 
 [[bin]]
 name = "nix-installer"
@@ -55,11 +55,12 @@ semver = { version = "1.0.14", default-features = false, features = ["serde", "s
 term = { version = "0.7.0", default-features = false }
 uuid = { version = "1.2.2", features = ["serde"] }
 os-release = { version = "0.1.0", default-features = false, optional = true }
+is_ci = { version = "1.1.1", default-features = false, optional = true }
 strum = { version = "0.24.1", features = ["derive"] }
 
 [dev-dependencies]
 eyre = { version = "0.6.8", default-features = false, features = [ "track-caller" ] }
-tempdir = "0.3.7"
+tempfile = "3.3.0"
 
 [profile.release]
 strip = true  # Automatically strip symbols from the binary.

diff --git a/flake.lock b/flake.lock
diff --git a/flake.nix b/flake.nix
@@ -133,6 +133,7 @@
               toolchain
               rust-analyzer
               cargo-outdated
+              cargo-audit
               nixpkgs-fmt
               check.check-rustfmt
               check.check-spelling

diff --git a/src/action/base/create_directory.rs b/src/action/base/create_directory.rs
@@ -222,7 +222,7 @@ mod test {
 
     #[tokio::test]
     async fn creates_and_deletes_empty_directory() -> eyre::Result<()> {
-        let temp_dir = tempdir::TempDir::new("nix_installer_tests_create_file")?;
+        let temp_dir = tempfile::tempdir()?;
         let test_dir = temp_dir.path().join("creates_and_deletes_empty_directory");
         let mut action = CreateDirectory::plan(test_dir.clone(), None, None, None, false).await?;
 
@@ -237,7 +237,7 @@ mod test {
 
     #[tokio::test]
     async fn creates_and_deletes_populated_directory_if_prune_true() -> eyre::Result<()> {
-        let temp_dir = tempdir::TempDir::new("nix_installer_tests_create_file")?;
+        let temp_dir = tempfile::tempdir()?;
         let test_dir = temp_dir
             .path()
             .join("creates_and_deletes_populated_directory_if_prune_true");
@@ -257,7 +257,7 @@ mod test {
 
     #[tokio::test]
     async fn creates_and_leaves_populated_directory_if_prune_false() -> eyre::Result<()> {
-        let temp_dir = tempdir::TempDir::new("nix_installer_tests_create_file")?;
+        let temp_dir = tempfile::tempdir()?;
         let test_dir = temp_dir
             .path()
             .join("creates_and_leaves_populated_directory_if_prune_false");

diff --git a/src/action/base/create_file.rs b/src/action/base/create_file.rs
@@ -261,7 +261,7 @@ mod test {
 
     #[tokio::test]
     async fn creates_and_deletes_file() -> eyre::Result<()> {
-        let temp_dir = tempdir::TempDir::new("nix_installer_tests_create_file")?;
+        let temp_dir = tempfile::tempdir()?;
         let test_file = temp_dir.path().join("creates_and_deletes_file");
         let mut action =
             CreateFile::plan(test_file.clone(), None, None, None, "Test".into(), false).await?;
@@ -277,7 +277,7 @@ mod test {
 
     #[tokio::test]
     async fn creates_and_deletes_file_even_if_edited() -> eyre::Result<()> {
-        let temp_dir = tempdir::TempDir::new("nix_installer_tests_create_file")?;
+        let temp_dir = tempfile::tempdir()?;
         let test_file = temp_dir
             .path()
             .join("creates_and_deletes_file_even_if_edited");
@@ -297,7 +297,7 @@ mod test {
 
     #[tokio::test]
     async fn recognizes_existing_exact_files_and_reverts_them() -> eyre::Result<()> {
-        let temp_dir = tempdir::TempDir::new("nix_installer_tests_create_file")?;
+        let temp_dir = tempfile::tempdir()?;
         let test_file = temp_dir
             .path()
             .join("recognizes_existing_exact_files_and_reverts_them");
@@ -326,7 +326,7 @@ mod test {
 
     #[tokio::test]
     async fn recognizes_existing_different_files_and_errors() -> eyre::Result<()> {
-        let temp_dir = tempdir::TempDir::new("nix_installer_tests_create_file")?;
+        let temp_dir = tempfile::tempdir()?;
         let test_file = temp_dir
             .path()
             .join("recognizes_existing_different_files_and_errors");
@@ -354,7 +354,7 @@ mod test {
 
     #[tokio::test]
     async fn recognizes_wrong_mode_and_errors() -> eyre::Result<()> {
-        let temp_dir = tempdir::TempDir::new("nix_installer_tests_create_file")?;
+        let temp_dir = tempfile::tempdir()?;
         let test_file = temp_dir.path().join("recognizes_wrong_mode_and_errors");
         let initial_mode = 0o777;
         let expected_mode = 0o000;
@@ -392,7 +392,7 @@ mod test {
 
     #[tokio::test]
     async fn recognizes_correct_mode() -> eyre::Result<()> {
-        let temp_dir = tempdir::TempDir::new("nix_installer_tests_create_file")?;
+        let temp_dir = tempfile::tempdir()?;
         let test_file = temp_dir.path().join("recognizes_correct_mode");
         let initial_mode = 0o777;
 
@@ -421,7 +421,7 @@ mod test {
 
     #[tokio::test]
     async fn errors_on_dir() -> eyre::Result<()> {
-        let temp_dir = tempdir::TempDir::new("nix_installer_tests_create_file")?;
+        let temp_dir = tempfile::tempdir()?;
 
         match CreateFile::plan(
             temp_dir.path(),

diff --git a/src/action/base/create_or_insert_into_file.rs b/src/action/base/create_or_insert_into_file.rs
@@ -361,7 +361,7 @@ mod test {
 
     #[tokio::test]
     async fn creates_and_deletes_file() -> eyre::Result<()> {
-        let temp_dir = tempdir::TempDir::new("nix_installer_create_or_insert_into_file")?;
+        let temp_dir = tempfile::tempdir()?;
         let test_file = temp_dir.path().join("creates_and_deletes_file");
         let mut action = CreateOrInsertIntoFile::plan(
             test_file.clone(),
@@ -384,7 +384,7 @@ mod test {
 
     #[tokio::test]
     async fn edits_and_reverts_file() -> eyre::Result<()> {
-        let temp_dir = tempdir::TempDir::new("nix_installer_create_or_insert_into_file")?;
+        let temp_dir = tempfile::tempdir()?;
         let test_file = temp_dir.path().join("edits_and_reverts_file");
 
         let test_content = "Some other content";
@@ -419,7 +419,7 @@ mod test {
 
     #[tokio::test]
     async fn recognizes_existing_containing_exact_contents_and_reverts_it() -> eyre::Result<()> {
-        let temp_dir = tempdir::TempDir::new("nix_installer_create_or_insert_into_file")?;
+        let temp_dir = tempfile::tempdir()?;
         let test_file = temp_dir
             .path()
             .join("recognizes_existing_containing_exact_contents_and_reverts_it");
@@ -457,7 +457,7 @@ mod test {
 
     #[tokio::test]
     async fn recognizes_wrong_mode_and_errors() -> eyre::Result<()> {
-        let temp_dir = tempdir::TempDir::new("nix_installer_tests_create_or_insert_into_file")?;
+        let temp_dir = tempfile::tempdir()?;
         let test_file = temp_dir.path().join("recognizes_wrong_mode_and_errors");
         let initial_mode = 0o777;
         let expected_mode = 0o000;
@@ -495,7 +495,7 @@ mod test {
 
     #[tokio::test]
     async fn recognizes_correct_mode() -> eyre::Result<()> {
-        let temp_dir = tempdir::TempDir::new("nix_installer_tests_create_or_insert_into_file")?;
+        let temp_dir = tempfile::tempdir()?;
         let test_file = temp_dir.path().join("recognizes_correct_mode");
         let initial_mode = 0o777;
 
@@ -524,7 +524,7 @@ mod test {
 
     #[tokio::test]
     async fn errors_on_dir() -> eyre::Result<()> {
-        let temp_dir = tempdir::TempDir::new("nix_installer_tests_create_or_insert_into_file")?;
+        let temp_dir = tempfile::tempdir()?;
 
         match CreateOrInsertIntoFile::plan(
             temp_dir.path(),

diff --git a/src/diagnostics.rs b/src/diagnostics.rs
@@ -15,9 +15,8 @@ use reqwest::Url;
 pub enum DiagnosticStatus {
     Cancelled,
     Success,
-    /// This includes the [`strum::IntoStaticStr`] representation of the error, we take special care not to include parameters of the error (which may include secrets)
-    Failure(String),
     Pending,
+    Failure,
 }
 
 /// The action attempted
@@ -36,8 +35,11 @@ pub struct DiagnosticReport {
     pub os_name: String,
     pub os_version: String,
     pub triple: String,
+    pub is_ci: bool,
     pub action: DiagnosticAction,
     pub status: DiagnosticStatus,
+    /// Generally this includes the [`strum::IntoStaticStr`] representation of the error, we take special care not to include parameters of the error (which may include secrets)
+    pub failure_variant: Option<String>,
 }
 
 /// A preparation of data to be sent to the `endpoint`.
@@ -49,7 +51,9 @@ pub struct DiagnosticData {
     os_name: String,
     os_version: String,
     triple: String,
+    is_ci: bool,
     endpoint: Option<Url>,
+    failure_variant: Option<String>,
 }
 
 impl DiagnosticData {
@@ -58,6 +62,7 @@ impl DiagnosticData {
             Ok(os_release) => (os_release.name, os_release.version),
             Err(_) => ("unknown".into(), "unknown".into()),
         };
+        let is_ci = is_ci::cached();
         Self {
             endpoint,
             version: env!("CARGO_PKG_VERSION").into(),
@@ -66,9 +71,16 @@ impl DiagnosticData {
             os_name,
             os_version,
             triple: target_lexicon::HOST.to_string(),
+            is_ci,
+            failure_variant: None,
         }
     }
 
+    pub fn variant(mut self, variant: String) -> Self {
+        self.failure_variant = Some(variant);
+        self
+    }
+
     pub fn report(&self, action: DiagnosticAction, status: DiagnosticStatus) -> DiagnosticReport {
         let Self {
             version,
@@ -77,7 +89,9 @@ impl DiagnosticData {
             os_name,
             os_version,
             triple,
+            is_ci,
             endpoint: _,
+            failure_variant: variant,
         } = self;
         DiagnosticReport {
             version: version.clone(),
@@ -86,8 +100,10 @@ impl DiagnosticData {
             os_name: os_name.clone(),
             os_version: os_version.clone(),
             triple: triple.clone(),
+            is_ci: *is_ci,
             action,
             status,
+            failure_variant: variant.clone(),
         }
     }
 

diff --git a/src/error.rs b/src/error.rs
@@ -84,20 +84,3 @@ impl HasExpectedErrors for NixInstallerError {
         }
     }
 }
-
-// #[cfg(feature = "diagnostics")]
-// impl NixInstallerError {
-//     pub fn diagnostic_synopsis(&self) -> &'static str {
-//         match self {
-//             NixInstallerError::Action(inner) => inner.into(),
-//             NixInstallerError::Planner(inner) => inner.into(),
-//             NixInstallerError::RecordingReceipt(_, _)
-//             | NixInstallerError::CopyingSelf(_)
-//             | NixInstallerError::SerializingReceipt(_)
-//             | NixInstallerError::Cancelled
-//             | NixInstallerError::SemVer(_)
-//             | NixInstallerError::Diagnostic(_)
-//             | NixInstallerError::InstallSettings(_) => self.into(),
-//         }
-//     }
-// }
diff --git a/src/plan.rs b/src/plan.rs
@@ -162,12 +162,13 @@ impl InstallPlan {
                 if let Some(diagnostic_data) = &self.diagnostic_data {
                     diagnostic_data
                         .clone()
+                        .variant({
+                            let x: &'static str = (&err).into();
+                            x.to_string()
+                        })
                         .send(
                             crate::diagnostics::DiagnosticAction::Install,
-                            crate::diagnostics::DiagnosticStatus::Failure({
-                                let x: &'static str = (&err).into();
-                                x.to_string()
-                            }),
+                            crate::diagnostics::DiagnosticStatus::Failure,
                         )
                         .await?;
                 }
@@ -294,12 +295,13 @@ impl InstallPlan {
                 if let Some(diagnostic_data) = &self.diagnostic_data {
                     diagnostic_data
                         .clone()
+                        .variant({
+                            let x: &'static str = (&err).into();
+                            x.to_string()
+                        })
                         .send(
                             crate::diagnostics::DiagnosticAction::Uninstall,
-                            crate::diagnostics::DiagnosticStatus::Failure({
-                                let x: &'static str = (&err).into();
-                                x.to_string()
-                            }),
+                            crate::diagnostics::DiagnosticStatus::Failure,
                         )
                         .await?;
                 }

diff --git a/src/settings.rs b/src/settings.rs
@@ -286,15 +286,15 @@ impl CommonSettings {
             )],
             modify_profile: true,
             nix_build_group_name: String::from("nixbld"),
-            nix_build_group_id: 3000,
+            nix_build_group_id: 30_000,
             nix_build_user_prefix: nix_build_user_prefix.to_string(),
             nix_build_user_id_base,
             nix_package_url: url.parse()?,
             extra_conf: Default::default(),
             force: false,
             #[cfg(feature = "diagnostics")]
             diagnostic_endpoint: Some(
-                "https://install.determinate.systems/diagnostics".try_into()?,
+                "https://install.determinate.systems/nix/diagnostic".try_into()?,
             ),
         })
     }