Setup tz package overrides to reference system tzdata package correctly #611
Conversation
This commit changes the cabal2nix hooks for the `tz` Haskell package.
Originally, the `tz` package had a hook that added a phase override for
`preConfigure` that looked like the following:
```nix
preConfigure = "export TZDIR=${pkgs.tzdata}/share/zoneinfo";
```
This works for `hackage2nix`, but the problem is that when using
`cabal2nix` by itself, this produces a Nix file like the following:
```nix
{ mkDerivation, base, binary, bytestring, containers, criterion
, data-default, deepseq, HUnit, lens, lib, QuickCheck, tasty
, tasty-hunit, tasty-quickcheck, tasty-th, template-haskell, thyme
, time, timezone-olson, timezone-series, tzdata, vector
}:
mkDerivation {
pname = "tz";
version = "0.1.3.6";
preConfigure = "export TZDIR=${pkgs.tzdata}/share/zoneinfo";
...
}
```
This is trying to reference `pkgs.tzdata`, but `pkgs` is not available
here!
This commit changes the hook for `tz` to take a `system-tzdata` package
as an argument, and then correctly reference this `system-tzdata` for
the tests:
```nix
{ mkDerivation, base, binary, bytestring, containers, criterion
, data-default, deepseq, HUnit, lens, lib, QuickCheck
, system-tzdata, tasty, tasty-hunit, tasty-quickcheck, tasty-th
, template-haskell, thyme, time, timezone-olson, timezone-series
, tzdata, vector
}:
mkDerivation {
pname = "tz";
version = "0.1.3.6";
preCheck = "export TZDIR=${system-tzdata}/share/zoneinfo";
...
}
```
|
I've also tested this by bumping Also, it appears that CI is failing, but it doesn't appear related to this PR. |
| tzOverrides :: Derivation -> Derivation | ||
| tzOverrides = | ||
| -- The tz Haskell package uses zoneinfo files from the system tzdata package in its tests. | ||
| set phaseOverrides "preCheck = \"export TZDIR=${system-tzdata}/share/zoneinfo\";" . |
There was a problem hiding this comment.
This should be unnecessary, actually, since the tzata setup hook in nixpkgs sets this variable nowadays. Can you try removing this?
If we can drop this, I wonder if having no cabal2nix-override would be better, since system-tzdata needs an external override anyways and it is an arbitrarily chosen name, so probably more confusing than not having it!
There was a problem hiding this comment.
This should be unnecessary, actually, since the tzata setup hook in nixpkgs sets this variable nowadays. Can you try removing this?
Great suggestion, I made this change in c2b01f6 and it looks like it works well!
I also tried this change by bumping cabal2nix-unstable in Nixpkgs, regenerating haskellPackages.tz and rebuilding. It built fine.
If we can drop this, I wonder if having no cabal2nix-override would be better, since system-tzdata needs an external override anyways and it is an arbitrarily chosen name, so probably more confusing than not having it!
Yeah, that sounds reasonable as well. I don't have a strong opinion either way, but I do slightly agree with you that it is somewhat confusing having the arbitrarily-named system-tzdata argument.
Want me to just drop the override?
There was a problem hiding this comment.
Want me to just drop the override?
Yes, I think that is the best solution to this unfortunate situation!
There was a problem hiding this comment.
Alright, I've just completely dropped the override in 945e8ee.
@sternenseemann once you approve this, I'll update cabal2nix-unstable in Nixpkgs and fix up the tz package there as well.
The `tzdata` system package has a setup hook that sets the `TZDIR` env var. This was suggested in NixOS#611 (comment)
We've decided that the entire tz override doesn't make sense in cabal2nix. The tests for the tz Haskell package require the tzdata system library, but we can add that in Nixpkgs. That seems like the least confusing way to provide it.
See NixOS/cabal2nix#611 for discussion. While we're changing things, let's also use the tzdata setupHook for haskellPackages.tz.
This PR changes the cabal2nix hooks for the
tzHaskell package. Originally, thetzpackage had a hook that added a phase override forpreConfigurethat looked like the following:This works for
hackage2nix, but the problem is that when usingcabal2nixby itself, this produces a Nix file like the following:This is trying to reference
pkgs.tzdata, butpkgsis not available here!This commit changes the hook for
tzto take asystem-tzdatapackage as an argument, and then correctly reference thissystem-tzdatafor the tests: