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

Jump to bottom

Adds Haskell generic builder argument modifier #109059

Merged
merged 2 commits into from Jan 27, 2021
Merged

Adds Haskell generic builder argument modifier #109059

merged 2 commits into from Jan 27, 2021

Conversation

jkachmar
Copy link
Contributor

@jkachmar jkachmar commented Jan 11, 2021 •
edited

Motivation for this change

Closes #104349, or rather at least provides a means for users to override settings for Haskell builders that can result in issues like the one described in #104349.

TODO:

  • add documentation
  • add examples

A quick example, lifted from @cdepillabout's original fork that provided this solution, would be:

{
  # ... elided
  haskellPkgs = pkgs.haskell.packages.ghc884.override (hpArgs: {
    overrides = pkgs.lib.composeExtensions (hpArgs.overrides or (_: _: { })) (
      _hfinal: hprev: {
        mkDerivation = args: hprev.mkDerivation ({
          doCheck = false;
          doBenchmark = false;
          doHoogle = true;
          doHaddock = true;
          enableLibraryProfiling = true;
          enableExecutableProfiling = false;
        } // args);
      }
    );
  });
}

The above snippet provide a Haskell package set that does not run tests or benchmarks for any of the declared dependencies, but which can be used as follows:

# ... elided
haskellPkgs.shellFor {
  packages = p: [ p.foo ];
  genericBuilderArgsModifier = args: args // { doCheck = true; doBenchmark = true; };
}

...which would enable tests and benchmarks for the package foo being developed, and would result in their dependencies being compiled into the shell environment accordingly.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@jkachmar
Adds Haskell generic builder argument modifier
335f1fb 
This is a new argument to the Haskell builder's 'shellFor' which allows
the caller to adjust the 'genericBuilderArgs' after they are built.

One immediate application for this is that it allows users to disable
tests, enable benchmarks, etc. for _all_ packages in a given Haskell
package set.

This is often useful when so many of the derivations in the
package set have changed that most of the cache has been invalidated,
and there is no need for the user to run tests on their dependencies,
but they still want these dependencies available in their 'nix-shell'
environment for some package-under-development.
@jkachmar
Copy link
Contributor Author

Ping @cdepillabout and @expipiplus1, since we discussed this in the issue thread.

I've added documentation, but I'm worried it might be a bit verbose. If it is, any thoughts on how the example might be reduced in scope a bit?

If not, I think this should be good to merge; I've tested it with a fork of nixpkgs on my work codebase and it all seems to be working as expected.

@cdepillabout
Copy link
Member

Sorry for taking a while to review this.

I think the documentation is really easy to understand, so I am going to go ahead and merge this in! Thanks for taking the time to put this together!

It would be nice to have a test for this (since the problem it solves is somewhat non-trivial), but given that it would implicitly require IFD, we can't really have it run on Hydra.

@cdepillabout cdepillabout merged commit 987b80a into NixOS:master Jan 27, 2021
@jkachmar
Copy link
Contributor Author

Not a problem, thanks for getting this merged in!

Just as a final point of follow-up, what's the procedure for making sure this gets backported to the release-20.09 branch (if that's possible)?

@cdepillabout
Copy link
Member

Just as a final point of follow-up, what's the procedure for making sure this gets backported to the release-20.09 branch (if that's possible)?

Oh yeah, sorry I had forgot that you originally wanted this on 20.09. There is no real formal process. Could you git cherry-pick -x this commit against the release-20.09 branch and open a PR?

@jkachmar jkachmar mentioned this pull request Feb 2, 2021
Merged
10 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cdepillabout cdepillabout Awaiting requested review from cdepillabout

Assignees
No one assigned
Labels
6.topic: haskell 10.rebuild-darwin: 0 10.rebuild-linux: 0 12. first-time contribution
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Haskell builds do not always build/source test dependencies
3 participants
@jkachmar @cdepillabout @Janik-Haag