Skip to content

change!: make checkout() optionally consider the previous index - #2900

Closed
Dirkjan Ochtman (djc) wants to merge 1 commit into
GitoxideLabs:mainfrom
djc:checkout-cleanup
Closed

change!: make checkout() optionally consider the previous index#2900
Dirkjan Ochtman (djc) wants to merge 1 commit into
GitoxideLabs:mainfrom
djc:checkout-cleanup

Conversation

@djc

Copy link
Copy Markdown
Contributor

We've noticed some issues with how checkout() works differently from git checkout in gitoxide, since it does not remove files that become untracked when checking out into an existing working dir:

This PR adds a previous_index: Option<&gix_index::State> argument to make it more obvious that this is a thing that people need to think about, and makes it easier to get CLI-like behavior.

(With help from Claude.)

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 777163c7c1

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "Codex (@codex) review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "Codex (@codex) address that feedback".

Comment thread gix-worktree-state/src/checkout/mod.rs
@Byron

Sebastian Thiel (Byron) commented Aug 10, 2026

Copy link
Copy Markdown
Member

I spent 10m with it and arrived at the conclusion that this won't help me delivering the required quality level. So it's best to close it and wait for me to get to it, which will be in the coming months.
There is a reason checkout isn't fully implemented, it's deceptively simple.
Thanks for your understanding

Analysis

Codex created, not my speech.

• Found four behavioral differences from Git:

  1. High — partial deletion on checkout failure. gix-worktree-state/src/checkout/
    removal.rs:34 deletes entries while validating them. If a later stale file is
    modified, checkout fails after earlier files are gone. Git validates through
    unpack_trees() first, marks removals at /Users/byron/dev/github.com/git/git/unpack-
    trees.c:2690, then mutates at /Users/byron/dev/github.com/git/git/unpack-
    trees.c:460. A Git experiment with stale a and modified stale z aborted with both
    files preserved.

  2. High — force can recursively delete untracked files. gix-worktree-state/src/
    checkout/removal.rs:105 calls remove_dir_all() when a tracked file has become a
    directory. Git calls mode-aware unlink_entry() at /Users/byron/dev/github.com/git/
    git/entry.c:610; for the same git checkout -f case it warns that it cannot unlink
    the path and preserves the directory and its untracked contents.

  3. Medium — local-change detection is weaker. gix-worktree-state/src/checkout/
    removal.rs:113 checks file kind plus cached stat fields only. Git’s /Users/byron/
    dev/github.com/git/git/unpack-trees.c:2223 uses ie_match_stat(), covering
    executable-bit changes and racily-clean content checks. This branch can therefore
    delete locally modified files whose cached stat still matches.

  4. Medium — deleted submodules remain behind. gix-worktree-state/src/checkout/
    removal.rs:38 skips every Mode::COMMIT. Git explicitly moves/removes submodules in /
    Users/byron/dev/github.com/git/git/entry.c:612. The current tests intentionally
    avoid submodules, so this gap is uncovered.

  • [P1] Do not recursively delete replacement directories — /Users/byron/dev/github.com/GitoxideLabs/gitoxide.checkout-cleanup/gix-worktree-state/src/checkout/removal.rs:111-111
    When overwrite_existing is true and a formerly tracked file has been replaced by a directory, remove_dir_all() deletes every untracked file beneath it. This violates the API guarantee that untracked files are never removed; forced cleanup should leave this directory for checkout collision handling rather than recursively deleting unrelated user data.

  • [P1] Verify racy-clean files before removing them — /Users/byron/dev/github.com/GitoxideLabs/gitoxide.checkout-cleanup/gix-worktree-state/src/checkout/removal.rs:116-116
    When a stale tracked file is rewritten with different content of the same size within the same timestamp second, the default use_nsec = false comparison still matches because its inode and other fields remain unchanged. This then removes the local modification despite overwrite_existing = false; racy-clean entries need a content comparison or must conservatively be retained.

  • [P2] Use filesystem-aware mode matching — /Users/byron/dev/github.com/GitoxideLabs/gitoxide.checkout-cleanup/gix-worktree-state/src/checkout/removal.rs:114-114
    When options.fs.symlink is false, as it is by default on Windows, an index symlink is deliberately checked out as a regular file, so this comparison rejects an untouched stale entry unless forced. It also omits executable-bit changes; use the existing mode comparison with options.fs.symlink and options.fs.executable_bit instead of testing only is_symlink().

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants