From df284fa43c9365e1e03b08cda09b5e3ff83e2cfc Mon Sep 17 00:00:00 2001 From: Wolfgang Walther Date: Sat, 10 Feb 2024 15:15:56 +0100 Subject: haskellPackages: avoid re-enabling previously disabled tests The intent of all doCheck = , where condition is possibly true, is to disable the tests in a specific case. However, as currently written, this also has the effect of re-enabling the tests, even if they have been disabled by an override before, e.g. to mkDerivation. This also affects the default value given in mkDerivation, which is !isCross. Before this change, aeson for example, would have been built with tests when cross-compiling, which was not intended. The proper way is to set the doCheck = false attribute only conditionally, and otherwise rely on a previous override or the default value given in mkDerivation. --- .../haskell-modules/configuration-common.nix | 43 ++++++++++++---------- 1 file changed, 23 insertions(+), 20 deletions(-) (limited to 'pkgs/development/haskell-modules/configuration-common.nix') diff --git a/pkgs/development/haskell-modules/configuration-common.nix b/pkgs/development/haskell-modules/configuration-common.nix index ccb2a145effe..64b388e5e57a 100644 --- a/pkgs/development/haskell-modules/configuration-common.nix +++ b/pkgs/development/haskell-modules/configuration-common.nix @@ -145,11 +145,11 @@ self: super: { czipwith = doJailbreak super.czipwith; # Deal with infinite and NaN values generated by QuickCheck-2.14.3 - aeson = overrideCabal { + aeson = overrideCabal (lib.optionalAttrs pkgs.stdenv.hostPlatform.is32bit { # aeson's test suite includes some tests with big numbers that fail on 32bit # https://github.com/haskell/aeson/issues/1060 - doCheck = !pkgs.stdenv.hostPlatform.is32bit; - } (appendPatches [ + doCheck = false; + }) (appendPatches [ (pkgs.fetchpatch { name = "aeson-quickcheck-2.14.3-double-workaround.patch"; url = "https://github.com/haskell/aeson/commit/58766a1916b4980792763bab74f0c86e2a7ebf20.patch"; @@ -1320,9 +1320,10 @@ self: super: { # Requires pg_ctl command during tests beam-postgres = overrideCabal (drv: { - # https://github.com/NixOS/nixpkgs/issues/198495 - doCheck = pkgs.postgresql.doCheck; testToolDepends = (drv.testToolDepends or []) ++ [pkgs.postgresql]; + } // lib.optionalAttrs (!pkgs.postgresql.doCheck) { + # https://github.com/NixOS/nixpkgs/issues/198495 + doCheck = false; }) super.beam-postgres; # PortMidi needs an environment variable to have ALSA find its plugins: @@ -1364,8 +1365,6 @@ self: super: { sed -i test/PostgreSQL/Test.hs \ -e s^host=localhost^^ ''; - # https://github.com/NixOS/nixpkgs/issues/198495 - doCheck = pkgs.postgresql.doCheck; # Match the test suite defaults (or hardcoded values?) preCheck = drv.preCheck or "" + '' PGUSER=esqutest @@ -1379,6 +1378,9 @@ self: super: { pkgs.postgresql pkgs.postgresqlTestHook ]; + } // lib.optionalAttrs (!pkgs.postgresql.doCheck) { + # https://github.com/NixOS/nixpkgs/issues/198495 + doCheck = false; }) super.esqueleto; @@ -1482,14 +1484,11 @@ self: super: { sed -i test/PgInit.hs \ -e s^'host=" <> host <> "'^^ ''; - doCheck = - # https://github.com/commercialhaskell/stackage/issues/6884 - # persistent-postgresql-2.13.5.1 needs persistent-test >= 2.13.1.3 which - # is incompatible with the stackage version of persistent, so the tests - # are disabled temporarily. - false - # https://github.com/NixOS/nixpkgs/issues/198495 - && pkgs.postgresql.doCheck; + # https://github.com/commercialhaskell/stackage/issues/6884 + # persistent-postgresql-2.13.5.1 needs persistent-test >= 2.13.1.3 which + # is incompatible with the stackage version of persistent, so the tests + # are disabled temporarily. + doCheck = false; preCheck = drv.preCheck or "" + '' PGDATABASE=test PGUSER=test @@ -1498,6 +1497,9 @@ self: super: { pkgs.postgresql pkgs.postgresqlTestHook ]; + } // lib.optionalAttrs (!pkgs.postgresql.doCheck) { + # https://github.com/NixOS/nixpkgs/issues/198495 + doCheck = false; }) super.persistent-postgresql; @@ -1665,12 +1667,13 @@ self: super: { testToolDepends = drv.testToolDepends or [] ++ [ pkgs.postgresql pkgs.postgresqlTestHook ]; - # https://github.com/NixOS/nixpkgs/issues/198495 - doCheck = pkgs.postgresql.doCheck; preCheck = drv.preCheck or "" + '' # empty string means use default connection export DATABASE_URL="" ''; + } // lib.optionalAttrs (!pkgs.postgresql.doCheck) { + # https://github.com/NixOS/nixpkgs/issues/198495 + doCheck = false; }) (super.pg-client.override { resource-pool = self.hasura-resource-pool; ekg-core = self.hasura-ekg-core; @@ -2133,9 +2136,9 @@ self: super: { # Tests need to lookup target triple x86_64-unknown-linux # https://github.com/llvm-hs/llvm-hs/issues/334 - llvm-hs = overrideCabal { - doCheck = pkgs.stdenv.targetPlatform.system == "x86_64-linux"; - } super.llvm-hs; + llvm-hs = overrideCabal (lib.optionalAttrs (pkgs.stdenv.targetPlatform.system != "x86_64-linux") { + doCheck = false; + }) super.llvm-hs; # Fix build with bytestring >= 0.11 (GHC 9.2) # https://github.com/llvm-hs/llvm-hs/pull/389 -- cgit 1.4.1 From 72e03b91eaa7b2e631e6f642efdb538a5bcb29fa Mon Sep 17 00:00:00 2001 From: Wolfgang Walther Date: Sat, 10 Feb 2024 15:42:35 +0100 Subject: haskellPackages: add dontCheckIf helper Using this helper will prevent introducing problematic doCheck = condition overrides, which accidentally re-enable previously disabled tests. --- doc/languages-frameworks/haskell.section.md | 5 ++ .../haskell-modules/configuration-common.nix | 79 ++++++++++------------ .../haskell-modules/configuration-nix.nix | 6 +- pkgs/development/haskell-modules/lib/compose.nix | 5 ++ pkgs/development/haskell-modules/lib/default.nix | 5 ++ 5 files changed, 54 insertions(+), 46 deletions(-) (limited to 'pkgs/development/haskell-modules/configuration-common.nix') diff --git a/doc/languages-frameworks/haskell.section.md b/doc/languages-frameworks/haskell.section.md index 5646502cba77..5d7796b554de 100644 --- a/doc/languages-frameworks/haskell.section.md +++ b/doc/languages-frameworks/haskell.section.md @@ -1020,6 +1020,11 @@ failing because of e.g. a syntax error in the Haddock documentation. : Sets `doCheck` to `false` for `drv`. Useful if a package has a broken, flaky or otherwise problematic test suite breaking the build. +`dontCheckIf condition drv` +: Sets `doCheck` to `false` for `drv`, but only if `condition` applies. +Otherwise it's a no-op. Useful to conditionally disable tests for a package +without interfering with previous overrides or default values. + diff --git a/pkgs/development/haskell-modules/configuration-common.nix b/pkgs/development/haskell-modules/configuration-common.nix index 64b388e5e57a..4b63ec84051c 100644 --- a/pkgs/development/haskell-modules/configuration-common.nix +++ b/pkgs/development/haskell-modules/configuration-common.nix @@ -144,18 +144,18 @@ self: super: { # https://github.com/lspitzner/czipwith/issues/5 czipwith = doJailbreak super.czipwith; - # Deal with infinite and NaN values generated by QuickCheck-2.14.3 - aeson = overrideCabal (lib.optionalAttrs pkgs.stdenv.hostPlatform.is32bit { + aeson = # aeson's test suite includes some tests with big numbers that fail on 32bit # https://github.com/haskell/aeson/issues/1060 - doCheck = false; - }) (appendPatches [ - (pkgs.fetchpatch { - name = "aeson-quickcheck-2.14.3-double-workaround.patch"; - url = "https://github.com/haskell/aeson/commit/58766a1916b4980792763bab74f0c86e2a7ebf20.patch"; - sha256 = "1jk2xyi9g6dfjsi6hvpvkpmag3ivimipwy1izpbidf3wvc9cixs3"; - }) - ] super.aeson); + dontCheckIf pkgs.stdenv.hostPlatform.is32bit + # Deal with infinite and NaN values generated by QuickCheck-2.14.3 + (appendPatches [ + (pkgs.fetchpatch { + name = "aeson-quickcheck-2.14.3-double-workaround.patch"; + url = "https://github.com/haskell/aeson/commit/58766a1916b4980792763bab74f0c86e2a7ebf20.patch"; + sha256 = "1jk2xyi9g6dfjsi6hvpvkpmag3ivimipwy1izpbidf3wvc9cixs3"; + }) + ] super.aeson); # Lifts bounds on hoauth2, skylighting, and json adds compat with mtl >= 2.3 gitit = appendPatches [ @@ -1318,13 +1318,12 @@ self: super: { # https://github.com/mgajda/json-autotype/issues/25 json-autotype = dontCheck super.json-autotype; - # Requires pg_ctl command during tests - beam-postgres = overrideCabal (drv: { - testToolDepends = (drv.testToolDepends or []) ++ [pkgs.postgresql]; - } // lib.optionalAttrs (!pkgs.postgresql.doCheck) { + beam-postgres = lib.pipe super.beam-postgres [ + # Requires pg_ctl command during tests + (addTestToolDepends [pkgs.postgresql]) # https://github.com/NixOS/nixpkgs/issues/198495 - doCheck = false; - }) super.beam-postgres; + (dontCheckIf (!pkgs.postgresql.doCheck)) + ]; # PortMidi needs an environment variable to have ALSA find its plugins: # https://github.com/NixOS/nixpkgs/issues/6860 @@ -1378,11 +1377,9 @@ self: super: { pkgs.postgresql pkgs.postgresqlTestHook ]; - } // lib.optionalAttrs (!pkgs.postgresql.doCheck) { - # https://github.com/NixOS/nixpkgs/issues/198495 - doCheck = false; }) - super.esqueleto; + # https://github.com/NixOS/nixpkgs/issues/198495 + (dontCheckIf (!pkgs.postgresql.doCheck) super.esqueleto); # Requires API keys to run tests algolia = dontCheck super.algolia; @@ -1497,11 +1494,9 @@ self: super: { pkgs.postgresql pkgs.postgresqlTestHook ]; - } // lib.optionalAttrs (!pkgs.postgresql.doCheck) { - # https://github.com/NixOS/nixpkgs/issues/198495 - doCheck = false; }) - super.persistent-postgresql; + # https://github.com/NixOS/nixpkgs/issues/198495 + (dontCheckIf (!pkgs.postgresql.doCheck) super.persistent-postgresql); # Test suite requires a later version of persistent-test which depends on persistent 2.14 # https://github.com/commercialhaskell/stackage/issues/6884 @@ -1662,22 +1657,24 @@ self: super: { hasura-ekg-json = super.hasura-ekg-json.override { ekg-core = self.hasura-ekg-core; }; - pg-client = overrideCabal (drv: { - librarySystemDepends = with pkgs; [ postgresql krb5.dev openssl.dev ]; - testToolDepends = drv.testToolDepends or [] ++ [ - pkgs.postgresql pkgs.postgresqlTestHook + pg-client = lib.pipe + (super.pg-client.override { + resource-pool = self.hasura-resource-pool; + ekg-core = self.hasura-ekg-core; + }) [ + (overrideCabal (drv: { + librarySystemDepends = with pkgs; [ postgresql krb5.dev openssl.dev ]; + testToolDepends = drv.testToolDepends or [] ++ [ + pkgs.postgresql pkgs.postgresqlTestHook + ]; + preCheck = drv.preCheck or "" + '' + # empty string means use default connection + export DATABASE_URL="" + ''; + })) + # https://github.com/NixOS/nixpkgs/issues/198495 + (dontCheckIf (!pkgs.postgresql.doCheck)) ]; - preCheck = drv.preCheck or "" + '' - # empty string means use default connection - export DATABASE_URL="" - ''; - } // lib.optionalAttrs (!pkgs.postgresql.doCheck) { - # https://github.com/NixOS/nixpkgs/issues/198495 - doCheck = false; - }) (super.pg-client.override { - resource-pool = self.hasura-resource-pool; - ekg-core = self.hasura-ekg-core; - }); hcoord = overrideCabal (drv: { # Remove when https://github.com/danfran/hcoord/pull/8 is merged. @@ -2136,9 +2133,7 @@ self: super: { # Tests need to lookup target triple x86_64-unknown-linux # https://github.com/llvm-hs/llvm-hs/issues/334 - llvm-hs = overrideCabal (lib.optionalAttrs (pkgs.stdenv.targetPlatform.system != "x86_64-linux") { - doCheck = false; - }) super.llvm-hs; + llvm-hs = dontCheckIf (pkgs.stdenv.targetPlatform.system != "x86_64-linux") super.llvm-hs; # Fix build with bytestring >= 0.11 (GHC 9.2) # https://github.com/llvm-hs/llvm-hs/pull/389 diff --git a/pkgs/development/haskell-modules/configuration-nix.nix b/pkgs/development/haskell-modules/configuration-nix.nix index 8d976685e57a..d0a874187b6e 100644 --- a/pkgs/development/haskell-modules/configuration-nix.nix +++ b/pkgs/development/haskell-modules/configuration-nix.nix @@ -1103,7 +1103,7 @@ self: super: builtins.intersectAttrs super { rel8 = pkgs.lib.pipe super.rel8 [ (addTestToolDepend pkgs.postgresql) # https://github.com/NixOS/nixpkgs/issues/198495 - (overrideCabal (lib.optionalAttrs (!pkgs.postgresql.doCheck) { doCheck = false; })) + (dontCheckIf (!pkgs.postgresql.doCheck)) ]; # Wants running postgresql database accessible over ip, so postgresqlTestHook @@ -1178,9 +1178,7 @@ self: super: builtins.intersectAttrs super { # Some hash implementations are x86 only, but part of the test suite. # So executing and building it on non-x86 platforms will always fail. - hashes = overrideCabal (lib.optionalAttrs (!pkgs.stdenv.hostPlatform.isx86) { - doCheck = false; - }) super.hashes; + hashes = dontCheckIf (!pkgs.stdenv.hostPlatform.isx86) super.hashes; # Tries to access network aws-sns-verify = dontCheck super.aws-sns-verify; diff --git a/pkgs/development/haskell-modules/lib/compose.nix b/pkgs/development/haskell-modules/lib/compose.nix index 40195ac7a801..09cee08b91c1 100644 --- a/pkgs/development/haskell-modules/lib/compose.nix +++ b/pkgs/development/haskell-modules/lib/compose.nix @@ -108,6 +108,11 @@ rec { of test suites listed in the package description file. */ dontCheck = overrideCabal (drv: { doCheck = false; }); + /* The dontCheckIf variant sets doCheck = false if the condition + applies. In any other case the previously set/default value is used. + This prevents accidentally re-enabling tests in a later override. + */ + dontCheckIf = condition: if condition then dontCheck else lib.id; /* doBenchmark enables dependency checking and compilation for benchmarks listed in the package description file. diff --git a/pkgs/development/haskell-modules/lib/default.nix b/pkgs/development/haskell-modules/lib/default.nix index ffd9ac057890..2bcd8f25d114 100644 --- a/pkgs/development/haskell-modules/lib/default.nix +++ b/pkgs/development/haskell-modules/lib/default.nix @@ -105,6 +105,11 @@ rec { of test suites listed in the package description file. */ dontCheck = compose.dontCheck; + /* The dontCheckIf variant sets doCheck = false if the condition + applies. In any other case the previously set/default value is used. + This prevents accidentally re-enabling tests in a later override. + */ + dontCheckIf = drv: condition: compose.dontCheckIf condition drv; /* doBenchmark enables dependency checking, compilation and execution for benchmarks listed in the package description file. -- cgit 1.4.1