Merge pull request #12 from wrale/jmdots/dev

jmdots · web-flow · commit a92bd1128c29 · 2025-03-30T16:56:00.000-04:00
Ripping out all commit things, re: 2025-03-30-0007
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/Cargo.toml b/Cargo.toml
@@ -1,6 +1,6 @@
 [package]
 name = "wrale-acdm"
-version = "0.1.0"
+version = "0.2.0"
 edition = "2021"
 description = "Wrale Agnostic Content Dependency Manager"
 authors = ["Wrale LTD <contact@wrale.com>"]
diff --git a/GUIDANCE/seed/README.md b/GUIDANCE/seed/README.md
@@ -18,7 +18,7 @@ Wrale Agnostic Content Dependency Manager
 - **Version Locking**: Pin dependencies to specific commits, branches, or tags
 - **Clean Git History**: Changes to vendored content appear as normal changes in your repository
 - **Multiple Protocol Support**: Clone via SSH and HTTPS with appropriate authentication
-- **Atomic Operations**: All content updates happen in a single atomic commit
+- **Git-Aware Operations**: Operates only within clean Git repositories
 - **Domain-Driven Design**: Clean architecture with proper separation of concerns for extensibility
 
 ## Core Concepts
@@ -67,9 +67,9 @@ Unlike Git submodules or tools that establish ongoing connections to external re
 2. Extract only the specified content based on sparse paths
 3. Clean the target mount point by removing all existing content (not using Git but native filesystem operations)
 4. Copy new content to the clean target location in your project
-5. Stage all changes (additions and removals) for Git
-6. Commit all changes in a single atomic transaction
-7. Clean up temporary data
+5. Clean up temporary data
+
+After these operations, the user needs to manually stage and commit the changes if they wish to persist them.
 
 This approach ensures your repository maintains a clean history without extra metadata or hidden connections to external repositories. The complete cleaning of the mount point (step 3) is crucial for maintaining the integrity of the dependency directory, ensuring a clean slate for each update without relying on git-specific commands.
 
@@ -90,7 +90,7 @@ While [Gitman](../git/gitman.md) offers similar functionality for selective dire
 - Truly zero submodule footprint, with content appearing as normal files
 - More focused feature set without symlinks or post-checkout scripts
 - Clean architecture design for extensibility
-- Atomic operations for content updates
+- Git-aware operations that respect repository state
 
 ## Development Rationale
 
@@ -140,8 +140,8 @@ A key aspect of the implementation is how content changes are handled:
 1. **Complete Mount Point Cleaning**: For each update, the entire target directory is completely cleared using OS-native operations
 2. **Filesystem Abstraction**: Directory removal operations are dependency-inverted to handle cross-OS differences
 3. **Clean Slate Approach**: Rather than tracking diffs, each update creates a pristine copy of the current dependency state
-4. **Conflict Detection**: Optional pre-cleaning checks for local modifications to vendored content
-5. **Atomic Transactions**: All changes are applied in a single Git transaction after filesystem operations are complete
+4. **Conflict Detection**: Pre-operation checks ensure the Git repository is in a clean state
+5. **Safe Operations**: The tool operates only when Git status is clean, preventing accidental overwrites of uncommitted changes
 
 Example of dependency inversion for directory cleaning:
 
diff --git a/README.md b/README.md
@@ -14,7 +14,7 @@ Wrale Agnostic Content Dependency Manager
 - **Version Locking**: Pin dependencies to specific commits, branches, or tags
 - **Clean Git History**: Changes to vendored content appear as normal changes in your repository
 - **Multiple Protocol Support**: Clone via SSH and HTTPS with appropriate authentication
-- **Atomic Operations**: All content updates happen in a single atomic commit
+- **Git-Aware Operations**: Operates only within clean Git repositories
 - **Safe Operations**: Verifies clean git status before making changes
 - **Interactive Workflow**: Prompts before potentially destructive actions (can be bypassed with --force)
 - **Verbose Logging**: Detailed logging for debugging (can be disabled with --quiet)
