Add directory cleanup after ref deletion#160
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request enhances the git ref lock cleaner script by introducing a retry mechanism for git fetch operations and implementing automatic cleanup of empty parent directories within the .git structure after a reference is deleted. It also refines the error handling flow to return success statuses and updates regex patterns for better compatibility. A review comment identifies that the handle override in Type2Handler is redundant and potentially problematic due to a fragile regex and incorrect directory cleanup logic, suggesting that the base class implementation is sufficient.
There was a problem hiding this comment.
Pull request overview
This PR enhances the .github/scripts/git_ref_lock_cleaner.py helper used in CI/local automation to recover from git fetch failures caused by ref lock/conflict issues, adding retry logic and cleanup of now-empty ref/reflog directories under .git.
Changes:
- Refactors handler flow so
handle()/delete_ref()return a boolean success value, enabling controlled retries. - Adds cleanup of empty parent directories after ref/reflog deletions.
- Expands/refines error matching (including a more specific conflict case) and retries
git fetchup to 2 times when a fix was applied.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- git_ref_lock_cleaner.py: add clean_gc_log() and run_gc() before fetch, add timeout to git fetch (600s) and git prune (300s) - prepare_test_env.py: add timeout (600s) and progress logging to _execute_remote_command for SSH remote operations
- Type2Handler regex: use \s* after colon to handle multi-line git error output - remove_empty_parents: use startswith(stop_dir + os.sep) to prevent matching sibling paths like refs_backup when stop_dir is refs - cleanup_ref_dirs: use isfile() instead of exists(), handle directory-form ref paths that conflict with ref creation
| # error: cannot lock ref 'refs/remotes/origin/fix/3.0/TD-32817': is at 7af5 but expected eaba | ||
| # match the branch name before ‘is at’ with a regular expression | ||
| # match the branch name before 'is at' with a regular expression | ||
| def match(self, error_output: str) -> bool: | ||
| return "is at" in error_output and "but expected" in error_output |
This pull request refactors and enhances the
.github/scripts/git_ref_lock_cleaner.pyscript to improve error handling, robustness, and cleanup of empty directories after reference deletions. The changes ensure that the script can better identify and resolve git ref lock errors, clean up any resulting empty directories, and retry operations when appropriate.Error handling and recovery improvements:
handle,delete_ref, and related methods return a boolean indicating success, allowing for better control flow and retry logic. [1] [2] [3]git fetchup to two times if an error is detected and fixed, improving robustness against transient issues.Directory cleanup enhancements:
remove_empty_parentsutility and integrated it into the ref deletion process to automatically remove empty parent directories under.git/refsand.git/logs/refsafter a ref or reflog is deleted. [1] [2]Error pattern matching and handling:
These changes collectively make the script more reliable and easier to maintain in the face of various git ref lock error patterns.