Extract host config materialization into a reusable Home Manager module#1
Merged
Conversation
Reviewer's GuideExtracts host configuration materialization logic into a reusable Home Manager module with shared shell helpers, adds automated tests and CI checks, and exposes the module as a flake output for external use. Sequence diagram for hostConfig activation and file materializationsequenceDiagram
participant HM as HomeManager
participant Activation as home_activation_scripts
participant Lib as host_config_lib_sh
participant FS as Filesystem
HM->>Activation: run restoreNixLinks (entryBefore checkLinkTargets)
Activation->>Lib: source host_config_lib_sh
loop for each file in allFiles
Activation->>Lib: _hc_restore_file($HOME/file)
Lib->>FS: rm -f file.new
alt link_backup_exists
Lib->>FS: mv file.lnk to file
end
end
HM->>Activation: run createHostConfig (entryAfter linkGeneration)
Activation->>Lib: source host_config_lib_sh
loop for each file in allFiles
Activation->>Lib: _hc_create_file($HOME/file)
alt file_is_symlink
Lib->>FS: readlink -f file
Lib->>FS: cp target to file.new
Lib->>FS: mv file to file.lnk
Lib->>FS: mv file.new to file
Lib->>FS: chmod 644 file
else not_a_symlink
Lib-->>Activation: return 0
end
end
Class diagram for the new hostConfig Home Manager module and shell helpersclassDiagram
class HostConfigModule {
+bool enable
+list~string~ files
+bool xdgDesktopEntries
+list~string~ desktopFiles
+list~string~ allFiles
+string restoreScript
+string createScript
}
class HomeManagerConfig {
+HostConfigModule hostConfig
+XdgConfig xdg
+HomeActivation home_activation
}
class XdgConfig {
+map desktopEntries
}
class HomeActivation {
+string restoreNixLinks
+string createHostConfig
}
class HostConfigLib {
+_hc_restore_file(path)
+_hc_create_file(path)
}
class FlakeOutputs {
+HostConfigModule homeManagerModules_hostConfig
}
HomeManagerConfig --> HostConfigModule : has
HomeManagerConfig --> XdgConfig : uses
HomeManagerConfig --> HomeActivation : configures
HostConfigModule ..> HostConfigLib : activation_scripts_source
HostConfigModule --> HomeActivation : populates
XdgConfig --> HostConfigModule : derives_desktopFiles
FlakeOutputs --> HostConfigModule : exposes_as_output
Class diagram for CI workflow and Makefile check targetsclassDiagram
class Makefile {
+target check
+target check_nix
+target check_bats
}
class CheckNixTarget {
+command nix_build_homeConfigurations_thrix_activationPackage
+command nix_build_homeConfigurations_mvadkert_activationPackage
}
class CheckBatsTarget {
+command bats_tests
}
class GithubWorkflowCheck {
+job pre_commit
+job nix_build
+job bats
}
class PreCommitJob {
+uses actions_checkout_v4
+uses cachix_install_nix_action_v31
+uses pre_commit_action_v3_0_1
}
class NixBuildJob {
+uses actions_checkout_v4
+uses cachix_install_nix_action_v31
+run make_check_nix
}
class BatsJob {
+uses actions_checkout_v4
+uses cachix_install_nix_action_v31
+run nix_shell_bats_make_check_bats
}
Makefile --> CheckNixTarget : defines
Makefile --> CheckBatsTarget : defines
GithubWorkflowCheck --> PreCommitJob : has
GithubWorkflowCheck --> NixBuildJob : has
GithubWorkflowCheck --> BatsJob : has
NixBuildJob --> CheckNixTarget : invokes
BatsJob --> CheckBatsTarget : invokes
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- In
modules/host-config-lib.sh,_hc_create_fileunconditionally sets mode644, which will strip execute or group/owner bits if you ever materialize non-config files; consider either preserving the original target’s permissions (e.g. viastat/chmod --reference) or making the mode configurable via a module option. - In
flake.nix, you both exporthomeManagerModules.hostConfigand also reference./modules/host-config.nixdirectly in themoduleslist; consider consuming the exported module (e.g.self.homeManagerModules.hostConfig) in your own flake as well to avoid divergence if the module path or wiring changes later.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `modules/host-config-lib.sh`, `_hc_create_file` unconditionally sets mode `644`, which will strip execute or group/owner bits if you ever materialize non-config files; consider either preserving the original target’s permissions (e.g. via `stat`/`chmod --reference`) or making the mode configurable via a module option.
- In `flake.nix`, you both export `homeManagerModules.hostConfig` and also reference `./modules/host-config.nix` directly in the `modules` list; consider consuming the exported module (e.g. `self.homeManagerModules.hostConfig`) in your own flake as well to avoid divergence if the module path or wiring changes later.
## Individual Comments
### Comment 1
<location path="Makefile" line_range="24" />
<code_context>
+ nix build .#homeConfigurations.thrix.activationPackage --no-link
+ nix build .#homeConfigurations.mvadkert.activationPackage --no-link
+
+check/bats: ## Run bats unit tests
+ bats tests/
+
</code_context>
<issue_to_address>
**suggestion (testing):** Make `check/bats` self-contained by ensuring `bats` is available, similar to CI
This target currently assumes `bats` is installed locally, while CI runs it via `nix shell nixpkgs#bats`. To keep local runs consistent with CI, consider wrapping the command the same way:
```make
check/bats: ## Run bats unit tests
nix shell nixpkgs#bats --command bats tests/
```
Alternatively, add a check that fails with a clear error if `bats` is not available on PATH.
</issue_to_address>
### Comment 2
<location path="modules/host-config-lib.sh" line_range="14-23" />
<code_context>
+ fi
+}
+
+_hc_create_file() {
+ local _hc_file="$1"
+ [ -L "$_hc_file" ] || return 0
+ local _hc_target
+ _hc_target=$(readlink -f "$_hc_file")
+ echo -e "\e[32mCopying '$_hc_target' to '$_hc_file'\e[0m"
+ cp "$_hc_target" "$_hc_file.new" || return 1
+ mv "$_hc_file" "$_hc_file.lnk"
+ mv "$_hc_file.new" "$_hc_file"
+ chmod 644 "$_hc_file"
+}
</code_context>
<issue_to_address>
**🚨 suggestion (security):** Consider preserving original file mode instead of forcing permissions to 644
The final `chmod 644 "$_hc_file"` makes every materialized file world‑readable, regardless of the original target’s permissions, which can unintentionally expose sensitive data.
You can instead mirror the target’s mode, falling back to 644 on error:
```sh
_hc_mode=$(stat -c '%a' "$_hc_target") || _hc_mode=644
chmod "$_hc_mode" "$_hc_file"
```
This keeps permissions aligned with the source while retaining a safe default.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Replace inline activation scripts in home.nix with a new
modules/host-config.nix module that materializes Home Manager symlinks
as real files for Silverblue host access.
Changes:
- Extract shell logic into modules/host-config-lib.sh (single source
of truth for both activation scripts and bats tests)
- Add hostConfig.{enable,files,xdgDesktopEntries} options
- Expose module as homeManagerModules.hostConfig flake output
- Auto-derive desktop entry paths from xdg.desktopEntries (picks up
dropbox which was previously missing)
- Use atomic copy-before-move pattern for safer file materialization
- Add bats test suite exercising the real shell library
- Add GitHub Actions CI (nix build, bats, pre-commit)
- Add Dependabot for GitHub Actions version updates
- Add make check/nix, check/bats, check targets
Assisted-by: Claude Code
Signed-off-by: Miroslav Vadkerti <mvadkert@redhat.com>
thrix
force-pushed
the
host-config-module
branch
from
0ac5525 to
f50ee95
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
restoreNixLinks/createHostConfigactivation scripts inhome.nixwith a newmodules/host-config.nixHome Manager modulemodules/host-config-lib.sh— single source of truth for both activation scripts and bats testshostConfig.{enable, files, xdgDesktopEntries}options withxdgDesktopEntriesauto-deriving paths fromconfig.xdg.desktopEntries.new, mv symlink to.lnk, mv.newto target) for safer materializationhomeManagerModules.hostConfigas a flake output for external consumersmake checktargetsAssisted-by: Claude Code
Summary by Sourcery
Extract host configuration materialization into a reusable Home Manager module and wire it into the flake for local and external use.
New Features:
Enhancements:
CI:
Documentation:
Tests:
Chores: