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

Rework modules #1999

Merged
Merged

Conversation

andreabedini
Copy link
Member

@andreabedini andreabedini commented Jul 14, 2023

This PR aims to rework and simply some parts of ./modules.

Currently we use a DIY inhertitance scheme to pass options from the plan down to packages and then to components. This basically relies on manually chaining default values to use in mkOption. E.g.

  componentOptions = def: {
         ...
         configureFlags = mkOption {
            ...
            default = (def.configureFlags or []);
          };
         ...
  };

  packageOptions = def: componentOptions def // {
    preUnpack = mkOption {
      type = nullOr lines;
      default = (def.preUnpack or null);
    };
    ...
  };

This is unnecessary as the module system is perfectly able to express default values and their override.

Also this cleans up a strange patterns of passing arguments to module-producing functions only for the returning module to add those arguments to _module.args. Why not adding those arguments to _module.args directly?

# modules/plan.nix (simplified)
let mod_args = {
  inherit pkgs pkgconfPkgs haskellLib;
  inherit (config) hsPkgs errorHandler;
  inherit (config.cabal) system compiler;
 }; in
  attrsOf (submodule (import ./package.nix {
    inherit mod_args;
}));

where

# modules/package.nix (simplified)
{ mod_args, ... }:
{ config, ... }:
{
  config._module.args = mod_args;
  ...
}

What this PR does is:

  • Move componentOptions and packageOptions (minus what's already in componentOptions) to separate option-defining modules ./component-options.nixandpackage-options.nixkeeping the default value which would result fromdef = {}`.
  • Where componentOptions (or packageOptions) was used to populate options, I have instead imported ./component-options.nix (or package-options.nix).
  • Default values are propagated to submodules as default values when the submodule is declared.
  package = submodule [
    {
      _module.args = {
        inherit pkgs pkgconfPkgs haskellLib;
        inherit (config) hsPkgs errorHandler;
        inherit (config.cabal) system compiler;
      };
    }
    ./package.nix
    # pass down common options as default values (note config in the line below binds to the plan's config)
    ({ lib, options, ... }: lib.mkDefault (lib.filterAttrs (n: _v: builtins.hasAttr n options) config))
  ];

There might be a better way to express it but this seems to work.

  • Split the rest of the component submodule out of package.nix and into a separate file component.nix
  • This leaves us with two files /component-options.nix and package-options.nix which we can now see are always used toghether, so we might think of merging them into shared-component-package-options.nix.

All these changes are intended to be semantic preseving and I don't to change any of the resulting derivations. Even the API should be exactly the same, a part from the exposure in haskellLib.types of listOfFilteringNulls, getDefaultOrNull and uniqueStr which where previously hidden in modules/plan.nix. These types can be hidden again if that's desiderable.

I have verified nothing changes in the derivation of cardano-node as follows:

❯ nix path-info --quiet --quiet --derivation \
    github:input-output-hk/cardano-node/8.1.1 \
    --override-input haskellNix github:andreabedini/haskell.nix/50e7b671ca73ea9ab350d7f18ba434bf8393051a
/nix/store/vs5lz9rpkvbsj4zjxpb9694hvdy5x2nc-cardano-node-exe-cardano-node-8.1.1.drv
❯ nix path-info --quiet --quiet --derivation \
    github:input-output-hk/cardano-node/8.1.1 \
    --override-input haskellNix github:andreabedini/haskell.nix/d9aeb2580e656f2a3640309008fcf1fbacd00379
/nix/store/vs5lz9rpkvbsj4zjxpb9694hvdy5x2nc-cardano-node-exe-cardano-node-8.1.1.drv

I am trying to extend the comparison to hydraJobs.required but I am running into some issues due to cardano-node flake's reliance on self.rev. I will update.

@andreabedini andreabedini self-assigned this Jul 14, 2023
@andreabedini andreabedini marked this pull request as draft July 17, 2023 10:13
@andreabedini
Copy link
Member Author

The best I can offer is

❯ nix run github:nix-community/nix-eval-jobs -- \
 --flake github:input-output-hk/cardano-node/andrea/workbench-self-rev-relisient#legacyPackages.x86_64-linux.hydraJobs \
  --override-input haskellNix github:andreabedini/haskell.nix/50e7b671ca73ea9ab350d7f18ba434bf8393051a > orig.json
❯ sha256sum orig.json
a75a7149e1f0247646bfb5a1cea0f28fc88d71d46f24c81f79aa306cf9950829  orig.json
❯ nix run github:nix-community/nix-eval-jobs -- \
  --flake github:input-output-hk/cardano-node/andrea/workbench-self-rev-relisient#legacyPackages.x86_64-linux.hydraJobs \
  --override-input haskellNix github:andreabedini/haskell.nix/d9aeb2580e656f2a3640309008fcf1fbacd00379 > new.json
❯ sha256sum new.json
a75a7149e1f0247646bfb5a1cea0f28fc88d71d46f24c81f79aa306cf9950829  new.json

@andreabedini andreabedini marked this pull request as ready for review July 21, 2023 05:50
@andreabedini
Copy link
Member Author

andreabedini commented Jul 21, 2023

@hamishmack so far I have not found any derivation change induced by this PR changes. Could you give this a look? (Note I have merged #2003 into this).

@hamishmack hamishmack merged commit 509a4ca into input-output-hk:master Jul 25, 2023
29 checks passed
@andreabedini andreabedini deleted the andrea/rework-modules-1 branch July 25, 2023 13:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants