Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Refactor and simplify parts of overlays/haskell.nix #1976

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

Conversation

andreabedini
Copy link
Member

I am opening this for visibility and to track my work. As it stands this PR is riddled with unnecessary changes and might not be worth reviewing in the current state.

The core parts of haskell.nix are based on NixOS module system, which powers haskell.nix's flexible and customisable behaviour. I have noticed that the configurtion coming from this module layer is then "post-processed" in a seemingly ad-hoc way, rather than being done as a module as well.

An instance of this is cabalProject' and stackProject'. Taking the first as an example, the current flow is:

  # in overlays/haskell.nix
  cabalProject' = projectModule:
    haskellLib.evalProjectModule
      ../modules/cabal-project.nix
      projectModule
      # this is the function does the post-processing, which we call `f`
      ({ src, compiler-nix-name, compilerSelection, evalPackages, ... }:
        let
          # omitted
          project = { }; # omitted
        in
        project
      );

  # in lib/default.nix
  evalProjectModule = projectType: m: f: # <- this is `f`
    let
      project = f (lib.evalModules {
        modules =
          (if builtins.isList m then m else [ m ]) ++
          [
            (import ../modules/project-common.nix)
            (import projectType)
            ({ config, lib, ... }: {
              _module.args = { inherit pkgs; inherit (project) buildProject; };
              inherit (project) hsPkgs;
            })
          ];
      }).config;
    in
    project;

Basically f is passed the config obtained from evaluating the modules (after injecting the user's projectModule and ../modules/cabal-project.nix). So args in f is just the module config. This calls for f to be a module in itself. We can inline evalProjectModule into cabalProject' and turn f into a module that adds something to the config:

  # in overlays/haskell.nix
  cabalProject' = projectModule:
    (final.lib.evalModules {
      modules =
        (if builtins.isList projectModule then projectModule else [ projectModule ]) ++
        [
          (import ../modules/project-common.nix)
          (import ../modules/cabal-project.nix)
          ({ config, lib, ... }: {
            _module.args = { pkgs = final; inherit (config.result-of-f) buildProject; };
            inherit (config.result-of-f) hsPkgs;
          })
          # f-now-turned-module
          ({ config, lib, pkgs, ... }:
            let
              args = config;
              inherit (config) compiler-nix-name compilerSelection evalPackages;
              # omitted
              project = { }; # omitted
            in
            {
              options.result-of-f = lib.mkOption { type = lib.types.unspecified; };
              config.result-of-f = project;
            }
          )
        ];
    }).config.result-of-f;

As far as I can tell, this transformation is entirely syntactic and should preseve the semantic of the original code. We can also avoid setting _module.args since f-now-turned-module can export it directly (with minimal changes to the other modules). A step further would moving f-now-turned-module into ../modules/cabal-project.nix given it is a necessary part of configuring a cabal project.

A similar transformation can be done for stackProject'.

Furthermore, if I understand correctly, there are two different invocations of pkgs.lib.evalModules corresponding to the configuration of very different things (the project and the package-set). I am considering separating the configuration of projects and package-sets into two separate directories, to make the distinction clear.

Special thanks to @Radvendii who helped me reason about this change.

@andreabedini andreabedini self-assigned this Jun 2, 2023
};
inherit (config.result-of-f) hsPkgs;
})
# FIXME: should this be part of cabal-project.nix?
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That would be nice

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We did move most of the code for hackage-project into a module file and I think it is nicer like that.

@hamishmack
Copy link
Collaborator

This might be something for another PR, but I have often wondered if we should be doing the modules argument differently? When we first added the project functions the arguments we passed them were not modules at all. So when we made them support passing modules we just wound up with this nested modules attribute. Is there a better way? Even if there is not I wonder if we should rename modules to something more descriptive.

@andreabedini
Copy link
Member Author

Yes, I also thought about renaming the modules attribute into something project and package-set specific but I had some doubts.
At the moment these two "module systems" (I am not sure how to call them) are evaluated independently, basically before and after the plan-nix IFD.
Maybe they could be part of the same evaluation? Or maybe the IFD in the middle prevents that?

@angerman
Copy link
Collaborator

angerman commented Jun 3, 2023

Just be careful around backwards compatibility and breaking changes. Haskell.nix is actively used. Existing users would need to be carefully guided to the new syntax.

@andreabedini
Copy link
Member Author

@angerman 👍 after I clean this up, there should no change to how cabalProjct and stackProject are called. I think evalProjectModules is exposed so maybe we can keep that around.

@andreabedini andreabedini force-pushed the andrea/embrace-modules branch from 852663d to 43de0d0 Compare June 6, 2023 15:20
@andreabedini andreabedini force-pushed the andrea/embrace-modules branch from 43de0d0 to 8a39583 Compare June 20, 2023 06:24
@andreabedini
Copy link
Member Author

I am still interested in making this change, there are two things left in progress:

  1. Inlining evalProjectModules introduces some code duplication between cabalProject and stackProject. I removed what I believe were bad abstractions but I might need to find other, better, abstractions to reduce this duplication.
  2. This needs to be rebased on top of Rework modules #1999

@andreabedini andreabedini force-pushed the andrea/embrace-modules branch from 8a39583 to a46dd90 Compare August 16, 2023 07:55
Copy link

stale bot commented Dec 14, 2023

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 Dec 14, 2023
@stale stale bot closed this Feb 13, 2024
@angerman
Copy link
Collaborator

@andreabedini did anything ever happen here? Should we keep this open?

@andreabedini
Copy link
Member Author

Nothing happened after #1999. I still have my branch so maybe it's ok to leave it closed... until next time? 😂

@andreabedini andreabedini deleted the andrea/embrace-modules branch May 24, 2024 10:05
@andreabedini andreabedini restored the andrea/embrace-modules branch May 24, 2024 10:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants
Morty Proxy This is a proxified and sanitized view of the page, visit original site.