tests.nixpkgs-check-by-name: Enforce pkgs/by-name for new packages
This commit is contained in:
Silvan Mosberger
parent
69fc71a82c
commit
57afdc545b
34 changed files with 243 additions and 14 deletions
|
|
@ -1,11 +1,9 @@
|
|||
# Name-based package directories
|
||||
|
||||
The structure of this directory maps almost directly to top-level package attributes.
|
||||
This is the recommended way to add new top-level packages to Nixpkgs [when possible](#limitations).
|
||||
Add new top-level packages to Nixpkgs using this mechanism [whenever possible](#limitations).
|
||||
|
||||
Packages found in the named-based structure do not need to be explicitly added to the
|
||||
`top-level/all-packages.nix` file unless they require overriding the default value
|
||||
of an implicit attribute (see below).
|
||||
Packages found in the name-based structure are automatically included, without needing to be added to `all-packages.nix`. However if the implicit attribute defaults need to be changed for a package, this [must still be declared in `all-packages.nix`](#changing-implicit-attribute-defaults).
|
||||
|
||||
## Example
|
||||
|
||||
|
|
|
|||
|
|
@ -45,6 +45,8 @@ The current ratchets are:
|
|||
|
||||
- New manual definitions of `pkgs.${name}` (e.g. in `pkgs/top-level/all-packages.nix`) with `args = { }`
|
||||
(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.
|
||||
|
||||
## Development
|
||||
|
||||
|
|
|
|||
|
|
@ -93,10 +93,18 @@ let
|
|||
# Information on all attributes that exist but are not in pkgs/by-name.
|
||||
# We need this to enforce pkgs/by-name for new packages
|
||||
nonByNameAttrs = map (name:
|
||||
let
|
||||
output = attrInfo name pkgs.${name};
|
||||
result = builtins.tryEval (builtins.deepSeq output null);
|
||||
in
|
||||
[
|
||||
name
|
||||
{
|
||||
NonByName = null;
|
||||
NonByName =
|
||||
if result.success then
|
||||
{ EvalSuccess = output; }
|
||||
else
|
||||
{ EvalFailure = null; };
|
||||
}
|
||||
]
|
||||
) (builtins.attrNames (builtins.removeAttrs pkgs attrs));
|
||||
|
|
|
|||
|
|
@ -16,7 +16,14 @@ enum Attribute {
|
|||
/// An attribute that should be defined via pkgs/by-name
|
||||
ByName(ByNameAttribute),
|
||||
/// An attribute not defined via pkgs/by-name
|
||||
NonByName,
|
||||
NonByName(NonByNameAttribute),
|
||||
}
|
||||
|
||||
#[derive(Deserialize)]
|
||||
enum NonByNameAttribute {
|
||||
/// The attribute doesn't evaluate
|
||||
EvalFailure,
|
||||
EvalSuccess(AttributeInfo),
|
||||
}
|
||||
|
||||
#[derive(Deserialize)]
|
||||
|
|
@ -145,11 +152,63 @@ pub fn check_values(
|
|||
use AttributeInfo::*;
|
||||
use ByNameAttribute::*;
|
||||
use CallPackageVariant::*;
|
||||
use NonByNameAttribute::*;
|
||||
|
||||
let check_result = match attribute_value {
|
||||
NonByName => Success(ratchet::Package {
|
||||
empty_non_auto_called: Tight,
|
||||
}),
|
||||
// The attribute succeeds evaluation and is NOT defined in pkgs/by-name
|
||||
NonByName(EvalSuccess(attribute_info)) => {
|
||||
let uses_by_name = match attribute_info {
|
||||
// In these cases the package doesn't qualify for being in pkgs/by-name,
|
||||
// so the UsesByName ratchet is already as tight as it can be
|
||||
NonAttributeSet => Success(Tight),
|
||||
NonCallPackage => Success(Tight),
|
||||
// This is an odd case when _internalCallByNamePackageFile is used to define a package.
|
||||
CallPackage(CallPackageInfo {
|
||||
call_package_variant: Auto,
|
||||
..
|
||||
}) => NixpkgsProblem::InternalCallPackageUsed {
|
||||
attr_name: attribute_name.clone(),
|
||||
}
|
||||
.into(),
|
||||
// Only derivations can be in pkgs/by-name,
|
||||
// so this attribute doesn't qualify
|
||||
CallPackage(CallPackageInfo {
|
||||
is_derivation: false,
|
||||
..
|
||||
}) => Success(Tight),
|
||||
|
||||
// The case of an attribute that qualifies:
|
||||
// - Uses callPackage
|
||||
// - Is a derivation
|
||||
CallPackage(CallPackageInfo {
|
||||
is_derivation: true,
|
||||
call_package_variant: Manual { path, empty_arg },
|
||||
}) => Success(Loose(ratchet::UsesByName {
|
||||
call_package_path: path,
|
||||
empty_arg,
|
||||
})),
|
||||
};
|
||||
uses_by_name.map(|x| ratchet::Package {
|
||||
empty_non_auto_called: Tight,
|
||||
uses_by_name: x,
|
||||
})
|
||||
}
|
||||
NonByName(EvalFailure) => {
|
||||
// This is a bit of an odd case: We don't even _know_ whether this attribute
|
||||
// would qualify for using pkgs/by-name. We can either:
|
||||
// - Assume it's not using pkgs/by-name, which has the problem that if a
|
||||
// package evaluation gets broken temporarily, the fix can remove it from
|
||||
// pkgs/by-name again
|
||||
// - Assume it's using pkgs/by-name already, which has the problem that if a
|
||||
// package evaluation gets broken temporarily, fixing it requires a move to
|
||||
// pkgs/by-name
|
||||
// We choose the latter, since we want to move towards pkgs/by-name, not away
|
||||
// from it
|
||||
Success(ratchet::Package {
|
||||
empty_non_auto_called: Tight,
|
||||
uses_by_name: Tight,
|
||||
})
|
||||
}
|
||||
ByName(Missing) => NixpkgsProblem::UndefinedAttr {
|
||||
relative_package_file: relative_package_file.clone(),
|
||||
package_name: attribute_name.clone(),
|
||||
|
|
@ -182,6 +241,7 @@ pub fn check_values(
|
|||
check_result.and(match &call_package_variant {
|
||||
Auto => Success(ratchet::Package {
|
||||
empty_non_auto_called: Tight,
|
||||
uses_by_name: Tight,
|
||||
}),
|
||||
Manual { path, empty_arg } => {
|
||||
let correct_file = if let Some(call_package_path) = path {
|
||||
|
|
@ -198,6 +258,7 @@ pub fn check_values(
|
|||
} else {
|
||||
Tight
|
||||
},
|
||||
uses_by_name: Tight,
|
||||
})
|
||||
} else {
|
||||
NixpkgsProblem::WrongCallPackage {
|
||||
|
|
|
|||
|
|
@ -1,3 +1,4 @@
|
|||
use crate::structure;
|
||||
use crate::utils::PACKAGE_NIX_FILENAME;
|
||||
use rnix::parser::ParseError;
|
||||
use std::ffi::OsString;
|
||||
|
|
@ -87,6 +88,19 @@ pub enum NixpkgsProblem {
|
|||
text: String,
|
||||
io_error: io::Error,
|
||||
},
|
||||
InternalCallPackageUsed {
|
||||
attr_name: String,
|
||||
},
|
||||
MovedOutOfByName {
|
||||
package_name: String,
|
||||
call_package_path: Option<PathBuf>,
|
||||
empty_arg: bool,
|
||||
},
|
||||
NewPackageNotUsingByName {
|
||||
package_name: String,
|
||||
call_package_path: Option<PathBuf>,
|
||||
empty_arg: bool,
|
||||
},
|
||||
}
|
||||
|
||||
impl fmt::Display for NixpkgsProblem {
|
||||
|
|
@ -213,6 +227,53 @@ impl fmt::Display for NixpkgsProblem {
|
|||
subpath.display(),
|
||||
text,
|
||||
),
|
||||
NixpkgsProblem::InternalCallPackageUsed { attr_name } =>
|
||||
write!(
|
||||
f,
|
||||
"pkgs.{attr_name}: This attribute is defined using `_internalCallByNamePackageFile`, which is an internal function not intended for manual use.",
|
||||
),
|
||||
NixpkgsProblem::MovedOutOfByName { package_name, call_package_path, empty_arg } => {
|
||||
let call_package_arg =
|
||||
if let Some(path) = &call_package_path {
|
||||
format!("./{}", path.display())
|
||||
} else {
|
||||
"...".into()
|
||||
};
|
||||
if *empty_arg {
|
||||
write!(
|
||||
f,
|
||||
"pkgs.{package_name}: This top-level package was previously defined in {}, but is now manually defined as `callPackage {call_package_arg} {{ }}` (e.g. in `pkgs/top-level/all-packages.nix`). Please move the package back and remove the manual `callPackage`.",
|
||||
structure::relative_file_for_package(package_name).display(),
|
||||
)
|
||||
} else {
|
||||
// This can happen if users mistakenly assume that for custom arguments,
|
||||
// pkgs/by-name can't be used.
|
||||
write!(
|
||||
f,
|
||||
"pkgs.{package_name}: This top-level package was previously defined in {}, but is now manually defined as `callPackage {call_package_arg} {{ ... }}` (e.g. in `pkgs/top-level/all-packages.nix`). While the manual `callPackage` is still needed, it's not necessary to move the package files.",
|
||||
structure::relative_file_for_package(package_name).display(),
|
||||
)
|
||||
}
|
||||
},
|
||||
NixpkgsProblem::NewPackageNotUsingByName { package_name, call_package_path, empty_arg } => {
|
||||
let call_package_arg =
|
||||
if let Some(path) = &call_package_path {
|
||||
format!("./{}", path.display())
|
||||
} else {
|
||||
"...".into()
|
||||
};
|
||||
let extra =
|
||||
if *empty_arg {
|
||||
"Since the second `callPackage` argument is `{ }`, no manual `callPackage` (e.g. in `pkgs/top-level/all-packages.nix`) is needed anymore."
|
||||
} else {
|
||||
"Since the second `callPackage` argument is not `{ }`, the manual `callPackage` (e.g. in `pkgs/top-level/all-packages.nix`) is still needed."
|
||||
};
|
||||
write!(
|
||||
f,
|
||||
"pkgs.{package_name}: This is a new top-level package of the form `callPackage {call_package_arg} {{ }}`. Please define it in {} instead. See `pkgs/by-name/README.md` for more details. {extra}",
|
||||
structure::relative_file_for_package(package_name).display(),
|
||||
)
|
||||
},
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -6,6 +6,7 @@ use crate::nixpkgs_problem::NixpkgsProblem;
|
|||
use crate::structure;
|
||||
use crate::validation::{self, Validation, Validation::Success};
|
||||
use std::collections::HashMap;
|
||||
use std::path::PathBuf;
|
||||
|
||||
/// The ratchet value for the entirety of Nixpkgs.
|
||||
#[derive(Default)]
|
||||
|
|
@ -33,16 +34,26 @@ impl Nixpkgs {
|
|||
pub struct Package {
|
||||
/// The ratchet value for the check for non-auto-called empty arguments
|
||||
pub empty_non_auto_called: RatchetState<EmptyNonAutoCalled>,
|
||||
|
||||
/// The ratchet value for the check for new packages using pkgs/by-name
|
||||
pub uses_by_name: RatchetState<UsesByName>,
|
||||
}
|
||||
|
||||
impl Package {
|
||||
/// Validates the ratchet checks for a top-level package
|
||||
pub fn compare(name: &str, optional_from: Option<&Self>, to: &Self) -> Validation<()> {
|
||||
RatchetState::<EmptyNonAutoCalled>::compare(
|
||||
name,
|
||||
optional_from.map(|x| &x.empty_non_auto_called),
|
||||
&to.empty_non_auto_called,
|
||||
)
|
||||
validation::sequence_([
|
||||
RatchetState::<EmptyNonAutoCalled>::compare(
|
||||
name,
|
||||
optional_from.map(|x| &x.empty_non_auto_called),
|
||||
&to.empty_non_auto_called,
|
||||
),
|
||||
RatchetState::<UsesByName>::compare(
|
||||
name,
|
||||
optional_from.map(|x| &x.uses_by_name),
|
||||
&to.uses_by_name,
|
||||
),
|
||||
])
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -102,3 +113,34 @@ impl ToNixpkgsProblem for EmptyNonAutoCalled {
|
|||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// The ratchet value of an attribute
|
||||
/// for the check that new packages use pkgs/by-name
|
||||
///
|
||||
/// This checks that all new package defined using callPackage must be defined via pkgs/by-name
|
||||
/// It also checks that once a package uses pkgs/by-name, it can't switch back to all-packages.nix
|
||||
#[derive(Clone)]
|
||||
pub struct UsesByName {
|
||||
/// The first callPackage argument, used for better errors
|
||||
pub call_package_path: Option<PathBuf>,
|
||||
/// Whether the second callPackage argument is empty, used for better errors
|
||||
pub empty_arg: bool,
|
||||
}
|
||||
|
||||
impl ToNixpkgsProblem for UsesByName {
|
||||
fn to_nixpkgs_problem(name: &str, a: &Self, existed_before: bool) -> NixpkgsProblem {
|
||||
if existed_before {
|
||||
NixpkgsProblem::MovedOutOfByName {
|
||||
package_name: name.to_owned(),
|
||||
call_package_path: a.call_package_path.clone(),
|
||||
empty_arg: a.empty_arg,
|
||||
}
|
||||
} else {
|
||||
NixpkgsProblem::NewPackageNotUsingByName {
|
||||
package_name: name.to_owned(),
|
||||
call_package_path: a.call_package_path.clone(),
|
||||
empty_arg: a.empty_arg,
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -0,0 +1,3 @@
|
|||
self: super: {
|
||||
foo = self._internalCallByNamePackageFile ./foo.nix;
|
||||
}
|
||||
|
|
@ -0,0 +1 @@
|
|||
import ../mock-nixpkgs.nix { root = ./.; }
|
||||
|
|
@ -0,0 +1 @@
|
|||
pkgs.foo: This attribute is defined using `_internalCallByNamePackageFile`, which is an internal function not intended for manual use.
|
||||
|
|
@ -0,0 +1 @@
|
|||
{ someDrv }: someDrv
|
||||
|
|
@ -0,0 +1,10 @@
|
|||
self: super: {
|
||||
foo1 = self.callPackage ({ someDrv }: someDrv) { };
|
||||
foo2 = self.callPackage ./without-config.nix { };
|
||||
foo3 = self.callPackage ({ someDrv, enableFoo }: someDrv) {
|
||||
enableFoo = null;
|
||||
};
|
||||
foo4 = self.callPackage ./with-config.nix {
|
||||
enableFoo = null;
|
||||
};
|
||||
}
|
||||
|
|
@ -0,0 +1 @@
|
|||
import ../../mock-nixpkgs.nix { root = ./.; }
|
||||
|
|
@ -0,0 +1 @@
|
|||
{ someDrv }: someDrv
|
||||
|
|
@ -0,0 +1 @@
|
|||
{ someDrv }: someDrv
|
||||
|
|
@ -0,0 +1 @@
|
|||
{ someDrv }: someDrv
|
||||
|
|
@ -0,0 +1 @@
|
|||
{ someDrv }: someDrv
|
||||
|
|
@ -0,0 +1 @@
|
|||
import ../mock-nixpkgs.nix { root = ./.; }
|
||||
|
|
@ -0,0 +1,4 @@
|
|||
pkgs.foo1: This top-level package was previously defined in pkgs/by-name/fo/foo1/package.nix, but is now manually defined as `callPackage ... { }` (e.g. in `pkgs/top-level/all-packages.nix`). Please move the package back and remove the manual `callPackage`.
|
||||
pkgs.foo2: This top-level package was previously defined in pkgs/by-name/fo/foo2/package.nix, but is now manually defined as `callPackage ./without-config.nix { }` (e.g. in `pkgs/top-level/all-packages.nix`). Please move the package back and remove the manual `callPackage`.
|
||||
pkgs.foo3: This top-level package was previously defined in pkgs/by-name/fo/foo3/package.nix, but is now manually defined as `callPackage ... { ... }` (e.g. in `pkgs/top-level/all-packages.nix`). While the manual `callPackage` is still needed, it's not necessary to move the package files.
|
||||
pkgs.foo4: This top-level package was previously defined in pkgs/by-name/fo/foo4/package.nix, but is now manually defined as `callPackage ./with-config.nix { ... }` (e.g. in `pkgs/top-level/all-packages.nix`). While the manual `callPackage` is still needed, it's not necessary to move the package files.
|
||||
|
|
@ -0,0 +1 @@
|
|||
{ someDrv, enableFoo }: someDrv
|
||||
|
|
@ -0,0 +1 @@
|
|||
{ someDrv }: someDrv
|
||||
|
|
@ -0,0 +1,11 @@
|
|||
self: super: {
|
||||
before = self.callPackage ({ someDrv }: someDrv) { };
|
||||
new1 = self.callPackage ({ someDrv }: someDrv) { };
|
||||
new2 = self.callPackage ./without-config.nix { };
|
||||
new3 = self.callPackage ({ someDrv, enableNew }: someDrv) {
|
||||
enableNew = null;
|
||||
};
|
||||
new4 = self.callPackage ./with-config.nix {
|
||||
enableNew = null;
|
||||
};
|
||||
}
|
||||
|
|
@ -0,0 +1,5 @@
|
|||
self: super: {
|
||||
|
||||
before = self.callPackage ({ someDrv }: someDrv) { };
|
||||
|
||||
}
|
||||
|
|
@ -0,0 +1 @@
|
|||
import ../../mock-nixpkgs.nix { root = ./.; }
|
||||
|
|
@ -0,0 +1 @@
|
|||
import ../mock-nixpkgs.nix { root = ./.; }
|
||||
|
|
@ -0,0 +1,4 @@
|
|||
pkgs.new1: This is a new top-level package of the form `callPackage ... { }`. Please define it in pkgs/by-name/ne/new1/package.nix instead. See `pkgs/by-name/README.md` for more details. Since the second `callPackage` argument is `{ }`, no manual `callPackage` (e.g. in `pkgs/top-level/all-packages.nix`) is needed anymore.
|
||||
pkgs.new2: This is a new top-level package of the form `callPackage ./without-config.nix { }`. Please define it in pkgs/by-name/ne/new2/package.nix instead. See `pkgs/by-name/README.md` for more details. Since the second `callPackage` argument is `{ }`, no manual `callPackage` (e.g. in `pkgs/top-level/all-packages.nix`) is needed anymore.
|
||||
pkgs.new3: This is a new top-level package of the form `callPackage ... { }`. Please define it in pkgs/by-name/ne/new3/package.nix instead. See `pkgs/by-name/README.md` for more details. Since the second `callPackage` argument is not `{ }`, the manual `callPackage` (e.g. in `pkgs/top-level/all-packages.nix`) is still needed.
|
||||
pkgs.new4: This is a new top-level package of the form `callPackage ./with-config.nix { }`. Please define it in pkgs/by-name/ne/new4/package.nix instead. See `pkgs/by-name/README.md` for more details. Since the second `callPackage` argument is not `{ }`, the manual `callPackage` (e.g. in `pkgs/top-level/all-packages.nix`) is still needed.
|
||||
|
|
@ -0,0 +1 @@
|
|||
{ someDrv, enableNew }: someDrv
|
||||
|
|
@ -0,0 +1 @@
|
|||
{ someDrv }: someDrv
|
||||
|
|
@ -0,0 +1,3 @@
|
|||
self: super: {
|
||||
iDontEval = throw "I don't eval";
|
||||
}
|
||||
|
|
@ -0,0 +1 @@
|
|||
import ../mock-nixpkgs.nix { root = ./.; }
|
||||
|
|
@ -0,0 +1 @@
|
|||
{ someDrv }: someDrv
|
||||
Loading…
Reference in a new issue