Skip to content

feat: take into effect system-features #587

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

feat: take into effect system-features #587

Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
0de787f
feat: take into effect system-features
sabify Sep 17, 2025
ba68af9
fix: get target default system-features
sabify Sep 30, 2025
21e6b53
fix: correctly handle not provided system-features
sabify Sep 30, 2025
bb8b38c
fix: ssh command should fail if it couldn't set the system-features
sabify Sep 30, 2025
d9035b9
feat: simplify and unify writing in nix.conf
sabify Sep 30, 2025
c5ba1f1
fix: formatting
sabify Sep 30, 2025
454ad62
fix: make runSshNoTty fallible for system_features and escape bash va…
sabify Sep 30, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 57 additions & 0 deletions src/nixos-anywhere.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1048,6 +1048,63 @@ echo "extra-trusted-public-keys = ${trustedPublicKeys}" >> ~/.config/nix/nix.con
SSH
fi

# Get system-features with a specific cpu architecture from the machine and add them to the installer
if [[ -n ${flake} ]]; then
system_features=$(runSshNoTty -o ConnectTimeout=10 nix --extra-experimental-features 'nix-command' config show system-features)
Copy link
Member

@Mic92 Mic92 Sep 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why is the list of current system-features important? Should this not be overruled by what the user has specified in the machine?

Copy link
Author

@sabify sabify Sep 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sometimes you may not want to explicitly provide system-features and only want to configure gcc.arch, in that case the default target platform feature set needs to append gccarch-${cpu-arch} to it. If you don't do this and only set the gccarch feature, it will override (not append) the whole default features and the build will fail.

# Escape the bash variable for safe interpolation into Nix
system_features="$(printf '%s' "$system_features" | sed 's/\\/\\\\/g; s/"/\\"/g')"
# First, try to evaluate all nix settings from the flake in one go
Comment on lines +1052 to +1056
Copy link

@coderabbitai coderabbitai bot Sep 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Use consistent variable naming and add error handling for remote SSH command.

Two issues:

  1. Naming inconsistency: system_features uses snake_case, while the rest of the script uses camelCase (e.g., sshConnection, flakeAttr, nixConfContent).

  2. Missing error handling: Line 1040 will cause the script to abort under set -euo pipefail if the SSH command fails (e.g., network issue, nix not found on remote).

Apply this diff:

   if [[ -n ${flake} ]]; then
-    system_features=$(runSshNoTty -o ConnectTimeout=10 nix --extra-experimental-features 'nix-command' config show system-features)
+    systemFeatures=$(runSshNoTty -o ConnectTimeout=10 nix --extra-experimental-features 'nix-command' config show system-features 2>/dev/null || echo "")
     # Escape the bash variable for safe interpolation into Nix
-    system_features="$(printf '%s' "$system_features" | sed 's/\\/\\\\/g; s/"/\\"/g')"
+    escapedSystemFeatures="$(printf '%s' "$systemFeatures" | sed 's/\\/\\\\/g; s/"/\\"/g')"

Then update line 1055 to use the renamed variable:

         remoteFeatures = let
-            remoteFeaturesStr = \"${system_features}\";
+            remoteFeaturesStr = \"${escapedSystemFeatures}\";
📝 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.

