Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 4 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -96,3 +96,7 @@ The current ratchets are:
(see [nix evaluation checks](#nix-evaluation-checks)) must not be introduced.
- New top-level packages defined using `pkgs.callPackage` must be defined with a package directory.
- Once a top-level package uses `pkgs/by-name`, it also can't be moved back out of it.
- New top-level packages must evaluate with `strictDeps = true`.
- Once a top-level package evaluates with `strictDeps = true`, it also can't regress to `false`.
- New top-level packages must evaluate with `__structuredAttrs = true`.
- Once a top-level package evaluates with `__structuredAttrs = true`, it also can't regress to `false`.
2 changes: 2 additions & 0 deletions src/eval.nix
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,8 @@ let
{
AttributeSet = {
is_derivation = pkgs.lib.isDerivation value;
strict_deps = value.strictDeps or false;
structured_attrs = value.__structuredAttrs or false;
definition_variant =
if !value ? _callPackageVariant then
{ ManualDefinition.is_semantic_call_package = false; }
Expand Down
273 changes: 165 additions & 108 deletions src/eval.rs

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion src/nix_file.rs
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ impl NixFile {
}

/// Information about `callPackage` arguments.
#[derive(Debug, PartialEq, Eq)]
#[derive(Clone, Debug, PartialEq, Eq)]
pub struct CallPackageArgumentInfo {
/// The relative path of the first argument, or `None` if it's not a path.
pub relative_path: Option<RelativePathBuf>,
Expand Down
26 changes: 26 additions & 0 deletions src/problem/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,10 @@ pub mod npv_160;
pub mod npv_161;
pub mod npv_162;
pub mod npv_163;
pub mod npv_164;
pub mod npv_165;
pub mod npv_166;
pub mod npv_167;

const WIKI_BASE_URL: &str = "https://github.com/NixOS/nixpkgs-vet/wiki";

Expand Down Expand Up @@ -134,6 +138,20 @@ pub enum Problem {
NewTopLevelPackageShouldBeByNameWithCustomArgument(
npv_163::NewTopLevelPackageShouldBeByNameWithCustomArgument,
),

/// NPV-164: new top-level package must enable strictDeps
NewTopLevelPackageMustEnableStrictDeps(npv_164::NewTopLevelPackageMustEnableStrictDeps),

/// NPV-165: top-level package disabled strictDeps
TopLevelPackageDisabledStrictDeps(npv_165::TopLevelPackageDisabledStrictDeps),

/// NPV-166: new top-level package must enable __structuredAttrs
NewTopLevelPackageMustEnableStructuredAttrs(
npv_166::NewTopLevelPackageMustEnableStructuredAttrs,
),

/// NPV-167: top-level package disabled __structuredAttrs
TopLevelPackageDisabledStructuredAttrs(npv_167::TopLevelPackageDisabledStructuredAttrs),
}

impl Problem {
Expand Down Expand Up @@ -170,6 +188,10 @@ impl Problem {
Self::TopLevelPackageMovedOutOfByNameWithCustomArguments(..) => "NPV-161",
Self::NewTopLevelPackageShouldBeByName(..) => "NPV-162",
Self::NewTopLevelPackageShouldBeByNameWithCustomArgument(..) => "NPV-163",
Self::NewTopLevelPackageMustEnableStrictDeps(..) => "NPV-164",
Self::TopLevelPackageDisabledStrictDeps(..) => "NPV-165",
Self::NewTopLevelPackageMustEnableStructuredAttrs(..) => "NPV-166",
Self::TopLevelPackageDisabledStructuredAttrs(..) => "NPV-167",
}
}

Expand Down Expand Up @@ -212,6 +234,10 @@ impl fmt::Display for Problem {
Self::TopLevelPackageMovedOutOfByNameWithCustomArguments(inner) => inner.fmt(f),
Self::NewTopLevelPackageShouldBeByName(inner) => inner.fmt(f),
Self::NewTopLevelPackageShouldBeByNameWithCustomArgument(inner) => inner.fmt(f),
Self::NewTopLevelPackageMustEnableStrictDeps(inner) => inner.fmt(f),
Self::TopLevelPackageDisabledStrictDeps(inner) => inner.fmt(f),
Self::NewTopLevelPackageMustEnableStructuredAttrs(inner) => inner.fmt(f),
Self::TopLevelPackageDisabledStructuredAttrs(inner) => inner.fmt(f),
}
}
}
Expand Down
26 changes: 26 additions & 0 deletions src/problem/npv_164.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
use std::fmt;

use derive_new::new;
use indoc::writedoc;
use relative_path::RelativePathBuf;

#[derive(Clone, new)]
pub struct NewTopLevelPackageMustEnableStrictDeps {
#[new(into)]
package_name: String,
#[new(into)]
file: RelativePathBuf,
}

impl fmt::Display for NewTopLevelPackageMustEnableStrictDeps {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
let Self { package_name, file } = self;
writedoc!(
f,
"
- Attribute `pkgs.{package_name}` is a new package with `strictDeps` unset or set to `false`.
Please enable `strictDeps = true;` in {file}.
",
)
}
}
26 changes: 26 additions & 0 deletions src/problem/npv_165.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
use std::fmt;

use derive_new::new;
use indoc::writedoc;
use relative_path::RelativePathBuf;

#[derive(Clone, new)]
pub struct TopLevelPackageDisabledStrictDeps {
#[new(into)]
package_name: String,
#[new(into)]
file: RelativePathBuf,
}

impl fmt::Display for TopLevelPackageDisabledStrictDeps {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
let Self { package_name, file } = self;
writedoc!(
f,
"
- Attribute `pkgs.{package_name}` previously evaluated with `strictDeps = true`, but now evaluates with `strictDeps = false`.
Please re-enable `strictDeps = true;` in {file}.
",
)
}
}
26 changes: 26 additions & 0 deletions src/problem/npv_166.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
use std::fmt;

use derive_new::new;
use indoc::writedoc;
use relative_path::RelativePathBuf;

#[derive(Clone, new)]
pub struct NewTopLevelPackageMustEnableStructuredAttrs {
#[new(into)]
package_name: String,
#[new(into)]
file: RelativePathBuf,
}

impl fmt::Display for NewTopLevelPackageMustEnableStructuredAttrs {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
let Self { package_name, file } = self;
writedoc!(
f,
"
- Attribute `pkgs.{package_name}` is a new package with `__structuredAttrs` unset or set to `false`.
Please enable `__structuredAttrs = true;` in {file}.
Comment on lines +21 to +22
Copy link
Copy Markdown

@doronbehar doronbehar Apr 12, 2026

Choose a reason for hiding this comment

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

I just encountered this CI message here:

https://github.com/NixOS/nixpkgs/actions/runs/24303117878/job/70959851952

The PR adds something similar to the following:

{
  python3Packages,
}:

python3Packages.toPythonApplication (
  python3Packages.optiland.override {
    enableGui = true;
  }
)

to pkgs/by-name/op/optiland/package.nix.

I am not sure how to set __structuredAttrs explicitly there. Could anyone here give a hand please? Maybe the check should be modified a bit to exclude such expressions?

Copy link
Copy Markdown
Member

@Eveeifyeve Eveeifyeve Apr 12, 2026

Choose a reason for hiding this comment

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

Does toPythonApplication implement __structuredAttrs? I would start there, if it doesn't, it would be best to make a pr.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

In this case wouldn't it be enough to set __structuredAttrs = true; on the base python3Packages.optiland package?

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

In this case wouldn't it be enough to set __structuredAttrs = true; on the base python3Packages.optiland package?

Well indeed it worked. I'm surprised though - I remember I noticed a while ago that Python packages are built with structuredAttrs enabled by default, as I always saw:

structuredAttrs is enabled

In the build logs. However I can't reproduce it now.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Now that you mention it... yes, that is indeed strange. I don't have time right now but I'll have to look into that.

",
)
}
}
26 changes: 26 additions & 0 deletions src/problem/npv_167.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
use std::fmt;

use derive_new::new;
use indoc::writedoc;
use relative_path::RelativePathBuf;

#[derive(Clone, new)]
pub struct TopLevelPackageDisabledStructuredAttrs {
#[new(into)]
package_name: String,
#[new(into)]
file: RelativePathBuf,
}

impl fmt::Display for TopLevelPackageDisabledStructuredAttrs {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
let Self { package_name, file } = self;
writedoc!(
f,
"
- Attribute `pkgs.{package_name}` previously evaluated with `__structuredAttrs = true`, but now evaluates with `__structuredAttrs = false`.
Please re-enable `__structuredAttrs = true;` in {file}.
",
)
}
}
80 changes: 79 additions & 1 deletion src/ratchet.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,17 @@
//!
//! Each type has a `compare` method that validates the ratchet checks for that item.

use std::marker::PhantomData;

use relative_path::RelativePath;
use std::collections::BTreeMap;

use relative_path::RelativePathBuf;

use crate::nix_file::CallPackageArgumentInfo;
use crate::problem::{Problem, npv_160, npv_161, npv_162, npv_163};
use crate::problem::{
Problem, npv_160, npv_161, npv_162, npv_163, npv_164, npv_165, npv_166, npv_167,
};
use crate::validation::{self, Validation, Validation::Success};

