tree-wide: Replace unsupported dontCheck with doCheck attribute #307838
Conversation
|
I believe this is a serious inconsistency in Nixpkgs. |
|
In your mind should both be supported, e.g. one being a "negating alias" of the other? |
|
In my mind only the But, in the name of backwards compatibility, your idea of "negating alias" is good. |
|
If |
|
Reading through |
|
I have few to nil idea. Maybe @Ericson2314 should know. |
The most typical variant is
The same for all the other |
|
I found another interesting tidbit in relation to I'd like to look into creating a "negating alias" for all the |
|
No objections, only recommendations! |
There was a problem hiding this comment.
Looks like my wording was not clear, here are the changes I've described.
diff --git a/pkgs/development/python-modules/gpt-2-simple/default.nix b/pkgs/development/python-modules/gpt-2-simple/default.nix
index aae6dfbfbc40..ef19eb0ba1bb 100644
--- a/pkgs/development/python-modules/gpt-2-simple/default.nix
+++ b/pkgs/development/python-modules/gpt-2-simple/default.nix
@@ -1,10 +1,10 @@
-{ lib, buildPythonPackage, fetchFromGitHub, regex, requests, tqdm, numpy
+{ lib, buildPythonPackage, fetchFromGitHub, setuptools, regex, requests, tqdm, numpy
, toposort, tensorflow }:
buildPythonPackage rec {
pname = "gpt-2-simple";
version = "0.8.1";
- format = "setuptools";
+ pyproject = true;
src = fetchFromGitHub {
owner = "minimaxir";
@@ -13,9 +13,9 @@ buildPythonPackage rec {
hash = "sha256-WwD4sDcc28zXEOISJsq8e+rgaNrrgIy79Wa4J3E7Ovc=";
};
- propagatedBuildInputs = [ regex requests tqdm numpy toposort tensorflow ];
+ build-system = [ setuptools ];
- dontCheck = true; # no tests in upstream
+ dependencies = [ regex requests tqdm numpy toposort tensorflow ];
meta = with lib; {
description =
diff --git a/pkgs/development/python-modules/kaitaistruct/default.nix b/pkgs/development/python-modules/kaitaistruct/default.nix
index 71ad13fadb55..6d30a4e03dd4 100644
--- a/pkgs/development/python-modules/kaitaistruct/default.nix
+++ b/pkgs/development/python-modules/kaitaistruct/default.nix
@@ -2,6 +2,7 @@
, buildPythonPackage
, fetchPypi
, fetchFromGitHub
+, setuptools
, brotli
, lz4
}:
@@ -17,7 +18,7 @@ in
buildPythonPackage rec {
pname = "kaitaistruct";
version = "0.10";
- format = "setuptools";
+ pyproject = true;
src = fetchPypi {
inherit pname version;
@@ -29,14 +30,13 @@ buildPythonPackage rec {
sed '32ipackages = kaitai/compress' -i setup.cfg
'';
- propagatedBuildInputs = [
+ build-system = [ setuptools ];
+
+ dependencies = [
brotli
lz4
];
- # no tests
- dontCheck = true;
-
pythonImportsCheck = [
"kaitaistruct"
"kaitai.compress"This works, because when using pyproject = true setuptoolsCheckHook is not automatically added, so testing isn't enabled.
I also recommend changing the naming of the dependency lists to the newer ones, like shown in the diff.
|
Thank you for your feedback, @TomaSajt, very much appreciated. I may not have read your previous comment as thorough as I should have before making more changes to this PR. Hopefully I've now paid closer attention—although it is getting a bit late in the evening for me 🥱 |
|
Thanks for reaching out to the python matrix chat, @TomaSajt, hopefully I've understood you correctly (see latest changes) |
afh
added
the
12.approvals: 2
|
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: https://discourse.nixos.org/t/prs-already-reviewed/2617/1643 |
wegank
removed
the
12.approvals: 2
|
Personal opinion: |
|
I have too much on my plate and cannot review these changes this week. |
|
I've created separate commits as preferred, @AndersonTorres. Thanks for the heads up, @ShamrockLee. |
afh
changed the title
wegank
added
12.approvals: 2
and replace unsupported dontCheck with doCheck attribute
and replace unsupported dontCheck with doCheck attribute
ofborg
bot
removed
the
2.status: merge conflict
|
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: https://discourse.nixos.org/t/prs-already-reviewed/2617/1701 |
|
Result of 28 packages built:
|
|
Thanks everyone for your helpful feedback on this and helping getting it merged, very much appreciated 🙏 |
Description of changes
Remove unsupported
dontCheckattribute (for context see this comment on #293498).ℹ️ Since
python3Module.gpt-2-simpleis broken onaarch64-darwinI was not able to test it locally; yet the GitHub tests indicate that the changes proposed in this PR do not affect it negatively.Things done
nix.conf? (See Nix manual)sandbox = relaxedsandbox = truenix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/)Add a 👍 reaction to pull requests you find important.