feature: Flake check for detSys installed nix by niksingh710 · Pull Request #473 · juspay/omnix

niksingh710 · 2025-08-12T08:00:18Z

Resolves: #472 (based on the approach 2 mentioned in #472)

On MacOs virtual Machine

On Ubuntu virtual Machine (Simple Nix installation)

Commands used to test the package for backward compatibility.

Run Version Parsing Tests: cargo test --package nix_rs test_parse_nix_version
Run Flake Detection Tests: cargo test --package omnix-health flake_enabled
Test Core Packages cargo test --package nix_rs --package omnix-health
Build and Test Health Command cargo build --bin om
Verify JSON Output ./target/debug/om health --json

Backward Compatibility

✅ No breaking changes - all existing functionality preserved
✅ Regular Nix installations continue to work as before
✅ Adds new capability without removing existing features

Expected Behavior

DetSys Nix users: Health check shows "✅ Flakes Enabled" with message "Flakes enabled via Determinate Systems Nix"
Regular Nix users: Existing behavior unchanged - checks experimental-features config

niksingh710 · 2025-08-12T08:19:15Z

For minimal changes diff, I guess we can drop the test cases that I got implemented by llm juspay/vertex in commit e6648a3
(the project juspay/vertex is impressive and makes writing test cases and stuff ease)

Copilot

Pull Request Overview

This PR adds support for detecting Determinate Systems Nix installations in the flake enabled health check. It automatically considers flakes enabled for DetSys Nix installations without requiring explicit configuration.

Modified flake enabled check to detect DetSys installations and treat them as having flakes enabled by default
Updated NixVersion struct to include an is_detsys field and enhanced version parsing to detect DetSys installations
Added comprehensive tests covering both DetSys and regular Nix scenarios

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File	Description
crates/omnix-health/src/check/flake_enabled.rs	Enhanced flake enabled check logic to handle DetSys installations and added comprehensive test coverage
crates/nix_rs/src/version.rs	Added `is_detsys` field to NixVersion struct and updated parsing logic to detect Determinate Systems installations
crates/nix_rs/src/version_spec.rs	Updated NixVersionSpec tests to include the new `is_detsys` field
crates/nix_rs/src/config.rs	Updated static NixVersion constant to include the new `is_detsys` field

crates/nix_rs/src/version.rs

srid · 2025-08-12T14:52:29Z

crates/nix_rs/src/version.rs

    pub patch: u32,
+    /// Nix installation type
+    pub installation_type: NixInstallationType,
 }


What effect does this have on comparing two NixVersion with same version but different installation type?

For example:

- NixVersion { major: 2, minor: 13, patch: 0, installation_type: Official } - NixVersion { major: 2, minor: 13, patch: 0, installation_type: DeterminateSystems }

The above two are no longer equal because their installation_type values differ.

For ordering (>, >=, <, <=):

Rust’s derived PartialOrd compares fields in declaration order.

It compares major, then minor, then patch, and finally installation_type.

Since Official is declared before DeterminateSystems in the enum, Official < DeterminateSystems.

I believe this behavior is fine, which is why I haven’t implemented custom PartialOrd or Eq for NixVersion comparisons. If you think it’s necessary, I can implement them as well.

@srid: What effect does this have [..]
...
@niksingh710 [..] The above two are no longer equal because their installation_type values differ.

Does it not have the effect of breaking the nix_version check?

I tested this on both a virtual device and my personal system with different versions in the yaml file, and confirmed that the nix_version check works correctly, since Rust’s PartialOrd handles it well.

How exhaustive was your test? Can you read the code around:

omnix/crates/nix_rs/src/version_spec.rs

Line 75 in 99ddd5a

NixVersionSpec::Gt(v) => version > v,

And see how the > check can fail or pass depending on the installation_type field?

The presence of installation_type seems quite problematic to me in this regard. NixVersion means just that ... nix version (conceptually, it says nothing about the provider of that installation).

Perhaps the installer info should go in NixInfo

omnix/crates/nix_rs/src/info.rs

Lines 9 to 16 in d692e74

pub struct NixInfo {

/// Nix version string

pub nix_version: NixVersion,

/// nix.conf configuration

pub nix_config: NixConfig,

/// Environment in which Nix was installed

pub nix_env: NixEnv,

}

srid · 2025-08-12T14:53:05Z

crates/omnix-health/src/check/flake_enabled.rs

Is it possible to disable flakes on DetSys installer? And if so, does the PR behave correctly?

I did a bit of searching and will be doing a more thorough check on how to disable flakes in DetSys. So far, I haven’t seen any option in the installer to disable flakes. In the configuration, extra-experimental-features is being used to enable flakes.
However, removing flakes from there still kept flakes enabled.

does the PR behave correctly?

For now, if the installed nix is from Determinate Systems the health output is green for flakes.

srid · 2025-08-12T14:54:14Z

crates/omnix-health/src/check/flake_enabled.rs

Having two similar matches in the same function here sounds like a code smell

srid · 2025-08-12T14:55:48Z

