CreateOrMergeNixConfig: handle existing, invalid nix.custom.conf contents

nix/tests/vm-test/default.nix

@@ -192,6 +192,20 @@ let
       uninstall = installCases.install-default.uninstall;
       uninstallCheck = installCases.install-default.uninstallCheck;
     };
+    install-invalid-custom-conf = {
+      preinstall = ''
+        sudo mkdir -p /etc/nix
+        sudo touch /etc/nix/nix.custom.conf
+        sudo chmod 777 /etc/nix/nix.custom.conf
+        echo "foobar" > /etc/nix/nix.custom.conf
+      '';
+      install = installCases.install-default.install;
+      check = installCases.install-default.check + ''
+        grep --quiet "^# foobar" /etc/nix/nix.custom.conf
+      '';
+      uninstall = installCases.install-default.uninstall;
+      uninstallCheck = installCases.install-default.uninstallCheck;
+    };
     install-determinate = {
       install = nix-installer-install-determinate;
       check = ''

src/action/base/create_or_merge_nix_config.rs

@@ -71,8 +71,15 @@ impl CreateOrMergeNixConfig {
         };
 
         if this.path.exists() {
-            let (merged_nix_config, _) =
-                Self::validate_existing_nix_config(&this.pending_nix_config, &this.path)?;
+            let is_existing_custom_conf =
+                crate::action::common::place_nix_configuration::CUSTOM_NIX_CONFIG_HEADER
+                    == this.header;
+            let (merged_nix_config, _) = Self::validate_nix_config_against_path(
+                &this.pending_nix_config,
+                &this.path,
+                is_existing_custom_conf,
+            )
+            .await?;
 
             if !merged_nix_config.settings().is_empty() {
                 return Ok(StatefulAction::uncompleted(this));
@@ -142,11 +149,12 @@ impl CreateOrMergeNixConfig {
         Ok((merged_nix_config, existing_nix_config.clone()))
     }
 
-    fn validate_existing_nix_config(
+    async fn validate_nix_config_against_path(
         pending_nix_config: &NixConfig,
-        path: &Path,
+        existing_config_file: &Path,
+        is_existing_custom_conf: bool,
     ) -> Result<(NixConfig, NixConfig), ActionError> {
-        let path = path.to_path_buf();
+        let path = existing_config_file.to_path_buf();
         let metadata = path
             .metadata()
             .map_err(|e| Self::error(ActionErrorKind::GettingMetadata(path.clone(), e)))?;
@@ -155,7 +163,13 @@ impl CreateOrMergeNixConfig {
             return Err(Self::error(ActionErrorKind::PathWasNotFile(path)));
         }
 
-        let existing_nix_config = NixConfig::parse_file(&path)
+        let parse_ret = if is_existing_custom_conf {
+            Self::maybe_comment_out_invalid_conf(&path).await?
+        } else {
+            NixConfig::parse_file(&path)
+        };
+
+        let existing_nix_config = parse_ret
             .map_err(CreateOrMergeNixConfigError::ParseNixConfig)
             .map_err(Self::error)?;
 
@@ -168,6 +182,43 @@ impl CreateOrMergeNixConfig {
 
         Ok((merged_nix_config, existing_nix_config))
     }
+
+    async fn maybe_comment_out_invalid_conf(
+        path: &Path,
+    ) -> Result<Result<NixConfig, nix_config_parser::ParseError>, ActionError> {
+        let contents = tokio::fs::read_to_string(path)
+            .await
+            .map_err(|e| ActionErrorKind::Read(path.to_path_buf(), e))
+            .map_err(Self::error)?;
+        let mut lines = contents
+            .lines()
+            .map(str::trim)
+            .map(String::from)
+            .collect::<Vec<_>>();
+        lines.push(String::new());
+
+        let parse_ret = loop {
+            let parse_ret = NixConfig::parse_string(lines.join("\n"), Some(path));
+
+            if let Err(nix_config_parser::ParseError::IllegalConfiguration(bad_line, _)) = parse_ret
+            {
+                for line in lines.iter_mut() {
+                    if *line == bad_line {
+                        line.insert_str(0, "# ");
+                        break;
+                    }
+                }
+            } else {
+                break parse_ret;
+            }
+        };
+
+        crate::util::write_atomic(path, &lines.join("\n"))
+            .await
+            .map_err(Self::error)?;
+
+        Ok(parse_ret)
+    }
 }
 
 #[async_trait::async_trait]
@@ -264,7 +315,8 @@ impl Action for CreateOrMergeNixConfig {
 
         let (mut merged_nix_config, mut existing_nix_config) = if self.path.exists() {
             let (merged_nix_config, existing_nix_config) =
-                Self::validate_existing_nix_config(&self.pending_nix_config, &self.path)?;
+                Self::validate_nix_config_against_path(&self.pending_nix_config, &self.path, false)
+                    .await?;
             (merged_nix_config, Some(existing_nix_config))
         } else {
             (self.pending_nix_config.clone(), None)

src/action/common/place_nix_configuration.rs

@@ -23,7 +23,7 @@ const NIX_CONFIG_HEADER: &str = r#"# Generated by https://github.com/Determinate
 
 const NIX_CONFIG_FOOTER: &str = "!include nix.custom.conf";
 
-const CUSTOM_NIX_CONFIG_HEADER: &str = r#"# Written by https://github.com/DeterminateSystems/nix-installer.
+pub(crate) const CUSTOM_NIX_CONFIG_HEADER: &str = r#"# Written by https://github.com/DeterminateSystems/nix-installer.
 # The contents below are based on options specified at installation time.
 "#;
 

src/action/macos/create_fstab_entry.rs

@@ -113,7 +113,7 @@ impl Action for CreateFstabEntry {
 
         let updated_buf = current_fstab_lines.join("\n");
 
-        write_atomic(fstab_path, &updated_buf)
+        crate::util::write_atomic(fstab_path, &updated_buf)
             .await
             .map_err(Self::error)?;
         Ok(())
@@ -167,7 +167,7 @@ impl Action for CreateFstabEntry {
             current_fstab_lines.push("");
         }
 
-        write_atomic(fstab_path, &current_fstab_lines.join("\n"))
+        crate::util::write_atomic(fstab_path, &current_fstab_lines.join("\n"))
             .await
             .map_err(Self::error)?;
 
@@ -191,17 +191,3 @@ impl From<CreateFstabEntryError> for ActionErrorKind {
         ActionErrorKind::Custom(Box::new(val))
     }
 }
-
-async fn write_atomic(destination: &Path, body: &str) -> Result<(), ActionErrorKind> {
-    let temp = destination.with_extension("tmp");
-
-    tokio::fs::write(&temp, body)
-        .await
-        .map_err(|e| ActionErrorKind::Write(temp.to_owned(), e))?;
-
-    tokio::fs::rename(&temp, &destination)
-        .await
-        .map_err(|e| ActionErrorKind::Rename(temp, destination.into(), e))?;
-
-    Ok(())
-}

src/util.rs

@@ -1,5 +1,7 @@
 use std::path::Path;
 
+use crate::action::ActionErrorKind;
+
 #[derive(Debug, PartialEq, Eq)]
 pub(crate) enum OnMissing {
     Ignore,
@@ -33,3 +35,17 @@ pub(crate) async fn remove_dir_all(path: &Path, on_missing: OnMissing) -> std::i
         e @ Err(_) => e,
     }
 }
+
+pub(crate) async fn write_atomic(destination: &Path, body: &str) -> Result<(), ActionErrorKind> {
+    let temp = destination.with_extension("tmp");
+
+    tokio::fs::write(&temp, body)
+        .await
+        .map_err(|e| ActionErrorKind::Write(temp.to_owned(), e))?;
+
+    tokio::fs::rename(&temp, &destination)
+        .await
+        .map_err(|e| ActionErrorKind::Rename(temp, destination.into(), e))?;
+
+    Ok(())
+}