Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Modify package options for all local packages #298

Open
purefn opened this issue Nov 6, 2019 · 19 comments
Open

Modify package options for all local packages #298

purefn opened this issue Nov 6, 2019 · 19 comments
Assignees
Labels
preserved Keep stale bot away

Comments

@purefn
Copy link
Contributor

purefn commented Nov 6, 2019

The current way of modifying package configurations seems to have a few drawbacks I haven't figured out a good way around. One is changing an option, like disabling haddock or setting a postUnpack value, for a subset of unnamed of packages determined by a function, like checking if a package is local. It seems like you can only do it by manually listing the packages.

A separate issue is being able to patch one package with the nix store path of another Haskell package.

Is there a way to accomplish these that I'm not finding?

@purefn
Copy link
Contributor Author

purefn commented Nov 6, 2019

Nevermind. I just realized that you can do something like

  pkgSet = pkgs.haskell-nix.mkStackPkgSet {
    stack-pkgs = import ./pkgs.nix;
    pkg-def-extras = [];
    modules = [
      ({ config, options, ...}:
      {
      })
    ];
  };

which I think will solve at least one of my problems.

It would be good to put this in the documentation.

@purefn purefn closed this as completed Nov 6, 2019
@purefn
Copy link
Contributor Author

purefn commented Nov 6, 2019

Ok, so that was a dead end. Trying to define the configuration in terms of the config or options passed to the module results in an infinite recursion:

  pkgSet = pkgs.haskell-nix.mkStackPkgSet {
    # stack-pkgs = patchCommonPackage (import ./pkgs.nix);
    stack-pkgs = import ./pkgs.nix;
    pkg-def-extras = [];
    modules = [
      ({ config, options, ...}:
      {
        config.packages =
          let
            localPkgs = pkgs.lib.filterAttrs (pkg: pkgs.isLocal) options.packages.value;
            localNames = pkgs.lib.attrNames localPkgs;
            patchCommon = {
              postUnpack = ''
                if [ -e "$sourceRoot/package.yaml" ]; then
                  sed  -i '/_common/c\_common: !include "${./common-package.yaml}"' "$sourceRoot/package.yaml"
                fi
              '';
            };
          in builtins.listToAttrs (map (n: pkgs.lib.nameValuePair n patchCommon) localNames);
      })
    ];
  };

@angerman @hamishmack, is there a way to accomplish what I'm trying to do? If not, can you suggest an alternative approach?

@purefn purefn reopened this Nov 6, 2019
@hamishmack
Copy link
Collaborator

Maybe we need more low level isLocal (one that we could not override) to avoid infinite recursion.

@hamishmack
Copy link
Collaborator

Perhaps we could have plan-to-nix and stack-to-nix output localNames (but not as part of the modules). So in your example instead of:

  localPkgs = pkgs.lib.filterAttrs (pkg: pkgs.isLocal) options.packages.value;
  localNames = pkgs.lib.attrNames localPkgs;

We could just write:

  inherit (stack-pkgs) localNames;

@hamishmack
Copy link
Collaborator

First try:

pkgSet = pkgs.haskell-nix.mkStackPkgSet {
    # stack-pkgs = patchCommonPackage (import ./pkgs.nix);
    stack-pkgs = import ./pkgs.nix;
    pkg-def-extras = [];
    modules = [
      ({ config, options, hackage, ...}:
      {
        config.packages =
          let
            localPkgs = pkgs.lib.filterAttrs (pkg: pkgs.isLocal) (stack-pkgs.packages hackage);
            localNames = pkgs.lib.attrNames localPkgs;
            patchCommon = {
              postUnpack = ''
                if [ -e "$sourceRoot/package.yaml" ]; then
                  sed  -i '/_common/c\_common: !include "${./common-package.yaml}"' "$sourceRoot/package.yaml"
                fi
              '';
            };
          in builtins.listToAttrs (map (n: pkgs.lib.nameValuePair n patchCommon) localNames);
      })
    ];
  };

@purefn
Copy link
Contributor Author

purefn commented Nov 7, 2019

@hamishmack modifying plan-to-nix and stack-to-nix would be great!

I tried your example and had to change a few things, mainly the way stack-pkgs is used. I was able to check for a local package by checking if the value is a path or not. It also seems that hackage is not one of the parameters to the module expression, so instead I'm grabbing it from the config.

  pkgSet = pkgs.haskell-nix.mkStackPkgSet rec {
    # stack-pkgs = fixPkgs (import ./pkgs.nix);
    stack-pkgs = import ./pkgs.nix;
    pkg-def-extras = [];
    modules = [
      ({ config, options, ...}:
      {
        config.packages =
          let
            localPkgs = pkgs.lib.filterAttrs (_: pkg: builtins.typeOf pkg == "path") (stack-pkgs.extras config.hackage.configs).packages;
            localNames = pkgs.lib.attrNames localPkgs;
            patchCommon = {
              postUnpack = ''
                if [ -e "$sourceRoot/package.yaml" ]; then
                  sed  -i '/_common/c\_common: !include "${./common-package.yaml}"' "$sourceRoot/package.yaml"
                fi
              '';
            };
          in builtins.listToAttrs (map (n: pkgs.lib.nameValuePair n patchCommon) localNames);
      })
    ];
  };

