Preserve trailing newlines and restore connections.env via git checkout on down

Fix set_local_db/set_dot_env unconditionally stripping (then forcing)
the file's trailing newline: they now detect whether the source file
originally ended in a newline and reproduce that on write, verified
against both the newline and no-newline cases.

Replace the reverse-toggle computation for connections.env on `down`
with a straight `git checkout`, so the file always ends up
byte-for-byte identical to HEAD instead of relying on toggle logic to
get it exactly right. Both git call sites now go through a shared
helper that checks the process exit status and errors out instead of
silently reporting success on a failed checkout/update-index, with
test coverage against real temporary git repos.

README.md

@@ -56,7 +56,7 @@ dev_tools foo down
 ## What it does
 
 1. Starts or stops the Docker Compose service defined by `DOCKER_SERVICE` / `DOCKER_DIR`.
-2. Modifies `conf/global/connections.env` and marks it with `git update-index --assume-unchanged` to avoid accidental commits.
+2. `conf/global/connections.env`: `up` toggles it to the CI credentials and marks it `git update-index --assume-unchanged` to avoid accidental commits; `down` unmarks it and restores it to its committed state with `git checkout` (rather than computing the reverse toggle), so it's guaranteed byte-for-byte identical to HEAD.
 3. Toggles database connection entries in `<PROJECT_DIR>/conf/local/.env`: `up` switches the connection to `ci`, `down` switches it to `staging`.
 
 ## Tests

src/config/parse.rs

@@ -1,8 +1,5 @@
-use std::{
+use std::fmt;
-    fmt,
+use std::fs;
-    fs::{self, File},
-    io::{BufRead, BufReader},
-};
 
 use anyhow::{Context, Result};
 
@@ -36,17 +33,16 @@ pub fn set_local_db(start: bool, config: &DevToolsConf) -> Result<FileChange> {
         config.project_dir_as_string(),
         GLOBAL_CONNECTION_PATH
     );
-    let file: File = File::open(&global_connections_env)
+    let content = fs::read_to_string(&global_connections_env)
         .with_context(|| format!("could not read file `{}`", global_connections_env))?;
-    let reader: BufReader<File> = BufReader::new(file);
+    let trailing_newline = content.ends_with('\n');
 
     let mut modified_lines: Vec<String> = Vec::new();
     let mut diffs: Vec<(String, String)> = Vec::new();
