Use unit id for package key

[hamishmack]

Currently haskell.nix uses the package name as the key for hsPkgs. This means that only one package with a given name can exist for a given plan. When the cabal planner makes a plan it often includes more than one version of a given package. For instance if a package is needed for a build-tool-depends it is likely that it may have requirements that do not match the rest of the project.

When there are two versions of the same package in the plan haskell.nix currently chooses the most recent one. This is often the correct choice for the main plan (though it may not always be), but it can sometimes be the wrong choice for the build-tool-depends.

This PR aims to resolve this issue by using the unit ID from the plan.json file as the key for hsPkgs. This means that we can much more closely match the plan.

• Use the plan.json as much as possible (including dependencies and cabal flag settings).
• Fall back on existing sources for information not in plan.json.
• Include mappings from hsPkgs.${pkg-name} to unit ID based entries.
• Support overrides of the form packages.${pkg-name}... (these are applied to all the versions of the package).
• Per-component pre-existing packages based on the plan.json dependencies.

[andreabedini]

It looks like old code paths and new code paths are merged together and separated by conditionals. This makes it quite complex to follow. Would it possible to have, rather than a top level switch, separate entrypoints for the two code paths? Then common code will be available as functions to call (or modules to import). I can try to see what that would look like.

Why do we need addPackageKeys everywhere?

Am I understanding correctly that we are going back to using plan.json rather than integrating with cabal-install at the Haskell level? There are many details that are not available in plan.json (e.g. documentation, profiling, etc).

[andreabedini]

Uh, is this changing the type of packages based on a configured value? Maybe config is from a different module? This seems a bit scary to me, it might lead to recursion issues that will be hard to debug.

[hamishmack]

Why do we need addPackageKeys everywhere?

I had to change the type of packages to make pkgs.lib.modules.mkAliasDefinitions work here:

haskell.nix/modules/install-plan/override-package-by-name.nix

Line 8 in a479589

value = pkgs.lib.modules.mkAliasDefinitions (options.packages.${p.pkg-name});

This means we need to know the list of all the keys for packages when creating the type.

If you add an override for a package in the plan it should work fine, because we include every package id and name in package-keys here:

haskell.nix/modules/install-plan/override-package-by-name.nix

Line 5 in a479589

package-keys = map (p: p.pkg-name) config.plan-json.install-plan ++ map (p: p.id) config.plan-json.install-plan;

If you try to override a package that is not in the plan it will now raise an error. This might actually be quite a nice feature.

However we often want to include overrides that only apply to some plans (for instance packages only included for a given platform). In those cases we can either try to duplicate the logic that determines if the package will be in the plan and only include the override when it is, or we can add it to package-keys.

To make that second option a little easier, addPackageKeys takes the attrNames for packages and includes them in package-keys (it can only do this for packages and not config.packages to avoid infinite recursion).

[hamishmack]

it might lead to recursion issues that will be hard to debug

It scares me too, but I think it might be fairly safe. Do you have a concrete example of something that might fail?

I'm not too worried about use-package-keys since it is not something anyone should need to use directly (perhaps we could call it is-cabal-project or something).

package-keys is like a way of forward declaring the packages you are going to override. One thing does not work due to infinite recursion is getting addPackageKeys from the module arguments in any way.

{modules = [({haskellLib, ...}: haskellLib.addPackageKeys { package.x.flags.y = false; })];}

However that is not really related to the addPackageKeys function. You get the same infinite recursion issue if you reference any of the module args at the top level. It even happens if you try to look up a non existent function, for instance this fails with the same error: infinite recursion encountered:

{modules = [({haskellLib, ...}: haskellLib.foo {})];}

These alternatives all work:

{modules = [(haskellLib.addPackageKeys {packages.x.flags.y = false;})];} # Where haskellLib is in scope
{modules = [(rec {package-keys = builtins.attrNames packages; packages.x.flags.y = false;})];}
{modules = [((x: x // {package-keys = builtins.attrNames x.packages;}) {packages.x.flags.y = false;})];}

[hamishmack]

Am I understanding correctly that we are going back to using plan.json rather than integrating with cabal-install at the Haskell level? There are many details that are not available in plan.json (e.g. documentation, profiling, etc).

This PR fixes the worst of duplicate package issues in haskell.nix. It is still using the Cabal2Nix output from plan-to-nix to get stuff not in plan.json. It would be nice if we could streamline that in the future (perhaps by adding the missing information to plan.json or something). At the very least we should add the package version to the .nix file names we write the Cabal2Nix information into. I think that can wait for another PR though.

I did add the "available targets" information to plan.json using this hamishmack/cabal@c0647bc

They are used to populate the hsPkgs.${pkg-name} redirection packages here

haskell.nix/modules/install-plan/redirect.nix

Lines 1 to 51 in a479589

	# Add `hsPkgs.${pkg-name}` based on the available targets in the plan.
	{pkgs, lib, config, ...}: {
	hsPkgs = builtins.removeAttrs (builtins.mapAttrs (packageName: packageTargets:
	let
	byVersion = builtins.groupBy (x: x.pkg-version) packageTargets;
	versions = builtins.attrNames byVersion;
	in if builtins.length versions != 1
	then let
	err = throw "Multiple versions for ${packageName} ${builtins.toJSON versions}";
	in {
	isRedirect = true;
	identifier = { name = packageName; version = err; };
	components = err;
	checks = err;
	}
	else let
	componentsByName = builtins.listToAttrs (map (x: { name = x.component-name; value = x.available; }) packageTargets);
	lookupComponent = collectionName: name: available:
	let attrPath =
	if collectionName == ""
	then "${packageName}.components.library"
	else "${packageName}.components.${collectionName}.${name}";
	in if builtins.length available != 1
	then throw "Multiple avaialble targets for ${attrPath}"
	else if builtins.isString (builtins.head available)
	then throw "${builtins.head available} looking for ${attrPath}"
	else if collectionName == ""
	then config.hsPkgs.${(builtins.head available).id}.components.library
	else config.hsPkgs.${(builtins.head available).id}.components.${collectionName}.${name};
	componentsWithPrefix = collectionName: prefix:
	lib.listToAttrs (lib.concatLists (lib.mapAttrsToList (n: available:
	lib.optional (lib.hasPrefix "${prefix}:" n && (builtins.length available != 1 || !builtins.elem (builtins.head available) ["TargetNotBuildable" "TargetNotLocal"])) (
	let
	name = lib.removePrefix "${prefix}:" n;
	value = lookupComponent collectionName name available;
	in { inherit name value; }
	)) componentsByName));
	in rec {
	isRedirect = true;
	identifier = rec { name = packageName; version = builtins.head versions; id = "${name}-${version}"; };
	components =
	lib.mapAttrs componentsWithPrefix pkgs.haskell-nix.haskellLib.componentPrefix
	// lib.optionalAttrs (componentsByName ? lib) {
	library = lookupComponent "" "" componentsByName.lib;
	};
	checks = pkgs.recurseIntoAttrs (builtins.mapAttrs
	(_: d: pkgs.haskell-nix.haskellLib.check d)
	(lib.filterAttrs (_: d: d.config.doCheck) components.tests));
	})
	(builtins.groupBy (x: x.pkg-name) config.plan-json.targets)) config.preExistingPkgs;
	}

[hamishmack]

The error if a named package is not in the plan (and not added to package-keys), looks like this:

       error: The option `packages.x' does not exist. Definition values:
       - In `<unknown-file>':
           {
             flags = {
               y = false;
             };
           }