Skip to content

pkgs/top-level: init expression for haskell-updates jobset #121407

Merged
maralorn merged 2 commits intoNixOS:haskell-updatesfrom
sternenseemann:haskell-hydra
May 3, 2021
Merged

pkgs/top-level: init expression for haskell-updates jobset #121407
maralorn merged 2 commits intoNixOS:haskell-updatesfrom
sternenseemann:haskell-hydra

Conversation

@sternenseemann
Copy link
Copy Markdown
Member

@sternenseemann sternenseemann commented May 1, 2021

release-haskell.nix is intended to be a replacement for
https://github.com/peti/ci/blob/master/haskell-nixpkgs.nix
which is currently the main expression for the haskell-updates jobset
on hydra (in the nixpkgs project).

It has the same jobs as the old haskell-nixpkgs.nix file:

  • haskellPackages.*
  • haskell.compiler.*
  • Some extra haskell packages for certain compilers

The following jobs are new:

  • tests.haskell.*
  • A manually maintained list of top-level haskell packages (most of them
    using justStaticExecutables)
  • An aggregate job which is intended to aid merging the haskell-updates
    branch: It holds an arbitrary list of haskell-related packages and
    tests we intend have working at all times. This is still somewhat
    incomplete and should be extendend in the future.

Additionally a lot of refactoring has been done and some unnecessary
code has been eliminated. Due to the increased set of jobs and my
ideas of convenience however, the code size has grown overall.
I've tried document the individual parts and would be happy about
feedback in general.

One future improvement could be making adding top-level haskell packages
more convenient and adding them all to the aggregate job automatically.

Evaluation is currently at 20s on my laptop, I suspect the main offender is
versionedCompilerJobs, but haven't done any testing

Contains some commits of #121386.

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.

@sternenseemann sternenseemann changed the title Haskell hydra pkgs/top-level: init expression for haskell-updates jobset May 1, 2021
@ofborg ofborg bot added 8.has: clean-up This PR removes packages or removes other cruft 8.has: package (new) This PR adds a new package 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. labels May 1, 2021
@maralorn
Copy link
Copy Markdown
Member

maralorn commented May 1, 2021

This looks very good to me.

I am not enough of an expert to judge if this actually does exactly what we want on hydra. But I’ll get we will figure that out.

I like the mergeable job a lot. My suggestion would be to add the top-level package haskell-language-server in there.

@github-actions

This comment has been minimized.

Sign in to view
@ofborg ofborg bot added 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux. and removed 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. labels May 1, 2021
maralorn
maralorn approved these changes May 1, 2021
View reviewed changes
Copy link
Copy Markdown
Member

@maralorn maralorn left a comment

Choose a reason for hiding this comment

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

I think this is nice. Let’s try it and improve on it, if we notice any problems.

cdepillabout
cdepillabout reviewed May 2, 2021
View reviewed changes
cdepillabout
cdepillabout reviewed May 2, 2021
View reviewed changes
@cdepillabout
Copy link
Copy Markdown
Member

cdepillabout commented May 2, 2021

This looks good to me, although I really don't have enough experience with Hydra to give an in-depth review.

cdepillabout
cdepillabout reviewed May 2, 2021
View reviewed changes
andir
andir reviewed May 2, 2021
View reviewed changes
Copy link
Copy Markdown
Member

@andir andir left a comment

Choose a reason for hiding this comment

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

Doesn't look too bad. Just some quality of life comments. I started a test eval on my private hydra instance: https://hydra.h4ck.space/jobset/nixpkgs/haskell-update#tabs-evaluations (ipv6-only)

pkgs/top-level/release-haskell.nix Outdated
Copy link
Copy Markdown
Member

@andir andir May 2, 2021

Choose a reason for hiding this comment

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

Shouldn't this be the same as with the other release jobs? I am thinking that at least aarch64-linux should be in here as well.

Copy link
Copy Markdown
Member Author

@sternenseemann sternenseemann May 2, 2021

Choose a reason for hiding this comment

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

This is like this for historical reasons. Since haskell packages get built on aarch64-linux and x86_64-darwin in the main jobsets as well, I think we can enable it here as well. I just want to poke the infra team if it's okay.

pkgs/top-level/release-haskell.nix Outdated
Copy link
Copy Markdown
Member

@andir andir May 2, 2021

Choose a reason for hiding this comment

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

What is the requirement for a job to be in this section? What does it mean when one of them breaks on on compiler but not on another?

Copy link
Copy Markdown
Member Author

@sternenseemann sternenseemann May 3, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

I've elaborated on this a bit in a comment. I think what warrants inclusion will become clearer in the future, as we may want to expand the list of packages there.

pkgs/top-level/release-haskell.nix Outdated
Copy link
Copy Markdown
Member

@andir andir May 2, 2021