Seems to work quite well, thanks!

@purefn
Copy link
Contributor Author

purefn commented Nov 7, 2019

Oh, I spoke too soon! There are some packages that already have a postUnpack defined. I don't want to completely get rid of them, I want to add to them. But any attempt to get the original value results in infinite recursion.

@arianvp
Copy link
Contributor

arianvp commented Feb 18, 2020

Running into exactly the same problem.

How about we change the type of postUnpack to

types.listOf type.string

then nixos module system merging will take care of this. I'll see if I can come up with a patch!

@arianvp
Copy link
Contributor

arianvp commented Feb 18, 2020

I ended up with a hack where I check whether the package's src is of type path which means it's a local package (e.g. source code in your directory). This check does not give infinite recursion, but does filter out any non-local package.

By using mkIf and mkMerge we don't run into infinite recursion

pkgs.haskell-nix.mkStackPkgSet rec {
      stack-pkgs = import ./nix/stack/pkgs.nix;
      pkg-def-extras = [];

      modules =
        [
          (
            let
              pkg-names = lib.attrNames (stack-pkgs.extras {}).packages;
              patch = name: builtins.trace name {

                # Final hack, workaround for  https://github.com/input-output-hk/haskell.nix/issues/298
                # we know that local packages sure dont have postUnpack. We find out that they're local based
                # on their source beign local
                ${name} = { config, ... }: lib.mkIf (builtins.typeOf config.src == "path") {

                  postUnpack = ''
                    if [[ -e $sourceRoot/package.yaml ]]; then
                      substituteInPlace $sourceRoot/package.yaml --replace '../../package-defaults.yaml' "${./package-defaults.yaml}"
                    fi
                  '';
                  dontStrip = false;
                };
              };
            in
              {
                packages = lib.mkMerge (map patch pkg-names);
              }
          )
        ];
    };

The reason your version broke is that postUnpack is used to implement subdir support of stack.yaml

The durable fix is to indeed make postUnpack mergeable I think because now
you can not "patch" subdir'd packages. Which sounds like we should file a new issue for that with the fix I mentioned in the previous comment

@purefn
Copy link
Contributor Author

purefn commented Apr 15, 2020

@arianvp thanks, that's great! I agree it would be better if postUnpack was a list instead of a string.

One thing I'm also wanting to do is wrap executables and tests if they depend on tmp-postgres. I've been trying to add something like

${name} = { config, ... }: lib.mkIf config.package.isLocal {
  components.exes = mapAttrs wrapExe config.components.exes;
}

But keep getting infinite recursion. I haven't been able to figure out if or how a similar hack with mkMerge/mkIf could work. Any ideas?

@infinisil
Copy link
Contributor

It is indeed possible to do this with the module system without getting infinite recursion. The important thing to know is that if you have an option of type attrsOf (submodule ...) like packages, and you want to make changes to all the values, you can do so be redeclaring the option with the same type, but passing your own configuration in the submodule. This works because the module system merges multiple option declarations together.

As an untested example, you can add a postUnpack to all packages where package.isLocal is true with a module like this:

{ lib, ...}: {
  options.packages = lib.mkOption {
    type = lib.types.attrsOf (lib.types.submodule ({ config, ... }: {
      config = lib.mkIf config.package.isLocal {
        postUnpack = lib.mkAfter ''
          echo foo
        '';
      };
    }));
  };
}

@purefn
Copy link
Contributor Author

purefn commented Mar 15, 2021

@infinisil interesting, thanks. With haskell.nix, where would that go? In the modules list passed to project?

@infinisil
Copy link
Contributor

@purefn Yeah that goes into the modules list

@purefn
Copy link
Contributor Author

purefn commented Mar 22, 2021

@infinisil Thanks! That works perfectly!

@purefn purefn closed this as completed Mar 22, 2021
@michaelpj
Copy link
Collaborator

IMO this is still an issue, the workaround is pretty unpleasant.

@stale
Copy link

stale bot commented Sep 28, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix label Sep 28, 2022
@hamishmack hamishmack added preserved Keep stale bot away and removed wontfix labels Oct 10, 2022
@amesgen
Copy link
Member

amesgen commented Jun 7, 2023

Heads up: Usually, one probably does not want to modify all local packages, but rather only project packages: All project packages are local packages, but e.g. source-repository-packages are local non-project packages.

E.g. in #298 (comment), one can simply swap out isLocal for isProject.

@andreabedini
Copy link
Member

Heads up: Usually, one probably does not want to modify all local packages, but rather only project packages: All project packages are local packages, but e.g. source-repository-packages are local non-project packages.

project-vs-non-project packages is a distinction that haskell.nix makes right? AFAIU cabal only distinguishes local vs non-local.

@andreabedini
Copy link
Member

The solution suggested by @infinisil is the correct one. That does not mean we cannot improve on the situation. Perhaps we could have a localPackages submodule that spits out configuration for all local packages?

@andreabedini andreabedini self-assigned this Jun 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
preserved Keep stale bot away
Projects
None yet
Development

No branches or pull requests

7 participants