@@ -68,6 +68,15 @@ Update all dependencies:
 acdm update
 ```
 
+After running any command that modifies files, you'll need to review and commit the changes manually:
+
+```bash
+git add .
+git commit -m "Update external dependencies"
+```
+
+Note: `acdm` requires a clean git repository to perform updates. It will not automatically commit changes.
+
 ## Global Flags
 
 The following flags can be used with any command:
diff --git a/docs/user-guide.md b/docs/user-guide.md
@@ -102,10 +102,11 @@ Update specific dependencies:
 acdm update dep1 dep2
 ```
 
-Automatically commit changes:
+After updating dependencies, you'll need to review and commit the changes manually:
 
 ```bash
-acdm update --message "Update external dependencies"
+git add .
+git commit -m "Update external dependencies"
 ```
 
 Notes:
@@ -162,15 +163,17 @@ RUST_LOG=trace acdm update
 
 `acdm` implements several safety features to protect your repository:
 
-1. **Git Status Check**: Operations will fail if your git repository has uncommitted changes, ensuring a clean state before making modifications.
+1. **Git Repository Requirement**: Operations will only run within a Git repository, ensuring you can track changes.
 
-2. **Mount Point Confirmation**: Before purging mount points during updates, you'll be prompted to confirm the operation.
+2. **Git Status Check**: Operations will fail if your Git repository has uncommitted changes, ensuring a clean state before making modifications.
 
-3. **Atomic Operations**: All changes are staged together, ensuring consistent updates.
+3. **Mount Point Confirmation**: Before purging mount points during updates, you'll be prompted to confirm the operation.
 
 4. **Verbose Logging**: Detailed logs help troubleshoot issues and understand what's happening.
 
-These safety features can be bypassed with the `--force` flag when necessary, but this should be used with caution.
+The mount point confirmation can be bypassed with the `--force` flag when necessary, but this should be used with caution.
+
+Note: Unlike previous versions, `acdm` will not automatically stage or commit any changes. After running operations that modify files, you'll need to stage and commit the changes manually.
 
 ## Best Practices
 
@@ -184,11 +187,13 @@ These safety features can be bypassed with the `--force` flag when necessary, bu
 
 5. **Keep a clean git repository**: Commit your changes before running `acdm` operations.
 
-6. **Use atomic updates**: Update all dependencies at once to ensure consistency.
+6. **Batch updates**: Update all dependencies at once to ensure consistency.
 
 7. **Version lock**: Update dependencies deliberately, not automatically.
 
-8. **Review before committing**: After updates, review the changes before committing them.
+8. **Review and commit manually**: After updates, review all changes and commit them manually.
+
+9. **Create atomic commits**: For cleaner history, commit dependency updates separately from your own code changes.
 
 ## Troubleshooting
 