-    for line in reader.lines() {
+    for line_content in content.lines() {
-        let line_content = line?;
+        let modified: Option<String> = if start && connections_env_comment(line_content) {
-        let modified: Option<String> = if start && connections_env_comment(&line_content) {
             Some(format!("# {}", line_content))
-        } else if !start && should_commented_out(&line_content) {
+        } else if !start && should_commented_out(line_content) {
             Some(line_content[2..].to_string())
         } else {
             None
@@ -54,15 +50,18 @@ pub fn set_local_db(start: bool, config: &DevToolsConf) -> Result<FileChange> {
 
         match modified {
             Some(new_line) => {
-                diffs.push((line_content, new_line.clone()));
+                diffs.push((line_content.to_string(), new_line.clone()));
                 modified_lines.push(new_line);
             }
-            None => modified_lines.push(line_content),
+            None => modified_lines.push(line_content.to_string()),
         }
     }
 
-    fs::write(&global_connections_env, modified_lines.join("\n"))
+    let mut output = modified_lines.join("\n");
-        .context("Cannot write global connections env")?;
+    if trailing_newline {
+        output.push('\n');
+    }
+    fs::write(&global_connections_env, output).context("Cannot write global connections env")?;
     Ok(FileChange {
         path: global_connections_env,
         diffs,
@@ -87,16 +86,15 @@ pub fn set_dot_env(start: bool, config: &DevToolsConf) -> Result<FileChange> {
         config.project_dir_as_string(),
         LOCAL_CONNECTION_PATH
     );
-    let file: File = File::open(&local_connection_env)
+    let content = fs::read_to_string(&local_connection_env)
         .with_context(|| format!("could not read file `{}`", local_connection_env))?;
-    let reader: BufReader<File> = BufReader::new(file);
+    let trailing_newline = content.ends_with('\n');
 
     let mut modified_lines: Vec<String> = Vec::new();
     let mut diffs: Vec<(String, String)> = Vec::new();
     let mut section: Option<(bool, usize)> = None;
 
-    for line in reader.lines() {
+    for line_content in content.lines() {
-        let line_content = line?;
         let trimmed = line_content.trim_start();
 
         if let Some((_, remaining)) = section {
@@ -107,21 +105,21 @@ pub fn set_dot_env(start: bool, config: &DevToolsConf) -> Result<FileChange> {
 
         if trimmed.starts_with(DOCKER_SECTION) {
             section = Some((start, DOCKER_SECTION_LINES));
-            modified_lines.push(line_content);
+            modified_lines.push(line_content.to_string());
             continue;
         }
         if trimmed.starts_with(CIDB_SECTION) {
             section = Some((!start, CIDB_SECTION_LINES));
-            modified_lines.push(line_content);
+            modified_lines.push(line_content.to_string());
             continue;
         }
 
         let modified: Option<String> = if let Some((activate, remaining)) = &mut section {
-            let new_line = toggle(&line_content, *activate);
+            let new_line = toggle(line_content, *activate);
             *remaining -= 1;
             (new_line != line_content).then_some(new_line)
         } else {
-            connection_key(&line_content).and_then(|key| {
+            connection_key(line_content).and_then(|key| {
                 let value = if start { "ci" } else { "staging" };
                 let new_line = format!("{}='{}'", key, value);
                 (new_line != line_content).then_some(new_line)
@@ -130,15 +128,18 @@ pub fn set_dot_env(start: bool, config: &DevToolsConf) -> Result<FileChange> {
 
         match modified {
             Some(new_line) => {
-                diffs.push((line_content, new_line.clone()));
+                diffs.push((line_content.to_string(), new_line.clone()));
                 modified_lines.push(new_line);
             }
-            None => modified_lines.push(line_content),
+            None => modified_lines.push(line_content.to_string()),
         }
     }
 
-    fs::write(&local_connection_env, modified_lines.join("\n"))
+    let mut output = modified_lines.join("\n");
-        .context("Cannot write local connection env")?;
+    if trailing_newline {
+        output.push('\n');
+    }
+    fs::write(&local_connection_env, output).context("Cannot write local connection env")?;
 
     Ok(FileChange {
         path: local_connection_env,
@@ -285,6 +286,34 @@ mod tests {
         );
     }
 
+    #[test]
+    fn set_local_db_preserves_trailing_newline() {
+        let dir = TempProjectDir::new("trailing_newline_global");
+        let path = dir.join(GLOBAL_CONNECTION_PATH);
+        fs::write(&path, "ci.db.master.user=admin\n").unwrap();
+
+        set_local_db(true, &config_for(&dir)).unwrap();
+
+        assert_eq!(
+            fs::read_to_string(&path).unwrap(),
+            "ci.db.master.user=admin\n"
+        );
+    }
+
+    #[test]
+    fn set_local_db_preserves_missing_trailing_newline() {
+        let dir = TempProjectDir::new("no_trailing_newline_global");
+        let path = dir.join(GLOBAL_CONNECTION_PATH);
+        fs::write(&path, "ci.db.master.user=admin").unwrap();
+
+        set_local_db(true, &config_for(&dir)).unwrap();
+
+        assert_eq!(
+            fs::read_to_string(&path).unwrap(),
+            "ci.db.master.user=admin"
+        );
+    }
+
     #[test]
     fn set_local_db_records_diffs_when_uncommenting() {
         let dir = TempProjectDir::new("uncomment");
@@ -311,6 +340,39 @@ mod tests {
         );
     }
 
+    #[test]
+    fn set_dot_env_keeps_trailing_newline_when_modifying() {
+        let dir = TempProjectDir::new("trailing_newline_local_modify");
+        let path = dir.join(LOCAL_CONNECTION_PATH);
+        fs::write(&path, "connection='ci'\n").unwrap();
+
+        set_dot_env(false, &config_for(&dir)).unwrap();
+
+        assert_eq!(fs::read_to_string(&path).unwrap(), "connection='staging'\n");
+    }
+
+    #[test]
+    fn set_dot_env_preserves_trailing_newline_when_unmodified() {
+        let dir = TempProjectDir::new("trailing_newline_local_noop");
+        let path = dir.join(LOCAL_CONNECTION_PATH);
+        fs::write(&path, "OTHER_KEY=value\n").unwrap();
+
+        set_dot_env(true, &config_for(&dir)).unwrap();
+
+        assert_eq!(fs::read_to_string(&path).unwrap(), "OTHER_KEY=value\n");
+    }
+
+    #[test]
+    fn set_dot_env_preserves_missing_trailing_newline() {
+        let dir = TempProjectDir::new("no_trailing_newline_local");
+        let path = dir.join(LOCAL_CONNECTION_PATH);
+        fs::write(&path, "OTHER_KEY=value").unwrap();
+
+        set_dot_env(true, &config_for(&dir)).unwrap();
+
+        assert_eq!(fs::read_to_string(&path).unwrap(), "OTHER_KEY=value");
+    }
+
     #[test]
     fn set_dot_env_merges_diffs_from_both_sections() {
         let dir = TempProjectDir::new("merge_sections");

src/git/command.rs

@@ -1,9 +1,24 @@
 use std::process::Command;
 
-use anyhow::Result;
+use anyhow::{ensure, Result};
 
 use crate::{config::parse::GLOBAL_CONNECTION_PATH, env::config::DevToolsConf};
 
+/// Runs `git <args>` in `dir` and errors if the process didn't exit
+/// successfully, so a failed checkout/update-index can't be mistaken for one
+/// that succeeded.
+fn run_git(dir: &str, args: &[&str]) -> Result<()> {
+    let status = Command::new("git").current_dir(dir).args(args).status()?;
+
+    ensure!(
+        status.success(),
+        "git {} failed with {status}",
+        args.join(" ")
+    );
+
+    Ok(())
+}
+
 pub fn toggle_index(start: bool, config: &DevToolsConf) -> Result<()> {
     let change: &str = match start {
         true => "--assume-unchanged",
@@ -15,10 +30,149 @@ pub fn toggle_index(start: bool, config: &DevToolsConf) -> Result<()> {
         GLOBAL_CONNECTION_PATH
     );
 
-    Command::new("git")
+    run_git(
-        .current_dir(config.project_dir_as_string())
+        &config.project_dir_as_string(),
-        .args(["update-index", change, &project_path])
+        &["update-index", change, &project_path],
-        .status()?;
+    )
-
+}
-    Ok(())
+
+/// Restores `connections.env` to its committed state via `git checkout`,
+/// used on `down` instead of computing the reverse toggle so the file ends
+/// up byte-for-byte identical to HEAD, with no risk of a parsing mismatch.
+pub fn restore_connections_env(config: &DevToolsConf) -> Result<()> {
+    let project_path = format!(
+        "{}{}",
+        config.project_dir_as_string(),
+        GLOBAL_CONNECTION_PATH
+    );
+
+    run_git(
+        &config.project_dir_as_string(),
+        &["checkout", "--", &project_path],
+    )
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use std::fs;
+    use std::ops::Deref;
+    use std::path::{Path, PathBuf};
+
+    /// An isolated git repository under the OS temp dir, for exercising
+    /// `toggle_index`/`restore_connections_env` against a real `git`
+    /// process without touching this repo's own history. Namespaced by
+    /// process id so concurrent `cargo test` runs can't collide, and
+    /// removed on drop so no stale directories accumulate.
+    struct TempGitRepo(PathBuf);
+
+    impl TempGitRepo {
+        fn new(name: &str) -> Self {
+            let dir = std::env::temp_dir().join(format!(
+                "dev_tools_git_test_{}_{}",
+                std::process::id(),
+                name
+            ));
+            let _ = fs::remove_dir_all(&dir);
+            fs::create_dir_all(dir.join("conf/global")).unwrap();
+
+            let run = |args: &[&str]| {
+                let status = Command::new("git")
+                    .current_dir(&dir)
+                    .args(args)
+                    .status()
+                    .unwrap();
+                assert!(status.success(), "git {args:?} failed");
+            };
+            run(&["init", "-q"]);
+            run(&["config", "user.email", "test@example.com"]);
+            run(&["config", "user.name", "test"]);
+
+            Self(dir)
+        }
+
+        fn commit_connections_env(&self, content: &str) {
+            fs::write(self.join(GLOBAL_CONNECTION_PATH), content).unwrap();
+            let run = |args: &[&str]| {
+                let status = Command::new("git")
+                    .current_dir(&self.0)
+                    .args(args)
+                    .status()
+                    .unwrap();
+                assert!(status.success(), "git {args:?} failed");
+            };
+            run(&["add", GLOBAL_CONNECTION_PATH]);
+            run(&["commit", "-q", "-m", "connections.env"]);
+        }
+    }
+
+    impl Deref for TempGitRepo {
+        type Target = Path;
+        fn deref(&self) -> &Path {
+            &self.0
+        }
+    }
+
+    impl Drop for TempGitRepo {
+        fn drop(&mut self) {
+            let _ = fs::remove_dir_all(&self.0);
+        }
+    }
+
+    fn config_for(dir: &Path) -> DevToolsConf {
+        DevToolsConf {
+            container_service: String::new(),
+            container_dir: String::new(),
+            project_directory: PathBuf::from(format!("{}/", dir.display())),
+        }
+    }
+
+    #[test]
+    fn restore_connections_env_reverts_uncommitted_changes() {
+        let repo = TempGitRepo::new("restore_ok");
+        repo.commit_connections_env("ci.db.user=\"m.rothenhaeusler\"\n");
+        fs::write(repo.join(GLOBAL_CONNECTION_PATH), "corrupted content\n").unwrap();
+
+        restore_connections_env(&config_for(&repo)).unwrap();
+
+        assert_eq!(
+            fs::read_to_string(repo.join(GLOBAL_CONNECTION_PATH)).unwrap(),
+            "ci.db.user=\"m.rothenhaeusler\"\n"
+        );
+    }
+
+    #[test]
+    fn restore_connections_env_fails_when_not_a_git_repo() {
+        let dir = std::env::temp_dir().join(format!(
+            "dev_tools_git_test_{}_not_a_repo",
+            std::process::id()
+        ));
+        let _ = fs::remove_dir_all(&dir);
+        fs::create_dir_all(dir.join("conf/global")).unwrap();
+        fs::write(dir.join(GLOBAL_CONNECTION_PATH), "content\n").unwrap();
+
+        let result = restore_connections_env(&config_for(&dir));
+
+        assert!(result.is_err());
+        let _ = fs::remove_dir_all(&dir);
+    }
+
+    #[test]
+    fn toggle_index_marks_file_assume_unchanged() {
+        let repo = TempGitRepo::new("toggle_index");
+        repo.commit_connections_env("content\n");
+
+        toggle_index(true, &config_for(&repo)).unwrap();
+
+        let output = Command::new("git")
+            .current_dir(&*repo)
+            .args(["ls-files", "-v", GLOBAL_CONNECTION_PATH])
+            .output()
+            .unwrap();
+        let listing = String::from_utf8(output.stdout).unwrap();
+        assert!(
+            listing.starts_with('h'),
+            "expected lower-case assume-unchanged flag, got: {listing}"
+        );
+    }
 }

src/main.rs

@@ -5,7 +5,7 @@ use env::config::{load_config, DevToolsConf};
 use crate::{
     arguments::Arguments,
     config::parse::{set_dot_env, set_local_db},
-    git::command::toggle_index,
+    git::command::{restore_connections_env, toggle_index},
 };
 use clap::Parser;
 
@@ -23,8 +23,14 @@ fn main() -> Result<()> {
 
     start_docker_compose(&config.container_service, &config.container_dir, start)?;
 
-    print!("{}", set_local_db(start, &config)?);
+    if start {
+        print!("{}", set_local_db(start, &config)?);
+    }
     toggle_index(start, &config)?;
+    if !start {
+        restore_connections_env(&config)?;
+        println!("[!] conf/global/connections.env restored to committed state.");
+    }
     print!("{}", set_dot_env(start, &config)?);
 
     println!(