Repository navigation
Fix phantom-files stuck in the staging-area - #571
Merged
Merged
Conversation
goneng
changed the title
goneng
force-pushed
the
fix-phantom-files-in-stage-area
branch
2 times, most recently
from
August 17, 2026 21:28
a9bbdbf to
48c58b6
Compare
goneng
marked this pull request as ready for review
August 17, 2026 21:28
Contributor
|
Please rebase to recent master |
Derive the whole file list from the three git outputs of a single index refresh, so that an entry cannot outlive the change it stood for. The Stage view could list files that were not changed at all, showing them as new. Staging one moved it to Staged Changes in GitX while git status reported that path as deleted, and nothing cleared the row short of relaunching GitX. Entries were only deleted once both 'hasStagedChanges' and 'hasUnstagedChanges' were NO, and those flags were cleared by inferring from a file's absence from one of the three dictionaries. An entry with status NEW and a non-nil 'commitBlobSHA' failed the guard on both branches that could clear 'hasUnstagedChanges', so it was never cleared and never deleted. The files array starts empty at launch, hence the relaunch being the only cure. Two ways in, both reachable from renaming and rebasing outside GitX. git 'diff-index --cached' reports an added path with an all-zero source SHA, which was recorded as a real blob while status stayed NEW. Separately, a tracked path appears in 'ls-files --others' whenever the index momentarily lacks it, which set status to NEW while keeping the blob SHA from an earlier pass. Which of the two fired depended on the order the three concurrent commands finished in, so the symptom was intermittent and hit arbitrary files. - Collect the three command outputs and reconcile once, in the completion block, instead of folding each into the shared file list as it arrives - Assign every property from the collected data, so the result depends on git's output alone and no entry can survive a clean working tree - Record no blob when the source mode is ':000000', rather than storing the "all-zero SHA" that git reports as though it were a real blob - Keep the previous list when a command failed, rather than reading a failure as an empty working tree - Reuse the existing entry for a path so table-selection survives a refresh - Announce the index-update once the reconcile has run, so that observers reading the file list are not served the previous refresh's contents - Add 'PBGitIndexReconcileTests' covering both routes into the stuck state, independence from prior state, and the status-precedence that staging and unstaging depend on
Re-run a refresh that arrived while another was in flight, rather than discarding the request and leaving the view showing stale state. A refresh requested during an in-flight one was skipped outright and never re-run. Rebasing or renaming from a terminal produces a burst of 'FSEvents', so the final settling event - the one carrying the state the user ends up looking at - was the most likely of the burst to be dropped. A skipped refresh also never posted 'PBGitIndexFinishedIndexRefresh', leaving the commit view's busy flag set and its status stuck on "Refreshing index". - Record the request instead of dropping it, and run it once the in-flight refresh finishes, so no overlapping refreshes still holds - Read and clear the pending flag together with the in-progress flag, so a request arriving as the refresh ends cannot be lost between the two
Read the index back after staging or unstaging, instead of assuming the outcome git was asked for is the outcome git recorded. Staging flipped each file's staged and unstaged flags optimistically and nothing re-read the index afterwards, so the view reported what had been requested rather than what happened. Staging is done with 'update-index --add --remove', which stages a deletion for any path that no longer exists on disk. A row left over from a path a rename had moved away, therefore, staged the deletion of that path while GitX showed it as an ordinary staged change. - Refresh once the update-index call has succeeded, keeping the optimistic flag update so the tables still respond immediately - Safe to issue per operation, now that an overlapping refresh is coalesced rather than dropped or run concurrently
goneng
force-pushed
the
fix-phantom-files-in-stage-area
branch
from
August 18, 2026 09:47
48c58b6 to
24849b9
Compare
Collaborator
Author
Sure - done. |
Contributor
|
Btw, I would love to see some tests concerning your changes |
Collaborator
Author
@hannesa2, Most of my bug-fixes come from my workflow, where I rebase/force-push from the terminal while GitX is open in the background. |
hannesa2
pushed a commit
that referenced
this pull request
Aug 20, 2026
Add tests for the two commits of #571 that landed without any, and for the notification ordering that the third one changed. PBGitIndexReconcileTests covers how the file list is derived, but nothing exercised what surrounds it: that staging re-reads the index rather than trusting the flags it has just set, that a refresh announces the file list only once it has been rebuilt from all three commands, and that a request arriving during a refresh is deferred rather than discarded. None of that needs a repository on disk. -[PBGitRepository init] touches nothing outside itself, and the two collaborators that would otherwise reach the filesystem are ordinary overridable methods, so test-local subclasses record the calls instead of making them. - Extract the refresh guard into -beginRefreshOrDeferIt and -endRefreshTakingDeferred so the coalescing contract can be asserted directly, leaving what they do unchanged - Add PBGitIndexRefreshTests: staging and unstaging ask git and then read the index back, a rejected update-index reads nothing back, neither arm of a per-command result announces an index update, and a burst of requests collapses into exactly one re-run - Give the GitXTests target a header search path for Classes/Util, which PBGitRepository_PBGitBinarySupport.h needs
Collaborator
Author
Contributor
|
Cool, thank you ! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The stage view could list unchanged files as new, and staging one showed it
as staged in GitX while
git statusreported that path as deleted.Nothing cleared the rows short of relaunching.
(The full analysis is in the individual commit messages.)
an entry cannot outlive the state it came from and the result no longer
depends on which of the three concurrent commands finishes first
it, so the settling event of a rebase burst is not lost
git never recorded
Related
The maintainer diagnosed "incorrectly ordered data/calls when refreshing the index"
there in 2017; that is the mechanism fixed here.
The commit that closed it only hardened
dictionaryForLines:against an odd line count.the file-level unstage in step 4 appeared to work, and only
[CMD]-[R]revealed it had not taken effect."renders as empty ... spam the manual refresh" is the same ordering race.
Closed, cited only as corroboration.
Staging untracked files and then making more changes doesn't show the diff correctly #370,
which is a diff-rendering issue in
diffForFile:.On #428 / #530: an index-only write by GitX does not trigger the FSEvents
refresh, so the refresh added here is the only one for those operations
rather than a duplicate, and it runs the same three read-only commands as
CMD-R (no
update-index, so noindex.lock). Coalescing keeps at most onerefresh in flight, so 0903ca4's process-storm fix still holds.
Test plan
xcodebuild test -scheme GitX -only-testing:GitXTests- 11 tests pass,including 9 new ones covering both routes into the stuck state, order
independence, and the status precedence staging depends on
Scratch repo driven from a terminal with
git mvon files and foldersplus repeated rebases: the stage view settles to exactly
git status,with no leftover rows
60+ rapid index and working-tree mutations: panes matched
git statusexactly (20 staged / 22 unstaged), including a path both staged-added and
deleted in the working tree
Staging and unstaging through the UI now agree with
git statuswithin asecond; verified against the pre-change build, where an index-only
unstage left the view stale until a manual refresh
Toggling Amend while a refresh was in flight, with git slowed so the
window is deterministic: the staged pane and the Commit button both
settle on the amended commit's contents. Against the pre-change build
the same toggle left the pane empty and the button disabled for good,
with only a stage/unstage able to recover it
Depends on Stand up GitXTests unit-test target with a race-condition regression test #566,
which adds the GitXTests target and is included in this branch until it lands.
Will rebase once it is merged.