Suggested change
if [[ -n ${flake} ]]; then
system_features=$(runSshNoTty -o ConnectTimeout=10 nix --extra-experimental-features 'nix-command' config show system-features)
# Escape the bash variable for safe interpolation into Nix
system_features="$(printf '%s' "$system_features" | sed 's/\\/\\\\/g; s/"/\\"/g')"
# First, try to evaluate all nix settings from the flake in one go
if [[ -n ${flake} ]]; then
# Fetch remote system-features, but never fail under set -euo pipefail
systemFeatures=$(runSshNoTty -o ConnectTimeout=10 \
nix --extra-experimental-features 'nix-command' config show system-features \
2>/dev/null || echo "")
# Escape the bash variable for safe interpolation into Nix
escapedSystemFeatures="$(printf '%s' "$systemFeatures" \
| sed 's/\\/\\\\/g; s/"/\\"/g')"
# First, try to evaluate all nix settings from the flake in one go
nixConfContent=$(
nix --extra-experimental-features 'nix-command flakes' eval --raw --apply "
config:
let
settings = config.nix.settings or {};
gccArch = config.nixpkgs.hostPlatform.gcc.arch or null;
# Check if system-features are defined in configuration
configFeatures = settings.system-features or null;
hasConfig = configFeatures != null && configFeatures != [];
# Fallback to the escaped remote features if none in the flake
remoteFeatures = let
remoteFeaturesStr = \"${escapedSystemFeatures}\";
remoteFeaturesList = if remoteFeaturesStr != \"\" then
builtins.filter (x: builtins.isString x && x != \"\")
(builtins.split \" +\" remoteFeaturesStr)
else
[];
in remoteFeaturesList;
in
# … rest of the nixConfContent construction …
" "${flake}#${flakeAttr}"
)
🤖 Prompt for AI Agents 
In src/nixos-anywhere.sh around lines 1039-1043, rename the snake_case variable
system_features to camelCase systemFeatures for consistency with
sshConnection/flakeAttr/nixConfContent, and preserve the escaping step for
quotes/backslashes; additionally, run the remote SSH command with explicit error
handling: capture both its stdout and exit code, and if the SSH command fails
log a clear error including the exit code/output and exit non-zero to avoid an
unhelpful abrupt failure under set -euo pipefail. After renaming, update the
later usage on line 1055 to reference systemFeatures.

nixConfContent=$(nix --extra-experimental-features 'nix-command flakes' eval --raw --apply "
config:
let
settings = config.nix.settings or {};
gccArch = config.nixpkgs.hostPlatform.gcc.arch or null;

# Check if system-features are defined in configuration
configFeatures = settings.system-features or null;
hasConfigFeatures = configFeatures != null && configFeatures != [];

remoteFeatures = let
remoteFeaturesStr = \"${system_features}\";
# Parse remote features string (space-separated) into list
remoteFeaturesList = if remoteFeaturesStr != \"\" then
builtins.filter (x: builtins.isString x && x != \"\") (builtins.split \" +\" remoteFeaturesStr)
else [];
in remoteFeaturesList;

# Combine base features (config or remote) with platform-specific features
baseFeatures = if hasConfigFeatures then configFeatures else remoteFeatures;
# At least one of nix.settings.system-features or nixpkgs.hostPlatform.gcc.arch has been explicitly defined
allFeatures = if (gccArch != null) || hasConfigFeatures then baseFeatures ++ (if gccArch != null then [\"gccarch-\${gccArch}\"] else []) else [];

# Deduplicate using listToAttrs trick
uniqueFeatures = builtins.attrNames (builtins.listToAttrs (map (f: { name = f; value = true; }) allFeatures));

substituters = builtins.toString (settings.substituters or []);
trustedPublicKeys = builtins.toString (settings.trusted-public-keys or []);
systemFeatures = builtins.toString uniqueFeatures;

# Helper function for optional config lines
optionalLine = cond: line: if cond then line + \"\n\" else \"\";
useSubstituters = \"${machineSubstituters}\" == \"y\";
in
optionalLine (useSubstituters && substituters != \"\") \"extra-substituters = \${substituters}\"
+ optionalLine (useSubstituters && trustedPublicKeys != \"\") \"extra-trusted-public-keys = \${trustedPublicKeys}\"
+ optionalLine (systemFeatures != \"\") \"system-features = \${systemFeatures}\"
" "${flake}#${flakeAttr}")

# Write to nix.conf if we have any content
if [[ -n ${nixConfContent} ]]; then
runSsh sh <<SSH
mkdir -p ~/.config/nix
printf '%s\n' "\$(cat <<'CONTENT'
${nixConfContent}
CONTENT
)" >> ~/.config/nix/nix.conf
SSH
fi
fi

if [[ ${phases[disko]} == 1 ]]; then
runDisko "$diskoScript"
fi
Expand Down
Loading