diff --git a/src/application/dto.rs b/src/application/dto.rs
@@ -27,9 +27,7 @@ pub struct IncludePathsDto {
 pub struct UpdateDependenciesDto {
     pub config_path: PathBuf,
     pub dependencies: Option<Vec<String>>,
-    pub commit_message: Option<String>,
     pub force: bool,
-    pub skip_commit: bool,
 }
 
 /// DTO for dependency status
diff --git a/src/application/status.rs b/src/application/status.rs
@@ -26,9 +26,10 @@ impl<C: ConfigurationRepository> GetDependencyStatusQuery<C> {
             // Join with current directory to get absolute path
             std::env::current_dir()
                 .map_err(|e| {
-                    DomainError::ConfigurationError(
-                        format!("Failed to get current directory: {}", e).into(),
-                    )
+                    DomainError::ConfigurationError(format!(
+                        "Failed to get current directory: {}",
+                        e
+                    ))
                 })?
                 .join(config_path)
         };
@@ -45,9 +46,7 @@ impl<C: ConfigurationRepository> GetDependencyStatusQuery<C> {
         } else {
             // This should rarely happen with absolute paths, but handle it anyway
             std::env::current_dir().map_err(|e| {
-                DomainError::ConfigurationError(
-                    format!("Failed to get current directory: {}", e).into(),
-                )
+                DomainError::ConfigurationError(format!("Failed to get current directory: {}", e))
             })?
         };
 
diff --git a/src/application/use_cases.rs b/src/application/use_cases.rs
@@ -210,13 +210,7 @@ where
 
         // Update all dependencies
         self.dependency_manager
-            .update_all(
-                &dependencies_to_update,
-                repo_root,
-                dto.commit_message.as_deref(),
-                dto.force,
-                dto.skip_commit,
-            )
+            .update_all(&dependencies_to_update, repo_root, dto.force)
             .map_err(|e| anyhow::anyhow!("Failed to update dependencies: {}", e))?;
 
         Ok(())
diff --git a/src/cli/commands/add.rs b/src/cli/commands/add.rs
@@ -24,10 +24,6 @@ pub struct AddCommand {
     /// Target location for the dependency
     #[clap(long)]
     target: String,
-
-    /// Skip automatically committing changes
-    #[clap(long)]
-    skip_commit: bool,
 }
 
 impl AddCommand {
@@ -45,7 +41,6 @@ impl AddCommand {
             self.repository_url.clone(),
             self.rev.clone(),
             self.target.clone(),
-            self.skip_commit,
             force,
         )?;
 
diff --git a/src/cli/commands/include.rs b/src/cli/commands/include.rs
@@ -15,10 +15,6 @@ pub struct IncludeCommand {
 
     /// Paths to include
     paths: Vec<String>,
-
-    /// Skip automatically committing changes
-    #[clap(long)]
-    skip_commit: bool,
 }
 
 impl IncludeCommand {
@@ -28,12 +24,7 @@ impl IncludeCommand {
         debug!("Force mode: {}", force);
 
         let adapter = CliAdapter::new(config_path.to_path_buf());
-        adapter.include_paths(
-            self.dependency_name.clone(),
-            self.paths.clone(),
-            self.skip_commit,
-            force,
-        )?;
+        adapter.include_paths(self.dependency_name.clone(), self.paths.clone(), force)?;
 
         info!("Paths included successfully");
         Ok(())
diff --git a/src/cli/commands/update.rs b/src/cli/commands/update.rs
@@ -12,14 +12,6 @@ use crate::interfaces::cli::CliAdapter;
 pub struct UpdateCommand {
     /// Dependencies to update (all if not specified)
     dependencies: Vec<String>,
-
-    /// Commit message for the update
-    #[clap(short, long)]
-    message: Option<String>,
-
-    /// Skip automatically committing changes
-    #[clap(long)]
-    skip_commit: bool,
 }
 
 impl UpdateCommand {
@@ -40,9 +32,10 @@ impl UpdateCommand {
             Some(self.dependencies.clone())
         };
 
-        adapter.update_dependencies(deps, self.message.clone(), force, self.skip_commit)?;
+        adapter.update_dependencies(deps, force)?;
 
         info!("Dependencies updated successfully");
+        info!("Remember to review and commit your changes with git");
         Ok(())
     }
 }
diff --git a/src/domain/repositories.rs b/src/domain/repositories.rs
@@ -48,12 +48,6 @@ impl GitStatus {
 
 /// Interface for Git operations
 pub trait GitOperations {
-    /// Stages all changes in a directory
-    fn stage_all(&self, repo_path: &Path) -> Result<(), DomainError>;
-
-    /// Commits changes with a message
-    fn commit(&self, repo_path: &Path, message: &str) -> Result<(), DomainError>;
-
     /// Checks if a directory is inside a Git repository
     fn is_git_repository(&self, path: &Path) -> Result<bool, DomainError>;
 
diff --git a/src/domain/services.rs b/src/domain/services.rs
@@ -61,8 +61,7 @@ where
 
         // Check if repo_root is valid and is a git repository
         if repo_root.exists() && self.git_operations.is_git_repository(repo_root)? {
-            // Stage all changes
-            self.git_operations.stage_all(repo_root)?;
+            // No staging action needed anymore
         }
 
         // Clean up the temporary directory
@@ -103,31 +102,13 @@ where
         &self,
         dependencies: &[Dependency],
         repo_root: &Path,
-        commit_message: Option<&str>,
         _force: bool, // Unused but necessary for API compatibility
-        skip_commit: bool,
     ) -> Result<(), DomainError> {
         // Update each dependency
         for dependency in dependencies {
             self.dependency_updater.update(dependency, repo_root)?;
         }
 
-        // Commit changes if a commit message is provided, we're not skipping commits, and we're in a git repository
-        if !skip_commit {
-            if let Some(message) = commit_message {
-                if repo_root.exists()
-                    && self
-                        .dependency_updater
-                        .git_operations
-                        .is_git_repository(repo_root)?
-                {
-                    self.dependency_updater
-                        .git_operations
-                        .commit(repo_root, message)?;
-                }
-            }
-        }
-
         Ok(())
     }
 }
diff --git a/src/infrastructure/git.rs b/src/infrastructure/git.rs
@@ -24,75 +24,6 @@ impl GitOperationsImpl {
 }
 
 impl GitOperations for GitOperationsImpl {
-    fn stage_all(&self, repo_path: &Path) -> Result<(), DomainError> {
-        let output = Command::new("git")
-            .arg("add")
-            .arg("--all")
-            .current_dir(repo_path)
-            .output()
-            .map_err(|e| DomainError::GitError(format!("Failed to stage changes: {}", e)))?;
-
-        if !output.status.success() {
-            return Err(DomainError::GitError(format!(
-                "Git add command failed: {}",
-                String::from_utf8_lossy(&output.stderr)
-            )));
-        }
-
-        Ok(())
-    }
-
-    fn commit(&self, repo_path: &Path, message: &str) -> Result<(), DomainError> {
-        debug!("Attempting to commit with message: '{}'", message);
-
-        // First, check if there are any changes to commit
-        let status_result = self.get_status(repo_path)?;
-        if !status_result.has_staged_changes
-            && !status_result.has_unstaged_changes
-            && !status_result.has_untracked_files
-        {
-            debug!("Nothing to commit - repository is clean");
-            return Ok(());
-        }
-
-        // Stage any changes if needed
-        if !status_result.has_staged_changes
-            && (status_result.has_unstaged_changes || status_result.has_untracked_files)
-        {
-            debug!("Staging changes before commit");
-            self.stage_all(repo_path)?;
-        }
-
-        // Now try to commit
-        let output = Command::new("git")
-            .arg("commit")
-            .arg("-m")
-            .arg(message)
-            .current_dir(repo_path)
-            .output()
-            .map_err(|e| DomainError::GitError(format!("Failed to commit changes: {}", e)))?;
-
-        if !output.status.success() {
-            // Check if there were no changes to commit
-            let stderr = String::from_utf8_lossy(&output.stderr);
-            let stdout = String::from_utf8_lossy(&output.stdout);
-
-            if stderr.contains("nothing to commit") || stdout.contains("nothing to commit") {
-                debug!("Git reported nothing to commit");
-                return Ok(());
-            }
-
-            error!("Git commit failed: {}", stderr);
-            return Err(DomainError::GitError(format!(
-                "Git commit command failed: {}",
-                stderr
-            )));
-        }
-
-        debug!("Commit successful");
-        Ok(())
-    }
-
     fn is_git_repository(&self, path: &Path) -> Result<bool, DomainError> {
         debug!("Checking if path is a git repository: {}", path.display());
 
diff --git a/src/interfaces/cli.rs b/src/interfaces/cli.rs
diff --git a/tests/integration/dependency_management.rs b/tests/integration/dependency_management.rs
diff --git a/tests/integration/git_operations.rs b/tests/integration/git_operations.rs
diff --git a/tests/integration/mod.rs b/tests/integration/mod.rs
diff --git a/tests/integration/skip_commit.rs b/tests/integration/skip_commit.rs

Original file line number	Diff line number	Diff line change
`@@ -27,9 +27,7 @@ pub struct IncludePathsDto {`
`27`	`27`	`pub struct UpdateDependenciesDto {`
`28`	`28`	`pub config_path: PathBuf,`
`29`	`29`	`pub dependencies: Option<Vec<String>>,`
`30`		`- pub commit_message: Option<String>,`
`31`	`30`	`pub force: bool,`
`32`		`- pub skip_commit: bool,`
`33`	`31`	`}`
`34`	`32`
`35`	`33`	`/// DTO for dependency status`