Open
Conversation
Reviewer's GuideAdjusts nix-toolbox’s persistent Nix store layout to match upstream user-namespace sandboxing defaults, preserves legacy installs automatically, and updates documentation with details on the new path and a new host configuration module for Home Manager users. Sequence diagram for Home Manager hostConfig activation during home-manager switchsequenceDiagram
actor User
participant HomeManager
participant hostConfigModule
participant restoreNixLinks
participant linkGeneration
participant createHostConfig
participant HostFilesystem as Host_filesystem
participant HostApp as Host_application
User ->> HomeManager: home-manager switch
HomeManager ->> hostConfigModule: Evaluate configuration
hostConfigModule ->> restoreNixLinks: Register pre-activation script
hostConfigModule ->> createHostConfig: Register post-link script
Note over restoreNixLinks,createHostConfig: Activation phase begins
HomeManager ->> restoreNixLinks: Run restoreNixLinks (before checkLinkTargets)
restoreNixLinks ->> HostFilesystem: Restore .lnk backups
restoreNixLinks ->> HostFilesystem: Clean up leftover .new files
HomeManager ->> linkGeneration: Run linkGeneration
linkGeneration ->> HostFilesystem: Create / update symlinks into /nix/store
HomeManager ->> createHostConfig: Run createHostConfig (after linkGeneration)
createHostConfig ->> HostFilesystem: Copy symlink targets to real files
createHostConfig ->> HostFilesystem: Atomically move temp files into place
User ->> HostApp: Launch host application
HostApp ->> HostFilesystem: Read materialized config files
Flow diagram for Nix store path selection and bind mountingflowchart TD
A_start([Start container / run nix.sh]) --> B_check_legacy{Does $XDG_DATA_HOME/nix/store exist?}
B_check_legacy -- Yes --> C_set_legacy[Set NIX_STORE_DIR = $XDG_DATA_HOME/nix]
B_check_legacy -- No --> D_set_new[Set NIX_STORE_DIR = $XDG_DATA_HOME/nix/root/nix]
C_set_legacy --> E_mkdir
D_set_new --> E_mkdir
E_mkdir[mkdir -p $NIX_STORE_DIR] --> F_check_nix_exists{Does /nix exist?}
F_check_nix_exists -- Yes --> G_is_dir{Is /nix a directory?}
F_check_nix_exists -- No --> H_create_nix_dir[sudo mkdir -p /nix]
G_is_dir -- No --> I_error_non_dir[/Print error: /nix exists but is not a directory/]
I_error_non_dir --> J_exit1[Exit 1]
G_is_dir -- Yes --> K_check_mount
H_create_nix_dir --> K_check_mount
K_check_mount{Is /nix a mountpoint?} -- Yes --> L_done([Done: persistent store already mounted])
K_check_mount -- No --> M_bind_mount[sudo mount --bind $NIX_STORE_DIR /nix]
M_bind_mount --> N_mount_ok{Bind mount succeeded?}
N_mount_ok -- No --> O_error_mount[/Print error: Failed to bind-mount $NIX_STORE_DIR to /nix/]
O_error_mount --> J_exit1
N_mount_ok -- Yes --> P_done([Done: /nix bind-mounted from persistent store])
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've found 3 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="nix.sh" line_range="46-50" />
<code_context>
+# Determine persistent Nix store location
+# New installs use upstream-compatible path: $XDG_DATA_HOME/nix/root/nix/
+# Existing installs with store directly in $XDG_DATA_HOME/nix/ are preserved
+if [ -d "$XDG_DATA_HOME/nix/store" ]; then
+ NIX_STORE_DIR="$XDG_DATA_HOME/nix"
+else
+ NIX_STORE_DIR="$XDG_DATA_HOME/nix/root/nix"
+fi
+
# Ensure /nix is bind-mounted from persistent storage
</code_context>
<issue_to_address>
**issue:** Guard against XDG_DATA_HOME being unset to avoid accidentally probing /nix/store
When XDG_DATA_HOME is unset, this ends up probing `/nix/store`, which can incorrectly treat a system `/nix` as user data and change behavior based on the host. Consider either initializing XDG_DATA_HOME to a safe default earlier, or only running this logic when `[ -n "$XDG_DATA_HOME" ]` so it applies only when explicitly configured.
</issue_to_address>
### Comment 2
<location path="nix.sh" line_range="44-49" />
<code_context>
export GUM_SPIN_TITLE="Please wait, this might take a while"
+# Determine persistent Nix store location
+# New installs use upstream-compatible path: $XDG_DATA_HOME/nix/root/nix/
+# Existing installs with store directly in $XDG_DATA_HOME/nix/ are preserved
+if [ -d "$XDG_DATA_HOME/nix/store" ]; then
+ NIX_STORE_DIR="$XDG_DATA_HOME/nix"
+else
+ NIX_STORE_DIR="$XDG_DATA_HOME/nix/root/nix"
+fi
+
</code_context>
<issue_to_address>
**suggestion:** Clarify whether NIX_STORE_DIR is intended to point to the Nix root or its parent
`NIX_STORE_DIR` is set to `$XDG_DATA_HOME/nix/root/nix` and then bind-mounted to `/nix`, but in usual setups `/nix` itself is the Nix root (containing `store`, `var`, etc.), not its parent. Please clarify in the comment whether `.../root/nix` is meant to be the actual Nix root or a container directory, and adjust either the directory layout or wording so it’s unambiguous how this relates to upstream’s `$XDG_DATA_HOME/nix/root` convention.
</issue_to_address>
### Comment 3
<location path="docs/host-config.md" line_range="5" />
<code_context>
+
+When running Home Manager inside a nix-toolbox container on Fedora Atomic Desktops, config files managed by Home Manager are symlinks pointing to `/nix/store/...` paths.
+The Nix store data lives on the host at `$XDG_DATA_HOME/nix/root/nix/` (or `$XDG_DATA_HOME/nix` for legacy installs), but the `/nix` bind mount only exists inside the container.
+Host-side programs like sway, waybar, foot, or firefox cannot resolve these symlinks — the config files are effectively invisible to the host.
+
+The `hostConfig` Home Manager module solves this by materializing symlinks as real files after each `home-manager switch`.
</code_context>
<issue_to_address>
**nitpick (typo):** Consider capitalizing “Firefox” to match its usual spelling.
Here, only “Firefox” should be capitalized to match standard usage, while the other program names remain lowercase.
```suggestion
Host-side programs like sway, waybar, foot, or Firefox cannot resolve these symlinks — the config files are effectively invisible to the host.
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Use `$XDG_DATA_HOME/nix/root/nix/` as the default bind mount source for new installs, matching the path used by upstream Nix's user namespace sandboxing (e.g. Fedora's `nix-core` package). Existing installs with a store directly in `$XDG_DATA_HOME/nix/` are auto-detected (by checking for `$XDG_DATA_HOME/nix/store`) and preserved. This enables sharing the Nix store between nix-toolbox and host Nix installations that use user namespace sandboxing. Closes thrix#39 Assisted-by: Claude Code Signed-off-by: thrix-bot <thrix-bot@users.noreply.github.com>
for more information, see https://pre-commit.ci
The variable holds the path to the Nix root directory (containing `store/`, `var/`, etc.), not just the store. `NIX_ROOT_DIR` better reflects its purpose as the bind mount source for `/nix`. Assisted-by: Claude Code Signed-off-by: thrix-bot <thrix-bot@users.noreply.github.com>
98d23e9 to
d6c28fa
Compare
Owner
|
/test |
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
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
$XDG_DATA_HOME/nix/root/nix/as the default bind mount source for new installs, matching the path used by upstream Nix's user namespace sandboxing (e.g. Fedora'snix-corepackage)$XDG_DATA_HOME/nix/are auto-detected (by checking for$XDG_DATA_HOME/nix/store) and preservedCloses #39
Assisted-by: Claude Code
Summary by Sourcery
Align the toolbox Nix store location with upstream user-namespace Nix defaults while preserving existing installs and document the new host integration model.
New Features:
Enhancements:
Documentation: