Implement robust check for uncommitted changes in submodules#24
Conversation
Replaced the naive `head.is_none() || index.is_none()` check in `gix_ops.rs` with a proper check using `git2::Repository::statuses()` which correctly identifies uncommitted changes including untracked files while ignoring ignored files. This ensures submodules are not accidentally deinitialized when they have local modifications. Co-authored-by: bashandbone <89049923+bashandbone@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR strengthens safety checks before deinitializing submodules by replacing a naive “index vs HEAD” heuristic with a git2-based status scan to detect local changes.
Changes:
- Uses
git2::Repository::statuseswithStatusOptionsto detect dirty submodule worktrees. - Treats inability to open/read status as “possibly dirty” and blocks deinit unless
force=true. - Updates the error message for the “dirty” case.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/git_ops/gix_ops.rs
Outdated
| let mut status_opts = git2::StatusOptions::new(); | ||
| status_opts.include_untracked(true); | ||
| status_opts.include_ignored(false); | ||
|
|
||
| if let Ok(statuses) = git2_repo.statuses(Some(&mut status_opts)) { | ||
| let is_dirty = !statuses.is_empty(); | ||
| if is_dirty { |
src/git_ops/gix_ops.rs
Outdated
| if is_dirty { | ||
| return Err(anyhow::anyhow!( | ||
| "Submodule '{}' might have uncommitted changes. Use force=true to override.", | ||
| "Submodule '{}' has uncommitted changes. Use force=true to override.", |
src/git_ops/gix_ops.rs
Outdated
| // Check for uncommitted changes using git2 fallback as gix status API is complex | ||
| if let Ok(git2_repo) = git2::Repository::open(&submodule_path) { |
|
@jules please address the comments in #24 (review) -- regarding the use of git2 without a gix implementation, please either implement a gix implementation if possible, or explain why it's not from your research. |
I've re-implemented this to correctly use |
… git2 Switched back from `git2` to `gix` for checking uncommitted changes in `gix_ops.rs` using `submodule_repo.is_dirty()`, which is supported by our current `gix` version and setup. This aligns better with the `gix-first` codebase strategy. Co-authored-by: bashandbone <89049923+bashandbone@users.noreply.github.com>
- Fixed `error[E0716]: temporary value dropped while borrowed` in `tests/integration_tests.rs` by storing an owned `String` via `to_string()`. - Addressed `git submodule add` CLI fallback failure by fully wiping partial initialized repo and `.gitmodules` before passing it onto CLI in `src/git_ops/mod.rs`. - Fixed the shallow clone test setup using `.gitconfig` flags to correctly permit local shallow clone. Co-authored-by: bashandbone <89049923+bashandbone@users.noreply.github.com>
Codecov Report❌ Patch coverage is
... and 3 files with indirect coverage changes 🚀 New features to boost your workflow:
|
Replaced the naive uncommitted changes check in
gix_ops.rswith a robust implementation usinggit2fallback. The new implementation properly checks the status of the submodule repository usinggit2::StatusOptionsto detect uncommitted and untracked changes, preventing accidental loss of work when deinitializing submodules.PR created automatically by Jules for task 5189997611582941240 started by @bashandbone