hackage2nix: add excluded-packages config option

[wolfgangwalther]

This adds a config option excluded-packages, which can be used in Nixpkgs to remove a big chunk of old, broken packages.

WIP PR in Nixpkgs at NixOS/nixpkgs#441204.

[wolfgangwalther]

I'm currently experimenting with this in Nixpkgs and I'm confused about something: When I remove a package that is used by others, then in some cases hackage2nix / cabal2nix behave differently than in others:

• Sometimes the respective dependency is explicitly set to null in the callPackage args,
• and sometimes it's not, which then fails eval.

Two random examples. I excluded accelerate-llvm and haskell98:

  "caldims" = callPackage
    ({ mkDerivation, base, containers, directory, haskell98, mtl
     , parsec, readline
     }:
     mkDerivation {
       pname = "caldims";
       version = "0.1.0";
       sha256 = "0mlgxghah8mw0v17rywfj190kmc4jajpmjpgkpgfxdqzw8djyph0";
       isLibrary = true;
       isExecutable = true;
       libraryHaskellDepends = [
         base containers directory haskell98 mtl parsec readline
       ];
       executableHaskellDepends = [
         base containers directory haskell98 mtl parsec readline
       ];
       description = "Calculation tool and library supporting units";
       license = "GPL";
       hydraPlatforms = lib.platforms.none;
       mainProgram = "caldims";
       broken = true;
     }) {haskell98 = null;};

haskell98 is passed as null (good)

  "accelerate-fft" = callPackage
    ({ mkDerivation, accelerate, accelerate-llvm
     , accelerate-llvm-native, accelerate-llvm-ptx, base, bytestring
     , carray, containers, cuda, cufft, fft, file-embed, hashable
     , hedgehog, lens-accelerate, mtl, tasty, tasty-hedgehog
     , unordered-containers
     }:
     mkDerivation {
       pname = "accelerate-fft";
       version = "1.3.0.0";
       sha256 = "1a7cwzbs8r3rvaymrq2kfx83lqb3i7wz0gmz3ppz59f40rxn974x";
       libraryHaskellDepends = [
         accelerate accelerate-llvm accelerate-llvm-native
         accelerate-llvm-ptx base bytestring carray containers cuda cufft
         fft file-embed hashable lens-accelerate mtl unordered-containers
       ];
       testHaskellDepends = [
         accelerate accelerate-llvm-native accelerate-llvm-ptx base hedgehog
         tasty tasty-hedgehog
       ];
       description = "FFT using the Accelerate library";
       license = lib.licenses.bsd3;
       hydraPlatforms = lib.platforms.none;
       broken = true;
     }) {lens-accelerate = null;};

accelerate-llvm is not passed as null (bad).

Why is that? @sternenseemann any insight?

[sternenseemann]

I would need to investigate, too. Possible that this is stateful somehow. I've never really touched hackaeg2nix, but do know that it uses STM, so may be tricky behavior involved.

[wolfgangwalther]

So it seems like accelerate-fft depends on both lens-accelerate unconditionally and accelerate-llvm behind a flag. When I remove both dependencies, Cabal's finalizePD will return early with only a missing dependency of lens-accelerate- it doesn't even need to check flags, because it already knows it won't be able to succeed. This means we only set lens-accelerate = null, but not accelerate-llvm, because we do that based on the missing dependencies returned by that function.

[wolfgangwalther]

The first approach wasn't working well: Initially, I removed the excluded package from the hackage db at the very beginning. But this would lead to unexpected results later: Cabal would miss some dependencies, and thus toggle flags in a different way. This would also affect packages that were not broken, right now. For example prettyprinter has a text flag, which suddenly got toggled because Cabal couldn't find one of the benchmark dependencies anymore - this dependency was removed, because it was long broken. But since we never enabled the benchmarks anyway, we didn't care about this being broken. This then caused the whole set to rebuild.

The new approach is different: Now, I'm just writing foo = null; for each dependency that should be excluded. That already reduces the number of lines by a lot - and seems to work well, because the flag selection is not affected: Cabal still has access to the full package db. We could potentially just remove this null binding as well, and run the same thing in Nixpkgs, directly from the excluded file. That would save us another few thousand lines. But given the scale of what we're operating on, this isn't important right now: The diff in Nixpkgs has a few 100k lines less with this.

[maralorn]

LGTM

[sternenseemann]

Seems like a good starting point. I feel like we may eventually want to figure out a way to get rid of them completely, but I would do that later. (There may even be things we want to do first, e.g. changing the DB backend from all-cabal-hashes to the normal cabal-install index tar which now has cryptographic hashes for a hopefully large enough subset of Hackage).

One consideration could be to automatically create throws for the removed attribtues by

1. Passing config into hackage-packages.nix.
2. Adding a __throwIfAliases = name: if config.allowAliases then … else null
3. Using that in the generated binds.

Maybe that's overkill though?

[wolfgangwalther]

One consideration could be to automatically create throws for the removed attribtues by

1. Passing config into hackage-packages.nix.

2. Adding a __throwIfAliases = name: if config.allowAliases then … else null

3. Using that in the generated binds.

Maybe that's overkill though?

I want to have throws for better UX, yes. I was thinking about implementing them in Nixpkgs, though. My rough idea:

• Remove these attributes entirely from here. At this stage, they are still function arguments, so we would get errors "this argument doesn't exist".
• Recreate these as the if you mentioned above in Nixpkgs from the exclusion file.
• For this to work, represent the exclusion file as .json, which can be imported in Nix and is valid YAML for hackage2nix to read.

This would save us a few thousand lines in hackage-packages.nix, because we don't need to repeat each of these attributes in two files.

Also, it would make us more flexible regarding potential changes in how aliases / throws are represented in Nixpkgs (see my initial work in NixOS/nixpkgs#442066).

[sternenseemann]

Remove these attributes entirely from here. At this stage, they are still function arguments, so we would get errors "this argument doesn't exist".

This sounds good. It is probably better to have something explode if a package actually ends up depending on an excluded one as that would be very confusing to debug.

[wolfgangwalther]

Changed it here, so that these are not created at all. The Nixpkgs PR is updated accordingly.

[sternenseemann]

How do you prevent the old problem that the dependency specification/resolution is incorrect?

[wolfgangwalther]

The resolution is correct right now, because I exclude these packages later.

My first try excluded them here:

  hackage <- fixup <$> readHackage hackageRepository

hackage is also used for the lookups, IIUC.

But now I exclude them in db, which is only used here:

  pkgs <- runParIO $ flip parMapM (Map.toAscList db) $ \(name, vs) -> do

So it's just the iteration when writing out the packages that is affected now.

[wolfgangwalther]

I think the current state is what we'll want to do on the hackage2nix side - we just need to iterate on the nixpkgs side.

Are you OK with merging this as-is, @sternenseemann?

[sternenseemann]

Can you note why they have to be added to disabled here? This may not be clear since they are removed anyway.

[wolfgangwalther]

I think the idea was that if there is a maintained package that will be removed (excluded) that way, this would throw an error - which I think is a good thing to look into that case in more detail.

I didn't test this, though. Is that roughly what this function does?

[sternenseemann]

Yes, I believe so.

[sternenseemann]

Note that I did not test this or investigate if there are any problems with it in practice, but I believe you have done that.