/// The ratchet value for the entirety of Nixpkgs.
Expand Down Expand Up @@ -42,6 +46,12 @@ pub struct Package {

/// The ratchet value for the check for new packages using pkgs/by-name
pub uses_by_name: RatchetState<UsesByName>,

/// The ratchet value for the check for enabling `strictDeps`.
pub strict_deps: RatchetState<StrictDeps>,

/// The ratchet value for the check for enabling `__structuredAttrs`.
pub structured_attrs: RatchetState<StructuredAttrs>,
}

impl Package {
Expand All @@ -58,6 +68,16 @@ impl Package {
optional_from.map(|x| &x.uses_by_name),
&to.uses_by_name,
),
RatchetState::<StrictDeps>::compare(
name,
optional_from.map(|x| &x.strict_deps),
&to.strict_deps,
),
RatchetState::<StructuredAttrs>::compare(
name,
optional_from.map(|x| &x.structured_attrs),
&to.structured_attrs,
),
])
}
}
Expand Down Expand Up @@ -191,3 +211,61 @@ impl ToProblem for UsesByName {
}
}
}

pub trait EnabledAttributeProblem {
fn introduced_problem(name: &str, file: RelativePathBuf) -> Problem;
fn regressed_problem(name: &str, file: RelativePathBuf) -> Problem;
}

/// The ratchet value of an evaluated boolean attribute that must be enabled for new packages and
/// must not regress to `false` once enabled.
pub struct EnabledAttribute<ProblemKind>(PhantomData<ProblemKind>);

impl<ProblemKind: EnabledAttributeProblem> ToProblem for EnabledAttribute<ProblemKind> {
type ToContext = RelativePathBuf;

fn to_problem(name: &str, optional_from: Option<()>, file: &Self::ToContext) -> Problem {
if optional_from.is_some() {
ProblemKind::regressed_problem(name, file.clone())
} else {
ProblemKind::introduced_problem(name, file.clone())
}
}
}

/// The ratchet value of an attribute for enabling `strictDeps`.
///
/// New packages must evaluate with `strictDeps = true` unless their package set already makes
/// that the default. Once a package evaluates with `strictDeps = true`, it must not regress.
pub type StrictDeps = EnabledAttribute<StrictDepsProblem>;

pub enum StrictDepsProblem {}

impl EnabledAttributeProblem for StrictDepsProblem {
fn introduced_problem(name: &str, file: RelativePathBuf) -> Problem {
npv_164::NewTopLevelPackageMustEnableStrictDeps::new(name, file).into()
}

fn regressed_problem(name: &str, file: RelativePathBuf) -> Problem {
npv_165::TopLevelPackageDisabledStrictDeps::new(name, file).into()
}
}

/// The ratchet value of an attribute for enabling `__structuredAttrs`.
///
/// New packages must evaluate with `__structuredAttrs = true` unless their package set already
/// makes that the default. Once a package evaluates with `__structuredAttrs = true`, it must not
/// regress.
pub type StructuredAttrs = EnabledAttribute<StructuredAttrsProblem>;

pub enum StructuredAttrsProblem {}

impl EnabledAttributeProblem for StructuredAttrsProblem {
fn introduced_problem(name: &str, file: RelativePathBuf) -> Problem {
npv_166::NewTopLevelPackageMustEnableStructuredAttrs::new(name, file).into()
}

fn regressed_problem(name: &str, file: RelativePathBuf) -> Problem {
npv_167::TopLevelPackageDisabledStructuredAttrs::new(name, file).into()
}
}
2 changes: 2 additions & 0 deletions tests/mock-nixpkgs.nix
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@ let
// lib.mapAttrs (name: value: value) {
someDrv = {
type = "derivation";
strictDeps = true;
__structuredAttrs = true;
};
};

