feat: add automatic nix garbage collection and fix macOS shell tests - #1514
Conversation
|
You do not have enough credits to review this pull request. Please purchase more credits to continue. |
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 54 minutes and 28 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe changes introduce automated Nix garbage collection scheduling with weekly frequency and 30-day retention policies across both home-manager and nix-darwin configurations. A new launchd daemon manages garbage collection on macOS, and the test suite is refactored with portable command resolution utilities. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Mesa DescriptionTL;DRAdded weekly automatic Nix garbage collection on macOS and Linux, introduced What changed?
Description generated by Mesa. Update settings |
![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 introduces automated Nix garbage collection and store optimization across the environment, including updates to the Makefile, Home Manager, and nix-darwin configurations. It also improves the portability of shell scripts and test helpers, specifically addressing differences between GNU and BSD utilities like readlink and stat. Feedback focuses on ensuring consistent use of sudo for store optimization, utilizing idiomatic nix-darwin modules for garbage collection, and improving the robustness of the Python-based path resolution helper.
| @echo "🧹 Cleaning up old generations and garbage collecting..." | ||
| @$(SUDO) nix-collect-garbage -d | ||
| @$(SUDO) nix-collect-garbage --delete-older-than 30d | ||
| @nix store optimise |
There was a problem hiding this comment.
The nix store optimise command should be run with $(SUDO) to ensure it has the necessary permissions to hardlink files across the entire Nix store. Since the preceding nix-collect-garbage command uses $(SUDO), it is consistent to use it here as well for a full system cleanup.
@$(SUDO) nix store optimise
| clean-all: ## Delete ALL old Nix generations and garbage collect (nuclear). | ||
| @echo "🧹 Removing all old generations..." | ||
| @$(SUDO) nix-collect-garbage -d | ||
| @nix store optimise |
There was a problem hiding this comment.
Similar to the clean target, nix store optimise in clean-all should use $(SUDO) to effectively optimize the entire store.
@$(SUDO) nix store optimise
| launchd.daemons."com.shunkakinoki.nix-gc" = { | ||
| script = '' | ||
| /nix/var/nix/profiles/default/bin/nix-collect-garbage --delete-older-than 30d | ||
| ''; | ||
| serviceConfig = { | ||
| RunAtLoad = false; | ||
| StartCalendarInterval = [ | ||
| { | ||
| Weekday = 0; | ||
| Hour = 3; | ||
| Minute = 0; | ||
| } | ||
| ]; | ||
| }; | ||
| }; |
There was a problem hiding this comment.
Instead of manually defining a launchd daemon, it is more idiomatic to use the built-in nix.gc module provided by nix-darwin. This simplifies the configuration, uses the configured Nix package automatically, and follows established patterns for managing garbage collection on macOS.
nix.gc = {
automatic = true;
interval = {
Weekday = 0;
Hour = 3;
Minute = 0;
};
options = "--delete-older-than 30d";
};
| if readlink -f "$cmd_path" >/dev/null 2>&1; then | ||
| dirname "$(readlink -f "$cmd_path")" | ||
| else | ||
| dirname "$(python3 -c "import os; print(os.path.realpath('$cmd_path'))")" |
There was a problem hiding this comment.
Using $cmd_path directly inside the Python command string can lead to syntax errors or unexpected behavior if the path contains single quotes. It is safer to pass the path as an argument to the Python script and access it via sys.argv.
| dirname "$(python3 -c "import os; print(os.path.realpath('$cmd_path'))")" | |
| dirname "$(python3 -c 'import os, sys; print(os.path.realpath(sys.argv[1]))' "$cmd_path")" |
1 participant
Summary
make clean-allfor nuclear GC, changemake cleanto retain 30 days + runnix store optimisereadlink -f(GNU-only) with portableresolve_cmd_dirhelper, fixstat -c(GNU) with macOS-compatible wrapper, fix PATH isolation leakingpbcopyvia/usr/binTest plan
make shell-test- 326 examples, 0 failuresmake shell-lint- exit 0