Choose a reason for hiding this comment

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

Why is this called mergeable? I guess this is the equivalent of the tested jobset for haskell? The description could probably be better here.

Copy link
Copy Markdown
Member

@cdepillabout cdepillabout May 2, 2021

Choose a reason for hiding this comment

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

We talked about this shortly in our meeting:

This is an aggregate job that contains all "important" Haskell packages. If this is not passing, then we should not merge haskell-updates into master. In Hydra UI this shows up as a single job.

Copy link
Copy Markdown
Member Author

@sternenseemann sternenseemann May 3, 2021

Choose a reason for hiding this comment

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

I've elaborated the description, I hope Hydra can deal with newlines in that field.

pkgs/top-level/release-haskell.nix Outdated
Copy link
Copy Markdown
Member

@andir andir May 2, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

What are the consequences of a build failure for packages in this category? Does that mean someone has to look? Mind specifying that here (in the description).

Copy link
Copy Markdown
Member

@cdepillabout cdepillabout May 2, 2021

Choose a reason for hiding this comment

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

We talked about this shortly in our meeting today:

This is an aggregate job that contains all Haskell packages that have maintainer. If one of these packages is not building, then we should mark the package as broken and leave the maintainer as-is. We also need to ping them on the PR for the haskell-updates branch so that they have time to fix the problem.

We agreed that we need to give the maintainers at least 7 days to fix their packages before we mark them broken and merge haskell-updates into master.

Copy link
Copy Markdown
Member Author

@sternenseemann sternenseemann May 3, 2021

Choose a reason for hiding this comment

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

I wouldn't prescribe a consequence of this at this time, for now I think this is mostly a nice source for an overview.

@cdepillabout cdepillabout added the 6.topic: haskell General-purpose, statically typed, purely functional programming language label May 2, 2021
@cdepillabout cdepillabout mentioned this pull request May 2, 2021
Closed
@maralorn
Copy link
Copy Markdown
Member

maralorn commented May 2, 2021

So, @sternenseemann do you wanna add a bit more comments regarding to those questions? Or do we just merge (with the intention to document the meaning of those jobs somewhere else)?

@cdepillabout
Copy link
Copy Markdown
Member

cdepillabout commented May 3, 2021

Ah, one other request: could you add the file in this PR to both .github/CODEOWNERS and .github/labeler.yml so that we get notified when it is changed?

@sternenseemann sternenseemann requested a review from edolstra as a code owner May 3, 2021 09:00
@github-actions github-actions bot added 6.topic: policy discussion Discuss policies to work in and around Nixpkgs and removed 6.topic: haskell General-purpose, statically typed, purely functional programming language labels May 3, 2021
@sternenseemann
pkgs/top-level: init expression for haskell-updates jobset
ef9631e 
release-haskell.nix is intended to be a replacement for
https://github.com/peti/ci/blob/master/haskell-nixpkgs.nix
which is currently the main expression for the haskell-updates jobset
on hydra (in the nixpkgs project).

It has the same jobs as the old haskell-nixpkgs.nix file:

* haskellPackages.*
* haskell.compiler.*
* Some extra haskell packages for certain compilers

The following jobs are new:

* tests.haskell.*
* A manually maintained list of top-level haskell packages (most of them
  using justStaticExecutables)
* An aggregate job which is intended to aid merging the haskell-updates
  branch: It holds an arbitrary list of haskell-related packages and
  tests we intend have working at all times. This is still somewhat
  incomplete and should be extendend in the future.

Additionally a lot of refactoring has been done and some unnecessary
code has been eliminated. Due to the increased set of jobs and my
ideas of convenience however, the code size has grown overall.
I've tried document the individual parts and would be happy about
feedback in general.

One future improvement could be making adding top-level haskell packages
more convenient and adding them all to the aggregate job automatically.
@sternenseemann
pkgs/release-haskell.nix: add aggregate job of all pkgs with maintainers
233682c
@maralorn
Copy link
Copy Markdown
Member

maralorn commented May 3, 2021

Okay, let’s do this!

@maralorn maralorn merged commit 7a35321 into NixOS:haskell-updates May 3, 2021
@maralorn
Copy link
Copy Markdown
Member

maralorn commented May 3, 2021

Thx!

@sternenseemann sternenseemann deleted the haskell-hydra branch May 3, 2021 11:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cdepillabout cdepillabout cdepillabout left review comments

@andir andir andir left review comments

@maralorn maralorn maralorn approved these changes

@peti peti Awaiting requested review from peti

@edolstra edolstra Awaiting requested review from edolstra

Assignees

No one assigned

Labels

6.topic: policy discussion Discuss policies to work in and around Nixpkgs 8.has: clean-up This PR removes packages or removes other cruft 8.has: package (new) This PR adds a new package 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@sternenseemann @maralorn @cdepillabout @andir