Expand Down
4 changes: 4 additions & 0 deletions tests/strict-deps-new-false/expected
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
- Attribute `pkgs.foo` is a new package with `strictDeps` unset or set to `false`.
Please enable `strictDeps = true;` in pkgs/by-name/fo/foo/package.nix.
(https://github.com/NixOS/nixpkgs-vet/wiki/NPV-164)
This PR introduces additional instances of discouraged patterns as listed above. Please fix them before merging.
1 change: 1 addition & 0 deletions tests/strict-deps-new-false/main/default.nix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
import <test-nixpkgs> { root = ./.; }
1 change: 1 addition & 0 deletions tests/strict-deps-new-false/main/pkgs/by-name/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
# Test fixture
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
{ someDrv }: someDrv // { strictDeps = false; }
1 change: 1 addition & 0 deletions tests/strict-deps-new-true/expected
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Validated successfully
1 change: 1 addition & 0 deletions tests/strict-deps-new-true/main/default.nix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
import <test-nixpkgs> { root = ./.; }
1 change: 1 addition & 0 deletions tests/strict-deps-new-true/main/pkgs/by-name/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
# Test fixture
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
{ someDrv }: someDrv // { strictDeps = true; }
4 changes: 4 additions & 0 deletions tests/strict-deps-new-unset/expected
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
- Attribute `pkgs.foo` is a new package with `strictDeps` unset or set to `false`.
Please enable `strictDeps = true;` in pkgs/by-name/fo/foo/package.nix.
(https://github.com/NixOS/nixpkgs-vet/wiki/NPV-164)
This PR introduces additional instances of discouraged patterns as listed above. Please fix them before merging.
1 change: 1 addition & 0 deletions tests/strict-deps-new-unset/main/default.nix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
import <test-nixpkgs> { root = ./.; }
1 change: 1 addition & 0 deletions tests/strict-deps-new-unset/main/pkgs/by-name/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
# Test fixture
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
{ someDrv }: removeAttrs someDrv [ "strictDeps" ]
1 change: 1 addition & 0 deletions tests/strict-deps-regression/base/default.nix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
import <test-nixpkgs> { root = ./.; }
1 change: 1 addition & 0 deletions tests/strict-deps-regression/base/pkgs/by-name/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
# Test fixture
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
{ someDrv }: someDrv
4 changes: 4 additions & 0 deletions tests/strict-deps-regression/expected
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
- Attribute `pkgs.foo` previously evaluated with `strictDeps = true`, but now evaluates with `strictDeps = false`.
Please re-enable `strictDeps = true;` in pkgs/by-name/fo/foo/package.nix.
(https://github.com/NixOS/nixpkgs-vet/wiki/NPV-165)
This PR introduces additional instances of discouraged patterns as listed above. Please fix them before merging.
1 change: 1 addition & 0 deletions tests/strict-deps-regression/main/default.nix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
import <test-nixpkgs> { root = ./.; }
1 change: 1 addition & 0 deletions tests/strict-deps-regression/main/pkgs/by-name/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
# Test fixture
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
{ someDrv }: someDrv // { strictDeps = false; }
4 changes: 4 additions & 0 deletions tests/structured-attrs-new-false/expected
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
- Attribute `pkgs.foo` is a new package with `__structuredAttrs` unset or set to `false`.
Please enable `__structuredAttrs = true;` in pkgs/by-name/fo/foo/package.nix.
(https://github.com/NixOS/nixpkgs-vet/wiki/NPV-166)
This PR introduces additional instances of discouraged patterns as listed above. Please fix them before merging.
1 change: 1 addition & 0 deletions tests/structured-attrs-new-false/main/default.nix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
import <test-nixpkgs> { root = ./.; }
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
# Test fixture
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
{ someDrv }: someDrv // { __structuredAttrs = false; }
1 change: 1 addition & 0 deletions tests/structured-attrs-new-true/expected
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Validated successfully
1 change: 1 addition & 0 deletions tests/structured-attrs-new-true/main/default.nix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
import <test-nixpkgs> { root = ./.; }
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
# Test fixture
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
{ someDrv }: someDrv // { __structuredAttrs = true; }
4 changes: 4 additions & 0 deletions tests/structured-attrs-new-unset/expected
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
- Attribute `pkgs.foo` is a new package with `__structuredAttrs` unset or set to `false`.
Please enable `__structuredAttrs = true;` in pkgs/by-name/fo/foo/package.nix.
(https://github.com/NixOS/nixpkgs-vet/wiki/NPV-166)
This PR introduces additional instances of discouraged patterns as listed above. Please fix them before merging.
1 change: 1 addition & 0 deletions tests/structured-attrs-new-unset/main/default.nix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
import <test-nixpkgs> { root = ./.; }
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
# Test fixture
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
{ someDrv }: removeAttrs someDrv [ "__structuredAttrs" ]
1 change: 1 addition & 0 deletions tests/structured-attrs-regression/base/default.nix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
import <test-nixpkgs> { root = ./.; }
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
# Test fixture
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
{ someDrv }: someDrv
4 changes: 4 additions & 0 deletions tests/structured-attrs-regression/expected
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
- Attribute `pkgs.foo` previously evaluated with `__structuredAttrs = true`, but now evaluates with `__structuredAttrs = false`.
Please re-enable `__structuredAttrs = true;` in pkgs/by-name/fo/foo/package.nix.
(https://github.com/NixOS/nixpkgs-vet/wiki/NPV-167)
This PR introduces additional instances of discouraged patterns as listed above. Please fix them before merging.
1 change: 1 addition & 0 deletions tests/structured-attrs-regression/main/default.nix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
import <test-nixpkgs> { root = ./.; }
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
# Test fixture
Loading