Conversation
There was a problem hiding this comment.
Pull request overview
This PR refactors the Makefile to detect the platform automatically and provide unified target names instead of platform-specific ones. The change aims to simplify the developer experience by using the same command names (e.g., make build, make switch) on both macOS and NixOS systems.
Changes:
- Introduced platform detection using
uname -sto differentiate between Darwin (macOS) and other systems (NixOS) - Replaced platform-specific targets (
macos-build,nixos-build, etc.) with unified targets (build,switch, etc.) - Made Nix build and switch commands platform-aware through conditional variable assignment
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| clean: | ||
| sudo nix-env --delete-generations +7 --profile /nix/var/nix/profiles/system |
There was a problem hiding this comment.
The clean target now uses a unified profile path that is incorrect for macOS. On macOS (Darwin), the profile path should be /nix/var/nix/profiles/system-profiles/darwin as in the original macos-clean target, not /nix/var/nix/profiles/system. This will cause the clean operation to fail or target the wrong profile on macOS systems. Consider adding platform-specific logic similar to the build and switch commands.
| brew-install: | ||
| $(PWD)/brew/bin/setup.sh | ||
|
|
||
| nixos-switch: | ||
| sudo nixos-rebuild switch --flake .#nixos | ||
| brew: | ||
| brew bundle --verbose --file=$(PWD)/brew/config.d/base/Brewfile | ||
|
|
||
| nixos-update: | ||
| nix flake update | ||
| brew-gui: | ||
| brew bundle --verbose --file=$(PWD)/brew/config.d/gui/Brewfile | ||
|
|
||
| nixos-clean: | ||
| sudo nix-env --delete-generations +7 --profile /nix/var/nix/profiles/system | ||
| brew-optional: | ||
| brew bundle --verbose --file=$(PWD)/brew/config.d/optional/Brewfile | ||
|
|
||
| nixos-gc: | ||
| sudo nix-collect-garbage -d | ||
| brew-himkt: | ||
| brew bundle --verbose --file=$(PWD)/brew/config.d/himkt/Brewfile |
There was a problem hiding this comment.
The brew targets are labeled as "macOS only" but lack platform guards. When these targets are run on non-macOS systems, they will fail because the brew command and Brewfiles are not available. Consider adding conditional checks or making these targets conditional based on the detected platform, similar to how NIX_BUILD_CMD and NIX_SWITCH_CMD are set conditionally.
| build: | ||
| $(NIX_BUILD_CMD) |
There was a problem hiding this comment.
The new unified build target breaks existing CI workflows. The GitHub Actions workflows at .github/workflows/macos.yml (line 14) and .github/workflows/nixos.yml (line 14) still reference the old target names macos-build and nixos-build respectively. These files need to be updated to use the new build target, otherwise the CI will fail after this change is merged.
| build: | ||
| $(NIX_BUILD_CMD) |
There was a problem hiding this comment.
The README.md documentation is now outdated. It still documents the old target names (macos-build, macos-switch, macos-brew, macos-brew-gui, nixos-build, nixos-switch, etc.) in the Setup section (lines 33-47) and Makefile Targets table (lines 51-67). These need to be updated to reflect the new platform-aware target names (build, switch, brew, brew-gui, etc.) to avoid user confusion.
| NIX_BUILD_CMD := nix build .\#darwinConfigurations.macos.system | ||
| NIX_SWITCH_CMD := sudo darwin-rebuild switch --flake .\#macos | ||
| else | ||
| NIX_BUILD_CMD := nix build .\#nixosConfigurations.nixos.config.system.build.toplevel | ||
| NIX_SWITCH_CMD := sudo nixos-rebuild switch --flake .\#nixos |
There was a problem hiding this comment.
The hash character in the flake references is escaped with a backslash (e.g., .\#darwinConfigurations.macos.system), but the original working Makefile used unescaped hashes (.#darwinConfigurations.macos.system). In Makefile recipe lines (commands after tabs), the hash character is typically passed literally to the shell and does not start a comment, so the backslash escaping may be unnecessary and could potentially cause the command to fail. Verify that these commands work correctly with the escaped syntax, or revert to the unescaped syntax that was previously working.
1 participant
No description provided.