-
Notifications
You must be signed in to change notification settings - Fork 57
Fix cache flush order to prevent stale data issues #1515
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Conversation
Clear GitContext caches BEFORE calling owner.flush_caches() to ensure that when MacheteClient caches are cleared and potentially rebuilt, they use fresh GitContext data (especially reflogs) rather than stale cached data. This fixes an issue where 'git machete traverse -F' would not detect merged PRs on the first run but would detect them on the second run, due to MacheteClient using stale reflog data from GitContext caches. Add cache_fix_analysis.md documenting the root cause and solution.
Git-Machete Cache Issue AnalysisProblem DescriptionWhen running Root CauseCache ArchitectureGit-machete has two cache layers:
The Issue
Key Code LocationsFetch with cache flush ( if opt_fetch:
for rem in self.__git.get_remotes():
print(f"Fetching {bold(rem)}...")
self.__git.fetch_remote(rem) # This flushes GitContext cachesMacheteClient cache not cleared ( def flush_caches(self) -> None:
self.__branch_pairs_by_hash_in_reflog = None # Only clears this cacheGitContext owner relationship ( def flush_caches(self) -> None:
if self.owner: # This should be MacheteClient
self.owner.flush_caches() # But owner is not always setSolutionsImmediate Workarounds1. Double Traverse Pattern# Run twice - second run will work correctly
git machete traverse -F
git machete traverse -F2. Manual Cache Refresh# Force cache refresh by checking status first
git machete status > /dev/null
git machete traverse -F3. Explicit Fetch + Traverse# Separate fetch from traverse
git fetch --prune --all
git machete traverseLong-term Solutions1. Enhanced Wrapper ScriptCreate a wrapper that ensures proper cache management. 2. Upstream FixThe proper fix would be to ensure Testing the IssueTo reproduce:
VerificationCheck if the issue affects your setup: # After a PR is merged remotely:
git machete status | grep -E "(merged|red)" # Note any red branches
git machete traverse -F # See if it offers to slide out merged branches
git machete traverse -F # Second run should work if first didn't |
|
Hi @PawelLipski -- long time no talk! I've been running into this issue quite frequently and decided to do a some AI-driven investigation |
|
Wow I wasn't aware of this issue, TBH I thought that the caching is perfect already 😅 thanks, I'll take a look + add some regression tests |
|
Thank you @PawelLipski!
|
|
@PawelLipski I don't think the solution in here correct (it's just moving code around). But the problem is real |
|
Hmm I'm trying to reproduce the error scenario now 🤔 I don't think the analysis provided by AI is correct — I've tried a local repro ( Could you provide a screen of a |
Clear GitContext caches BEFORE calling owner.flush_caches() to ensure that when MacheteClient caches are cleared and potentially rebuilt, they use fresh GitContext data (especially reflogs) rather than stale cached data.
This fixes an issue where 'git machete traverse -F' would not detect merged PRs on the first run but would detect them on the second run, due to MacheteClient using stale reflog data from GitContext caches.