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

add maintainer scripts for haskell package generation #86699

Open
wants to merge 1 commit into
base: master
from

Conversation

@hyperfekt
Copy link
Contributor

hyperfekt commented May 4, 2020

Motivation for this change

Currently, the Nix file responsible for the Haskell package set is created from mutable state by a script run by @peti that makes commits to nixpkgs, making it hard to reproduce. This means that changes in the package set cannot easily be evaluated by users other than them, and the script that executes a regular task which needs to be applied to nixpkgs is hard to use and maintained outside of it.

Obviates #62105.

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.
@cdepillabout
Copy link
Member

cdepillabout commented May 4, 2020

@hyperfekt Thanks for working on this.

There are frequently contributors that ask if there is an easy way to regenerate the hackage package set, normally for testing changes they have made.

Here is the most recent: #86659 (comment)

I think having something like this in nixpkgs would be nice, mostly for the people like the above link.

I have some specific questions about this PR though, so I'll ask those questions inline.

@cdepillabout cdepillabout mentioned this pull request May 4, 2020
4 of 6 tasks complete
@peti peti force-pushed the NixOS:haskell-updates branch 3 times, most recently from 2da18dc to fc573e8 May 5, 2020
@hyperfekt hyperfekt force-pushed the hyperfekt:hackage-packages.nix branch from 8dd8003 to 238e2f9 May 5, 2020
@ofborg ofborg bot removed the 10.rebuild-linux: 1 label May 5, 2020
@hyperfekt hyperfekt force-pushed the hyperfekt:hackage-packages.nix branch from 238e2f9 to a30562d May 5, 2020
@hyperfekt
Copy link
Contributor Author

hyperfekt commented May 5, 2020

The update-hackage-packages.sh update-cabal2nix-latest.sh script now depends on a version of cabal2nix with NixOS/cabal2nix#455 merged.

@hyperfekt hyperfekt force-pushed the hyperfekt:hackage-packages.nix branch 3 times, most recently from eb77ebf to 08d5a1e May 5, 2020
@peti peti force-pushed the NixOS:haskell-updates branch 5 times, most recently from 33a27ef to 2a60d72 May 7, 2020
@peti peti force-pushed the NixOS:haskell-updates branch from 038e0f0 to 245a116 May 15, 2020
@hyperfekt
Copy link
Contributor Author

hyperfekt commented May 23, 2020

I suppose so, I think this PR fully subsumes that.

I still have to get back to this and make it so @peti's updates leave behind the values needed by these scripts without them having to parse commit messages.

@hyperfekt hyperfekt force-pushed the hyperfekt:hackage-packages.nix branch 2 times, most recently from ed22458 to 536cc18 May 25, 2020
@hyperfekt hyperfekt marked this pull request as ready for review May 25, 2020
@hyperfekt
Copy link
Contributor Author

hyperfekt commented May 25, 2020

I think the PR is ready now, although the update-cabal2nix-latest.sh script isn't functional yet because cabal2nix cannot use remote archives yet (see NixOS/cabal2nix#455), so it cannot update itself.

@hyperfekt hyperfekt requested a review from cdepillabout May 25, 2020
@hyperfekt hyperfekt force-pushed the hyperfekt:hackage-packages.nix branch 3 times, most recently from d25a70c to c10809d May 25, 2020
{ fetchurl }:

