feat(local-binaries): add module to sync local binaries to PATH - #470
Conversation
Add a home-manager module that reads binary paths from ~/dotfiles/.local-binaries.txt and symlinks them to ~/.local/bin during `make switch`. This allows managing local development binaries without tracking them in git. - Add sync-local-binaries.sh script with shellcheck compliance - Add home-manager activation hook to run on switch - Add .local-binaries.txt to .gitignore (machine-specific) - Add shellspec tests for the sync script
|
You do not have enough credits to review this pull request. Please purchase more credits to continue. |
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughSummary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings. WalkthroughAdds a Home Manager module and activation hook plus a shell script that syncs entries from Changes
Sequence DiagramsequenceDiagram
autonumber
participant HM as Home Manager
participant Hook as symlinkLocalBinaries (activation hook)
participant Script as sync-local-binaries.sh
participant Config as ~/.dotfiles/.local-binaries.txt
participant FS as Filesystem
HM->>Hook: trigger after writeBoundary
Hook->>Script: execute via bash
activate Script
Script->>Config: test -f (exists?)
alt missing
Script-->>Hook: log "file missing" & exit
else present
Script->>FS: mkdir -p ~/.local/bin
Script->>Config: read lines
loop per listed path
Script->>Script: trim, skip empty/comments
Script->>FS: test -e && test -x <path>
alt valid executable
Script->>FS: ln -sf <path> ~/.local/bin/<basename>
Script-->>Script: log "Linked: name -> path"
else invalid
Script-->>Script: log "Skipping: reason"
end
end
Script-->>Hook: return success
end
deactivate Script
Hook-->>HM: activation complete
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro Disabled knowledge base sources:
⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
💤 Files with no reviewable changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (13)
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 |
Summary of ChangesHello @shunkakinoki, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a new Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
Mesa DescriptionTL;DRAdds a home-manager module to sync machine-specific local binaries to 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 a new home-manager module to manage symlinks for local binaries. The implementation is solid, using a shell script triggered on activation to read a list of binaries from a text file and create symlinks in ~/.local/bin. I've identified a bug in the shell script where paths with spaces are not handled correctly, and I've also provided a suggestion to improve the test suite to be more robust by testing behavior instead of implementation details. Overall, this is a great addition.
| Describe 'local-binaries/sync-local-binaries.sh' | ||
| SCRIPT="$PWD/home-manager/modules/local-binaries/sync-local-binaries.sh" | ||
|
|
||
| Describe 'script properties' | ||
| It 'uses bash shebang' | ||
| When run bash -c "head -1 '$SCRIPT'" | ||
| The output should include '#!/usr/bin/env bash' | ||
| End | ||
|
|
||
| It 'uses strict mode' | ||
| When run bash -c "head -5 '$SCRIPT'" | ||
| The output should include 'set -euo pipefail' | ||
| End | ||
| End | ||
|
|
||
| Describe 'configuration' | ||
| It 'reads from dotfiles/.local-binaries.txt' | ||
| When run bash -c "grep 'BINARIES_FILE=' '$SCRIPT'" | ||
| The output should include 'dotfiles/.local-binaries.txt' | ||
| End | ||
|
|
||
| It 'writes to ~/.local/bin' | ||
| When run bash -c "grep 'BIN_DIR=' '$SCRIPT'" | ||
| The output should include '.local/bin' | ||
| End | ||
| End | ||
|
|
||
| Describe 'file handling' | ||
| It 'exits gracefully when binaries file is missing' | ||
| When run bash -c "grep -A 2 'if \[ ! -f' '$SCRIPT'" | ||
| The output should include 'exit 0' | ||
| End | ||
|
|
||
| It 'creates bin directory if needed' | ||
| When run bash -c "grep 'mkdir -p' '$SCRIPT'" | ||
| The output should include 'mkdir -p' | ||
| End | ||
| End | ||
|
|
||
| Describe 'line processing' | ||
| It 'skips empty lines' | ||
| When run bash -c "grep -E '\\[ -z' '$SCRIPT'" | ||
| The output should include 'continue' | ||
| End | ||
|
|
||
| It 'skips comment lines' | ||
| When run bash -c "grep -A 2 'Skip comments' '$SCRIPT'" | ||
| The output should include 'continue' | ||
| End | ||
|
|
||
| It 'checks if binary exists and is executable' | ||
| When run bash -c "grep '\[ ! -f' '$SCRIPT'" | ||
| The output should include '! -x' | ||
| End | ||
| End | ||
|
|
||
| Describe 'symlink creation' | ||
| It 'uses basename for symlink name' | ||
| When run bash -c "grep 'basename' '$SCRIPT'" | ||
| The output should include 'basename' | ||
| End | ||
|
|
||
| It 'creates symlinks with ln -sf' | ||
| When run bash -c "grep 'ln -sf' '$SCRIPT'" | ||
| The output should include 'ln -sf' | ||
| End | ||
| End | ||
|
|
||
| End |
There was a problem hiding this comment.
The tests in this file check the implementation details of the script by greping its source code, rather than testing its behavior. This makes the tests brittle; they can break even with valid refactoring that doesn't change the script's functionality.
For more robust and maintainable tests, I recommend switching to behavioral tests. You could use shellspec's features to:
- Set up a temporary
HOMEdirectory. - Create a mock
.local-binaries.txtfile with various test cases (e.g., valid paths, paths with spaces, commented lines, non-existent files). - Create dummy executable files.
- Run the script.
- Assert that the expected symlinks are created (or not created) in the temporary
~/.local/bindirectory.
This approach tests what the script does, not how it's written, which is much more valuable.
Here's an example of what a behavioral test could look like:
Describe 'sync-local-binaries.sh behavior'
setup() {
# Set up a temporary environment for testing
HOME="$SHELLSPEC_TMPDIR"
export HOME
BINARIES_FILE="$HOME/dotfiles/.local-binaries.txt"
BIN_DIR="$HOME/.local/bin"
mkdir -p "$(dirname "$BINARIES_FILE")"
# Create a dummy executable
DUMMY_BIN_PATH="$SHELLSPEC_TMPDIR/my-test-binary"
echo '#!/bin/sh' > "$DUMMY_BIN_PATH"
chmod +x "$DUMMY_BIN_PATH"
}
Before 'setup'
It 'creates a symlink for a valid binary path'
# Arrange
echo "$DUMMY_BIN_PATH" > "$BINARIES_FILE"
# Act
When run "$SCRIPT"
# Assert
The status should be success
The path "$BIN_DIR/my-test-binary" should be a symlink to "$DUMMY_BIN_PATH"
The output should include "Linked: my-test-binary -> $DUMMY_BIN_PATH"
End
End![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 5 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="home-manager/modules/local-binaries/sync-local-binaries.sh">
<violation number="1" location="home-manager/modules/local-binaries/sync-local-binaries.sh:36">
P2: Missing validation for absolute paths. The script documents that paths should be absolute but doesn't enforce it. A relative path that happens to resolve could create a broken symlink. Consider adding validation after the comment check:
```bash
case "$line" in
/*) ;;
*) echo "Skipping (not an absolute path): $line"; continue ;;
esac
```</violation>
</file>
Reply to cubic to teach it or ask questions. Tag @cubic-dev-ai to re-run a review.
| esac | ||
|
|
||
| # Check if binary exists and is executable | ||
| if [ ! -f "$line" ] || [ ! -x "$line" ]; then |
There was a problem hiding this comment.
P2: Missing validation for absolute paths. The script documents that paths should be absolute but doesn't enforce it. A relative path that happens to resolve could create a broken symlink. Consider adding validation after the comment check:
case "$line" in
/*) ;;
*) echo "Skipping (not an absolute path): $line"; continue ;;
esacPrompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At home-manager/modules/local-binaries/sync-local-binaries.sh, line 36:
<comment>Missing validation for absolute paths. The script documents that paths should be absolute but doesn't enforce it. A relative path that happens to resolve could create a broken symlink. Consider adding validation after the comment check:
```bash
case "$line" in
/*) ;;
*) echo "Skipping (not an absolute path): $line"; continue ;;
esac
```</comment>
<file context>
@@ -0,0 +1,47 @@
+ esac
+
+ # Check if binary exists and is executable
+ if [ ! -f "$line" ] || [ ! -x "$line" ]; then
+ echo "Skipping (not found or not executable): $line"
+ continue
</file context>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (3)
home-manager/modules/default.nix (1)
1-6: LGTM!The new module is correctly added to the imports list.
Consider sorting the module paths alphabetically for consistency:
🔎 Optional: Sort alphabetically
[ ./local-binaries + ./npm-globals + ./tailscale + ./yek +]becomes:
[ ./local-binaries ./npm-globals ./tailscale ./yek ]home-manager/modules/local-binaries/sync-local-binaries.sh (1)
22-47: Consider adding cleanup of stale symlinks.The script creates new symlinks but doesn't remove old ones when binaries are removed from
.local-binaries.txt. Over time,~/.local/binmay accumulate orphaned symlinks.Add logic to track managed symlinks and remove those no longer in the config file:
🔎 Example cleanup approach
Before the main loop, collect current managed links:
# Track what we link in this run declare -A current_links # In the loop, record each link: current_links["$bin_name"]=1 # After the loop, clean up unmanaged symlinks: for link in "$BIN_DIR"/*; do [ -L "$link" ] || continue bin_name="$(basename "$link")" if [[ ! -v current_links["$bin_name"] ]]; then echo "Removing stale symlink: $bin_name" rm "$link" fi doneNote: This requires careful design to avoid removing user-created symlinks. Consider a marker file or naming convention.
spec/local_binaries_spec.sh (1)
1-72: Tests validate script structure but not behavior.The current tests use
grepto verify that expected code patterns exist in the script. While this ensures the script contains the right constructs, it doesn't verify actual runtime behavior.Consider adding integration tests that:
- Create a temporary binaries file with test data
- Actually execute the script
- Verify symlinks are created correctly
- Test edge cases (missing files, non-executable files, comments, empty lines)
🔎 Example integration test structure
Describe 'integration tests' setup() { TEST_DIR="$(mktemp -d)" TEST_BIN="$TEST_DIR/bin" TEST_LIST="$TEST_DIR/.local-binaries.txt" mkdir -p "$TEST_BIN" # Create test binaries... } cleanup() { rm -rf "$TEST_DIR" } BeforeEach 'setup' AfterEach 'cleanup' It 'creates symlinks for valid binaries' # Create test binary echo '#!/bin/bash' > "$TEST_DIR/test-binary" chmod +x "$TEST_DIR/test-binary" echo "$TEST_DIR/test-binary" > "$TEST_LIST" # Run script with test paths When run env HOME="$TEST_DIR" bash sync-local-binaries.sh The path "$TEST_BIN/test-binary" should be symlink The status should be success End End
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
.gitignorehome-manager/modules/default.nixhome-manager/modules/local-binaries/default.nixhome-manager/modules/local-binaries/sync-local-binaries.shspec/local_binaries_spec.sh
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{sh,bash}
📄 CodeRabbit inference engine (CLAUDE.md)
Use shfmt with 2-space indentation for shell scripts
**/*.{sh,bash}: Use 2 spaces for indentation in shell scripts
Add proper shebang lines to shell scripts
Follow shellcheck recommendations in shell scripts
Document complex commands in shell scripts
Use consistent variable naming in shell scripts
Files:
spec/local_binaries_spec.shhome-manager/modules/local-binaries/sync-local-binaries.sh
**/*.nix
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.nix: Use nixfmt for formatting all Nix files
Document complex configurations with comments in Nix files
**/*.nix: Use 2 spaces for indentation in Nix files
Keep line length under 100 characters in Nix files
Sort attribute sets alphabetically in Nix files
Use consistent spacing around operators in Nix files
Format lists and sets consistently in Nix filesUse treefmt.toml for formatting Nix files
**/*.nix: UsemkOptionfor configurable options in Nix modules
Implement proper typing for all options in Nix modules
Follow the Nix expression language style guide
Files:
home-manager/modules/local-binaries/default.nixhome-manager/modules/default.nix
**/default.nix
📄 CodeRabbit inference engine (CLAUDE.md)
Use
default.nixfiles for module exports
Files:
home-manager/modules/local-binaries/default.nixhome-manager/modules/default.nix
home-manager/modules/*/default.nix
📄 CodeRabbit inference engine (.cursor/rules/home-manager.mdc)
Custom modules must be located in
home-manager/modules/<name>/and must include adefault.nixfile
Files:
home-manager/modules/local-binaries/default.nix
home-manager/modules/**/default.nix
📄 CodeRabbit inference engine (.cursor/rules/home-manager.mdc)
Custom modules should include proper option types and document all options
Each module in
home-manager/modules/should have a cleardefault.nixwith proper option declarations following the home-manager module structure
Files:
home-manager/modules/local-binaries/default.nixhome-manager/modules/default.nix
home-manager/**/*.nix
📄 CodeRabbit inference engine (.cursor/rules/home-manager.mdc)
home-manager/**/*.nix: Use typed options whenever possible in Nix configurations
Document all configuration options in Nix modules and programs
Follow home-manager's module structure and keep configurations modular
Use proper indentation and formatting in Nix configuration files
Files:
home-manager/modules/local-binaries/default.nixhome-manager/modules/default.nix
home-manager/modules/**/*.nix
📄 CodeRabbit inference engine (.cursor/rules/nix.mdc)
Document all custom modules and options
Files:
home-manager/modules/local-binaries/default.nixhome-manager/modules/default.nix
🧠 Learnings (16)
📓 Common learnings
Learnt from: CR
Repo: shunkakinoki/dotfiles PR: 0
File: .cursor/rules/general.mdc:0-0
Timestamp: 2025-11-25T09:34:40.062Z
Learning: Test Nix and home-manager configurations locally before pushing using `make test`
Learnt from: CR
Repo: shunkakinoki/dotfiles PR: 0
File: .cursor/rules/home-manager.mdc:0-0
Timestamp: 2025-11-25T09:34:55.014Z
Learning: Applies to home-manager/**/*.nix : Follow home-manager's module structure and keep configurations modular
📚 Learning: 2025-11-25T09:34:32.423Z
Learnt from: CR
Repo: shunkakinoki/dotfiles PR: 0
File: .cursor/rules/formatting.mdc:0-0
Timestamp: 2025-11-25T09:34:32.423Z
Learning: Applies to **/*.{sh,bash} : Follow shellcheck recommendations in shell scripts
Applied to files:
spec/local_binaries_spec.sh
📚 Learning: 2025-11-25T09:34:32.423Z
Learnt from: CR
Repo: shunkakinoki/dotfiles PR: 0
File: .cursor/rules/formatting.mdc:0-0
Timestamp: 2025-11-25T09:34:32.423Z
Learning: Applies to **/*.{sh,bash} : Add proper shebang lines to shell scripts
Applied to files:
spec/local_binaries_spec.sh
📚 Learning: 2025-11-25T09:34:32.423Z
Learnt from: CR
Repo: shunkakinoki/dotfiles PR: 0
File: .cursor/rules/formatting.mdc:0-0
Timestamp: 2025-11-25T09:34:32.423Z
Learning: Applies to **/*.{sh,bash} : Use consistent variable naming in shell scripts
Applied to files:
spec/local_binaries_spec.sh
📚 Learning: 2025-11-25T09:34:32.423Z
Learnt from: CR
Repo: shunkakinoki/dotfiles PR: 0
File: .cursor/rules/formatting.mdc:0-0
Timestamp: 2025-11-25T09:34:32.423Z
Learning: Applies to **/*.{sh,bash} : Document complex commands in shell scripts
Applied to files:
spec/local_binaries_spec.sh
📚 Learning: 2025-11-25T09:34:55.014Z
Learnt from: CR
Repo: shunkakinoki/dotfiles PR: 0
File: .cursor/rules/home-manager.mdc:0-0
Timestamp: 2025-11-25T09:34:55.014Z
Learning: Applies to home-manager/**/*.nix : Follow home-manager's module structure and keep configurations modular
Applied to files:
home-manager/modules/local-binaries/default.nixhome-manager/modules/default.nix
📚 Learning: 2025-11-25T09:35:01.066Z
Learnt from: CR
Repo: shunkakinoki/dotfiles PR: 0
File: .cursor/rules/nix.mdc:0-0
Timestamp: 2025-11-25T09:35:01.066Z
Learning: Applies to home-manager/modules/**/*.nix : Document all custom modules and options
Applied to files:
home-manager/modules/local-binaries/default.nixhome-manager/modules/default.nix
📚 Learning: 2025-11-25T09:35:01.066Z
Learnt from: CR
Repo: shunkakinoki/dotfiles PR: 0
File: .cursor/rules/nix.mdc:0-0
Timestamp: 2025-11-25T09:35:01.066Z
Learning: Applies to home-manager/modules/**/default.nix : Each module in `home-manager/modules/` should have a clear `default.nix` with proper option declarations following the home-manager module structure
Applied to files:
home-manager/modules/local-binaries/default.nixhome-manager/modules/default.nix
📚 Learning: 2025-11-25T09:34:55.014Z
Learnt from: CR
Repo: shunkakinoki/dotfiles PR: 0
File: .cursor/rules/home-manager.mdc:0-0
Timestamp: 2025-11-25T09:34:55.014Z
Learning: Applies to home-manager/modules/*/default.nix : Custom modules must be located in `home-manager/modules/<name>/` and must include a `default.nix` file
Applied to files:
home-manager/modules/local-binaries/default.nixhome-manager/modules/default.nix
📚 Learning: 2025-11-25T09:34:23.224Z
Learnt from: CR
Repo: shunkakinoki/dotfiles PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T09:34:23.224Z
Learning: Applies to **/default.nix : Use `default.nix` files for module exports
Applied to files:
home-manager/modules/local-binaries/default.nixhome-manager/modules/default.nix
📚 Learning: 2025-11-25T09:34:55.014Z
Learnt from: CR
Repo: shunkakinoki/dotfiles PR: 0
File: .cursor/rules/home-manager.mdc:0-0
Timestamp: 2025-11-25T09:34:55.014Z
Learning: Applies to home-manager/programs/*/default.nix : Program configurations should be located in `home-manager/programs/<name>/` and use home-manager's built-in modules when available
Applied to files:
home-manager/modules/local-binaries/default.nixhome-manager/modules/default.nix
📚 Learning: 2025-11-25T09:34:55.014Z
Learnt from: CR
Repo: shunkakinoki/dotfiles PR: 0
File: .cursor/rules/home-manager.mdc:0-0
Timestamp: 2025-11-25T09:34:55.014Z
Learning: Applies to home-manager/**/*.nix : Document all configuration options in Nix modules and programs
Applied to files:
home-manager/modules/local-binaries/default.nixhome-manager/modules/default.nix
📚 Learning: 2025-11-25T09:34:40.062Z
Learnt from: CR
Repo: shunkakinoki/dotfiles PR: 0
File: .cursor/rules/general.mdc:0-0
Timestamp: 2025-11-25T09:34:40.062Z
Learning: Document all major changes in Nix configurations
Applied to files:
home-manager/modules/local-binaries/default.nixhome-manager/modules/default.nix
📚 Learning: 2025-11-25T09:34:40.062Z
Learnt from: CR
Repo: shunkakinoki/dotfiles PR: 0
File: .cursor/rules/general.mdc:0-0
Timestamp: 2025-11-25T09:34:40.062Z
Learning: Keep configurations modular across home-manager, hosts, and nix-darwin directories
Applied to files:
home-manager/modules/local-binaries/default.nix
📚 Learning: 2025-11-25T09:34:55.014Z
Learnt from: CR
Repo: shunkakinoki/dotfiles PR: 0
File: .cursor/rules/home-manager.mdc:0-0
Timestamp: 2025-11-25T09:34:55.014Z
Learning: Applies to home-manager/programs/**/default.nix : Program configurations must include all necessary dependencies in their configuration
Applied to files:
home-manager/modules/local-binaries/default.nixhome-manager/modules/default.nix
📚 Learning: 2025-11-25T09:34:55.014Z
Learnt from: CR
Repo: shunkakinoki/dotfiles PR: 0
File: .cursor/rules/home-manager.mdc:0-0
Timestamp: 2025-11-25T09:34:55.014Z
Learning: Applies to home-manager/programs/**/*.nix : Follow program-specific best practices in program configuration files
Applied to files:
home-manager/modules/local-binaries/default.nixhome-manager/modules/default.nix
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (14)
- GitHub Check: cubic · AI code reviewer
- GitHub Check: e2e-run (Ubuntu, ubuntu-latest)
- GitHub Check: lua-neovim-test
- GitHub Check: lua-hammerspoon
- GitHub Check: nix-linux
- GitHub Check: nix-nixos
- GitHub Check: nix-darwin
- GitHub Check: e2e-run (NixOS, ubuntu-latest)
- GitHub Check: docker-build-push (linux/arm64, arm64, ubuntu-24.04-arm)
- GitHub Check: e2e-run (MacOS, macos-latest)
- GitHub Check: docker-build-push (linux/amd64, amd64, ubuntu-latest)
- GitHub Check: lua-neovim
- GitHub Check: shell-lint
- GitHub Check: shell-test
🔇 Additional comments (2)
.gitignore (1)
3-5: LGTM!The gitignore entry for machine-specific local binaries configuration is appropriately placed and documented.
home-manager/modules/local-binaries/sync-local-binaries.sh (1)
1-3: LGTM!Proper shebang and strict error handling are in place.
| { config, pkgs, ... }: | ||
| { | ||
| # Create ~/.local/bin directory | ||
| home.file.".local/bin/.keep".text = ""; | ||
|
|
||
| # Symlink local binaries during activation | ||
| home.activation.symlinkLocalBinaries = config.lib.dag.entryAfter [ "writeBoundary" ] '' | ||
| $DRY_RUN_CMD ${pkgs.bash}/bin/bash ${./sync-local-binaries.sh} | ||
| ''; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Module lacks proper option declarations and documentation.
Per home-manager module structure guidelines, custom modules should include:
- An options block with typed declarations
- An enable flag to allow users to opt in/out
- Configurable paths instead of hardcoded values
- Module documentation
Currently, the module always activates without user control and has no configuration options.
As per coding guidelines: "Custom modules should include proper option types and document all options" and "Each module in home-manager/modules/ should have a clear default.nix with proper option declarations following the home-manager module structure."
🔎 Proposed module structure
{ config, lib, pkgs, ... }:
with lib;
let
cfg = config.programs.local-binaries;
in
{
options.programs.local-binaries = {
enable = mkEnableOption "local binaries synchronization";
binariesFile = mkOption {
type = types.str;
default = "${config.home.homeDirectory}/dotfiles/.local-binaries.txt";
description = "Path to the file containing binary paths to symlink";
};
binDir = mkOption {
type = types.str;
default = "${config.home.homeDirectory}/.local/bin";
description = "Directory where symlinks will be created";
};
};
config = mkIf cfg.enable {
# Create bin directory
home.file."${cfg.binDir}/.keep".text = "";
# Symlink local binaries during activation
home.activation.symlinkLocalBinaries =
config.lib.dag.entryAfter [ "writeBoundary" ] ''
$DRY_RUN_CMD ${pkgs.bash}/bin/bash ${./sync-local-binaries.sh} \
"${cfg.binariesFile}" "${cfg.binDir}"
'';
};
}Note: This also requires updating sync-local-binaries.sh to accept parameters.
🤖 Prompt for AI Agents
In home-manager/modules/local-binaries/default.nix around lines 1 to 10, the
module is missing proper option declarations, an enable flag, configurable path
options, and documentation; update the file to define
options.programs.local-binaries with mkEnableOption "local binaries
synchronization", add mkOption entries for binariesFile (type str, default to a
file under config.home.homeDirectory) and binDir (type str, default to
${config.home.homeDirectory}/.local/bin) and descriptions, bind cfg =
config.programs.local-binaries and wrap the existing home.file and
home.activation entries in mkIf cfg.enable so the module only activates when
enabled, replace hardcoded paths with ${cfg.binDir} and pass
"${cfg.binariesFile}" and "${cfg.binDir}" as parameters to the
sync-local-binaries.sh invocation, and add a brief module description
block/documentation comment at top; also update sync-local-binaries.sh to accept
and use the two parameters.
| home.activation.symlinkLocalBinaries = config.lib.dag.entryAfter [ "writeBoundary" ] '' | ||
| $DRY_RUN_CMD ${pkgs.bash}/bin/bash ${./sync-local-binaries.sh} | ||
| ''; |
There was a problem hiding this comment.
No error handling for sync script failure.
If sync-local-binaries.sh encounters an error (despite set -e), the activation continues silently. Users won't be notified of synchronization failures.
Consider whether script failures should:
- Fail the entire activation (current behavior with
-e) - Log errors but continue (remove
-e, add explicit error logging) - Provide a configuration option for this behavior
The current approach with set -euo pipefail will fail activation on any error, which may be too strict if a single invalid binary prevents the entire home-manager switch.
| BINARIES_FILE="${HOME}/dotfiles/.local-binaries.txt" | ||
| BIN_DIR="${HOME}/.local/bin" |
There was a problem hiding this comment.
Hardcoded "dotfiles" directory name reduces portability.
The path ${HOME}/dotfiles/.local-binaries.txt assumes the repository is cloned to ~/dotfiles, which may not be true for all users or CI environments.
Consider one of these approaches:
- Make the path relative to the script location
- Accept the path as a parameter or environment variable
- Document this assumption clearly in the PR/README
🔎 Example: Use script location
-BINARIES_FILE="${HOME}/dotfiles/.local-binaries.txt"
+SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
+REPO_ROOT="$(cd "$SCRIPT_DIR/../../.." && pwd)"
+BINARIES_FILE="$REPO_ROOT/.local-binaries.txt"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| BINARIES_FILE="${HOME}/dotfiles/.local-binaries.txt" | |
| BIN_DIR="${HOME}/.local/bin" | |
| SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" | |
| REPO_ROOT="$(cd "$SCRIPT_DIR/../../.." && pwd)" | |
| BINARIES_FILE="$REPO_ROOT/.local-binaries.txt" | |
| BIN_DIR="${HOME}/.local/bin" |
🤖 Prompt for AI Agents
In home-manager/modules/local-binaries/sync-local-binaries.sh around lines 9-10
the path to .local-binaries.txt is hardcoded to
${HOME}/dotfiles/.local-binaries.txt which breaks portability; change the script
to resolve the file relative to the script location (e.g. determine SCRIPT_DIR
from $0 and use ${SCRIPT_DIR}/.local-binaries.txt) and also accept an override
via an environment variable or CLI parameter (e.g. LOCAL_BINARIES_FILE) so
callers/CI can specify a different location; update any docs/README to note the
new env/arg option if present.
| # Get binary name and create symlink | ||
| bin_name="$(basename "$line")" | ||
| target="$BIN_DIR/$bin_name" | ||
|
|
||
| ln -sf "$line" "$target" | ||
| echo "Linked: $bin_name -> $line" |
There was a problem hiding this comment.
Basename conflicts are not detected or reported.
If multiple binaries in .local-binaries.txt have the same basename (e.g., /opt/tool/v1/binary and /opt/tool/v2/binary), the last one silently overwrites earlier symlinks without warning.
Consider warning users when basename conflicts occur:
🔎 Proposed fix
# Get binary name and create symlink
bin_name="$(basename "$line")"
target="$BIN_DIR/$bin_name"
+
+ if [ -L "$target" ] && [ "$(readlink "$target")" != "$line" ]; then
+ echo "Warning: $bin_name already links to $(readlink "$target"), overwriting with $line"
+ fi
ln -sf "$line" "$target"
echo "Linked: $bin_name -> $line"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # Get binary name and create symlink | |
| bin_name="$(basename "$line")" | |
| target="$BIN_DIR/$bin_name" | |
| ln -sf "$line" "$target" | |
| echo "Linked: $bin_name -> $line" | |
| # Get binary name and create symlink | |
| bin_name="$(basename "$line")" | |
| target="$BIN_DIR/$bin_name" | |
| if [ -L "$target" ] && [ "$(readlink "$target")" != "$line" ]; then | |
| echo "Warning: $bin_name already links to $(readlink "$target"), overwriting with $line" | |
| fi | |
| ln -sf "$line" "$target" | |
| echo "Linked: $bin_name -> $line" |
1 participant
Changes
local-binarieshome-manager module that syncs custom binaries to~/.local/bin~/dotfiles/.local-binaries.txt(gitignored, machine-specific)make switchvia home-manager activationTechnical Details
sync-local-binaries.sh: Shell script that reads the txt file and creates symlinksdefault.nix: Home-manager module that runs the script during activation#) and skips non-existent/non-executable binariesln -sffor idempotent symlink creationUsage
Testing
make shell-testpasses (253 examples, 0 failures)make shell-checkpasses (shellcheck compliant)🤖 Generated with Claude Code by Claude
Summary by cubic
Adds a home-manager module that syncs machine-specific local binaries into PATH by symlinking them to ~/.local/bin during make switch. Reads entries from ~/dotfiles/.local-binaries.txt so you can use local tools without committing them.
New Features
Migration
Written for commit 821067b. Summary will update automatically on new commits.