crates/omnix-health/src/check/flake_enabled.rs

Why do you need all these irrelevant settings in the mock config? KISS and as small a code as possible.

srid · 2025-08-18T20:39:26Z

@niksingh710 For future reference, please do not use non-standard characters in your branch name (i.e., # in your fix#472), as that makes it impossible (or so it seems) to nix run the branch:

EDIT: nix run github:juspay/omnix/pull/473/head works

srid · 2025-08-18T20:45:11Z

crates/omnix-health/src/check/flake_enabled.rs

+                let flakes_enabled =
+                    val.contains(&"flakes".to_string()) && val.contains(&"nix-command".to_string());
+                if flakes_enabled {
+                    (
+                        flakes_enabled,
+                        format!("experimental-features = {}", val.join(" ")),
+                        CheckResult::Green,
+                    )
+                } else {
+                    (
+                        flakes_enabled,
+                        format!("experimental-features = {}", val.join(" ")),
+                        CheckResult::Red {
+                            msg: "Nix flakes are not enabled".into(),
+                            suggestion: "See https://nixos.wiki/wiki/Flakes#Enable_flakes".into(),
+                        },
+                    )
+                }
+            }


You should DRY this up.

srid · 2025-08-18T20:47:40Z

crates/omnix-health/src/check/flake_enabled.rs

Too verbose for my taste. Just mock the nix.conf file itself (keep it 1 line; for experimental-features), and then parse it.

The more code you introduce, the more PITA it can be to maintain.

srid · 2025-08-18T20:49:21Z

crates/omnix-health/src/check/flake_enabled.rs

Test only what matters. Why do we care about title equality here, for instance? Tests should be minimal.

srid · 2025-08-19T15:40:17Z

crates/omnix-health/src/check/flake_enabled.rs

+        let config_json = format!(
+            r#"{{
+            "experimental-features": {{ "value": [{}], "defaultValue": [], "description": "" }},
+            "cores": {{ "value": 1, "defaultValue": 1, "description": "" }},
+            "extra-platforms": {{ "value": [], "defaultValue": [], "description": "" }},
+            "flake-registry": {{ "value": "", "defaultValue": "", "description": "" }},
+            "max-jobs": {{ "value": 1, "defaultValue": 1, "description": "" }},
+            "substituters": {{ "value": [], "defaultValue": [], "description": "" }},
+            "system": {{ "value": "x86_64-linux", "defaultValue": "x86_64-linux", "description": "" }},
+            "trusted-users": {{ "value": [], "defaultValue": [], "description": "" }}
+        }}"#,


Just mock the nix.conf file itself (keep it 1 line; for experimental-features), and then parse it.

nix.conf is in INI format, not JSON, as you are mocking here.

niksingh710 · 2025-08-21T18:13:36Z

Closing in favour of #478

niksingh710 added 2 commits August 12, 2025 13:15

fix: Flake check for detSys installed nix

69cdd01

chore(test): Test cases added for the flake check

e6648a3

niksingh710 requested a review from Copilot August 12, 2025 08:28

Copilot AI reviewed Aug 12, 2025

View reviewed changes

crates/nix_rs/src/version.rs Outdated Show resolved Hide resolved

niksingh710 requested a review from srid August 12, 2025 08:29

refactor: Replace is_detsys boolean with NixInstallationType enum

12aa0cf

srid reviewed Aug 12, 2025

View reviewed changes

crates/nix_rs/src/version.rs Outdated Show resolved Hide resolved

srid reviewed Aug 12, 2025

View reviewed changes

refactor: Improve NixVersion parsing and flake detection rigor

c539eed

srid reviewed Aug 18, 2025

View reviewed changes

refactor: Improve FlakeEnabled check implementation

99ddd5a

srid reviewed Aug 19, 2025

View reviewed changes

niksingh710 mentioned this pull request Aug 21, 2025

fix(health): Flake check for detSys installed nix #478

Open

niksingh710 closed this Aug 21, 2025

	pub struct NixInfo {
	/// Nix version string
	pub nix_version: NixVersion,
	/// nix.conf configuration
	pub nix_config: NixConfig,
	/// Environment in which Nix was installed
	pub nix_env: NixEnv,
	}

Conversation

niksingh710 commented Aug 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

On MacOs virtual Machine

On Ubuntu virtual Machine (Simple Nix installation)

Commands used to test the package for backward compatibility.

Backward Compatibility

Expected Behavior

Uh oh!

niksingh710 commented Aug 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

srid Aug 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

srid Aug 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

srid commented Aug 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

srid Aug 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

niksingh710 commented Aug 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Comments

niksingh710 commented Aug 12, 2025 •

edited

Loading

niksingh710 commented Aug 12, 2025 •

edited

Loading

srid Aug 18, 2025 •

edited

Loading

srid Aug 19, 2025 •

edited

Loading

srid commented Aug 18, 2025 •

edited

Loading

srid Aug 18, 2025 •

edited

Loading

niksingh710 commented Aug 21, 2025 •

edited

Loading