fetchurl {
url = "https://github.com/commercialhaskell/all-cabal-hashes/archive/6515ef12bbcf8fbac87e12b4cb30b7eefa9ce9ce.tar.gz";
sha256 = "0plf0kk0wj1lbmks09afyqrl70z0miwxzfk3zh7y2qiw3g5l1v0x";
}
# Hackage database snapshot, used by maintainers/scripts/regenerate-hackage-packages.sh
{ fetchFromGitHub }:
let
pin = builtins.fromJSON (builtins.readFile ./pin.json);
in
fetchFromGitHub (pin // { passthru.commit = pin.rev; })
Comment on lines 1 to 6

This comment has been minimized.

Copy link
@cdepillabout

cdepillabout May 26, 2020

Member

@hyperfekt I was under the impression that this file was getting updated automatically every so often.

If you change it's format here, I can imagine that it will no longer be able to be updated automatically.

This comment has been minimized.

Copy link
@hyperfekt

hyperfekt May 26, 2020

Author Contributor

There was an update from time to time but it was not in sync with the version used to generate hackage-packages.nix. I opened NixOS/cabal2nix#456 and NixOS/cabal2nix#457 to make sure that is the case in the future.

This comment has been minimized.

Copy link
@hyperfekt

hyperfekt May 26, 2020

Author Contributor

In fact it is essential that either one of those PRs is merged before this one is because otherwise the script will not be able to find the versions used for generating the current hackage-packages.nix,
as I really wanted to avoid parsing the git commit messages in it.

@@ -7,6 +7,10 @@
# files.
self: super: {

# Used by maintainers/scripts/regenerate-hackage-packages.sh, and generated
# from the latest master instead of the current version on Hackage.
cabal2nix-latest = self.callPackage ./cabal2nix-latest.nix { };

This comment has been minimized.

Copy link
@cdepillabout

cdepillabout May 26, 2020

Member

If this cabal2nix-latest will only be used by the regenerate-hackage-packages.sh, then we could probably just use IFD with callCabal2nix to generate it here based on the current master in the cabal2nix repo.

That might be simpler than having to keep around cabal2nix-latest.nix and update-cabal2nix-latest.sh.

Or maybe you could even just use callCabal2nix directly in the regenerate-hackage-packages.sh file.

This comment has been minimized.

Copy link
@hyperfekt

hyperfekt May 26, 2020

Author Contributor

I don't think we want to use the current master of the cabal2nix repo because then the version isn't pinned anymore and the current hackage-packages.nix cannot be easily reproduced. We could IFD from a pinned tarball of the cabal2nix repo, but I didn't see a good reason to not have it prebuilt by Hydra instead of built by every user of the script.

This comment has been minimized.

Copy link
@cdepillabout

cdepillabout May 26, 2020

Member

I don't think we want to use the current master of the cabal2nix repo because then the version isn't pinned anymore and the current hackage-packages.nix cannot be easily reproduced.

I was under the impression that peti would sometimes update hackage-packages.nix with different (in-developement?) versions of cabal2nix.

If that is the case, I don't know if we really get anything from keeping this cabal2nix-latest pinned here in nixpkgs.

Unless we have some sort of automatic update, it will be somewhat annoying to keep cabal2nix-latest up-to-date manually (or semi-automatically).

This comment has been minimized.

Copy link
@hyperfekt

hyperfekt May 26, 2020

Author Contributor

All statements I found declare that the file is generated from the current master of the nixos/cabal2nix, and this seems to be at least commonly the case.
Merging NixOS/cabal2nix#456 would ensure that this is always the case, as I'd argue it absolutely should be for purposes of reproducibility.
The changes in that PR also ensure that cabal2nix-latest is kept in line with the version of cabal2nix used to generate hackage-packages.nix.
The other PR, NixOS/cabal2nix#457, also updates cabal2nix-latest to the latest master but does not ensure that the checked out version of cabal2nix used to generate hackage-packages.nix actually corresponds to that - it would be on the person making the commit to not forget to push the appropriate version to Github beforehand.

This comment has been minimized.

Copy link
@cdepillabout

cdepillabout May 30, 2020

Member

It looks like NixOS/cabal2nix#457 and NixOS/cabal2nix#456 have been closed.

Does that change the approach you want to take in this PR?

This comment has been minimized.

Copy link
@hyperfekt

hyperfekt May 30, 2020

Author Contributor

I'll leave it up to someone else to fork (or change directly, if they are a maintainer) this PR if they find it useful and implement commit parsing, as I do not find myself willing to put something like that into nixpkgs.

@hyperfekt hyperfekt force-pushed the hyperfekt:hackage-packages.nix branch from c10809d to ee17816 May 26, 2020
@peti peti force-pushed the NixOS:haskell-updates branch 6 times, most recently from 6ad39b5 to 32d2de8 May 29, 2020
Introduces a script that can be used to update the Nix expressions for
the Haskell package set. In service of that, also
- introduces cabal2nix-latest, which pins the hackage2nix version used
- changes all-cabal-hashes to use fetchFromGitHub
- adds update-hackage.sh & update-cabal2nix-latest.sh maintainer scripts
@hyperfekt hyperfekt changed the base branch from haskell-updates to master May 30, 2020
@hyperfekt hyperfekt force-pushed the hyperfekt:hackage-packages.nix branch from ee17816 to 1c8851e May 30, 2020
@hyperfekt
Copy link
Contributor Author

hyperfekt commented May 30, 2020

Changed base to master to prevent diff pollution from force-pushes on haskell-updates.

Copy link
Member

peti left a comment

I've discussed this PR in in yesterday's #haskell4nix live stream in great detail starting at https://www.twitch.tv/videos/635614107?t=02h15m14s.

@hyperfekt
Copy link
Contributor Author

hyperfekt commented May 30, 2020

Let me put it this way:
I fully understand that you derive no benefit from this PR being merged.
The PR is not meant to benefit you directly.
It is meant to benefit everyone else - its goal is to make it easy for contributors to test their changes to configuration-hackage2nix.yaml, and for people to make local changes to it that are never upstreamed because they are unlikely to serve others, or that are needed more quickly than the regeneration of hackage-packages.nix performed by you.
Ultimately my hope is that this leads to an increased number of contributors to the nixpkgs Haskell package set and with them an increased quality of the package set - ideally in a virtuous cycle with an increased number of users.

It has been shown in the past that, while certainly entirely possible, the hurdle of generating the Haskell package set anew with the current process is one that many do not clear, and there is no good reason for it being remotely this burdensome, requiring every user to replicate a frankly (for Nix standards) arcane setup; when with the power of Nix it can be as simple as executing a single command. I also recall hearing about people who have gained the understanding of how to do it still finding it too troublesome to perform.

I have chosen to include these scripts in nixpkgs because

  • the required hackage2nix being served from the cache further lowers the friction of contributing, and with a prebuilt hackage2nix the cabal2nix repository would have to be fetched just for these scripts,
  • I see a very low chance of this kind of portable script which is not tailored to your usecase actually being merged in the cabal2nix repository, and
  • I am convinced scripts that the nixpkgs contributors use to operate on nixpkgs should also live in nixpkgs.

Your declaration of not maintaining them I take absolutely no issue with (I'm happy to do that), and your not wanting to use them I have accepted. I don't think either of these are reasons not to include them. Again, they are not for you.

I do however need your minimal support in pinning the versions used to generate hackage-packages.nix in nixpkgs. Users will often work off of older versions either because they cannot trivially upgrade, or because they need the cache provided by the NixOS foundation.

I'm not sure I see the harm in adding this sort of pinning to the scripts you use, so maybe you could enlighten me about the problems it brings instead of outright rejecting it without giving a reason.

PS: I very much appreciate your intent to make the package generation more efficient through the direct use of Cabal's tarballs. But until that is implemented I think these scripts are very useful, and even when implemented we would want (slimmer, simpler) scripts as well as pinning in order to not increase the friction to more than necessary.

@maralorn
Copy link
Contributor

maralorn commented May 31, 2020

@peti I have thought about this a bit more. There is one reason, why we maybe want more reproducibility than we have right now.
When someone makes a change to the yaml-file, they should probably regenerate the hackage-packages.nix, so that they can

  1. test their change and
  2. contribute a PR which already works and can be tested easily.

But now if everyone where to generate the hackage-packages.nix with the e.g. cabal tarball available to them at that time, this will very often produce changes at different points in hackage-packages.nix. This has multiple disadvantages:

  1. It is harder to see if one accidentally changed anything else. That might be a problem for the person creating the PR and for the one reviewing it. It could even introduce evaluation errors that confuse the contributer (or at least prevent easy testing) without anything to do with the change being made.
  2. In general if everyone were to regenerate hackage-packages.nix with some version of hackage, PRs would get confusing, might create much more rebase problems and a lot more rebuilds (possibly even on the contributers machine) that don‘t have anything to do with the actual change.
  3. I don‘t want to think about the mess, when someone accidentally uses a slightly too old hackage version and rolls back some changes without realizing …

So I think the case can be made, that pinning the hackage version to generate hackage-packages.nix would make the contribution process much smoother. Because contributers could update exactly the part they changed, PRs would have a cleaner separation, would be easier to test and (Bonus!) situations where the PR causes a problem, when you regenerate the hackage-packages.nix, i.e. problems you need to solve, get rarer.

So I think the over all goal should be: a) Make the regeneration of the nix file easy and b) make the regeneration with the same hackage version easy.

Now, I have said nothing about what the best way to achieve that would be. But this PR supplies a solution that works right now. Another approach would be to have hackage2nix print the hackage snapshot to the top of the nix file (I know it‘s written in the commit message but that is harder to extract for tooling) and then have some tool (probably in the hackage2nix repo) that can pull that exact hackage version, be it a git-repo or a tarball …

@hyperfekt
Copy link
Contributor Author

hyperfekt commented Jun 1, 2020

Looking at make-package-set.nix, I think this PR currently breaks callHackage, which assumes all-cabal-hashes to be a tarball. The most direct solution is to revert it to be a tarball, and unpack it only when regenerating the package set. As an added benefit I think I can save a lot of time on slow filesystems by using the tarball via ratarmount. In the long term the direct use of the cabal tarball that peti suggested would probably be most appropriate.

@peti
Copy link
Member

peti commented Jun 2, 2020

There is one reason, why we maybe want more reproducibility than we have right now.

Just to avoid misunderstandings: we have reproducibility. hackage2nix records the version of itself and of Hackage that was used to generate the hackage-packages.nix file. What don't have is a --re-generate flag to update-nixpkgs.sh that resets the Hackage snapshot to the version that was used to generate the current Nixpkgs checkout, so that users can easily take advantage of that information.

@maralorn
Copy link
Contributor

maralorn commented Jun 3, 2020

I have tried to verify your claim. I remain unconvinced as of now:

NixOS/cabal2nix#459

It is likely that the issue is my mistake. But I needed 30 minutes to get to that point. I think the Haskell tooling in nixpkgs is great. I am really amazed by it. But I think it can be made easier to use.

@maralorn
Copy link
Contributor

maralorn commented Jun 12, 2020

I want to mention a script I hacked together which does at least some of this. @peti merged it into the cabal2nix repo.

NixOS/cabal2nix#461

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants
You can’t perform that action at this time.