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

postgresql: drop build-time dependency on GHC #354538

Merged
merged 1 commit into from
Nov 9, 2024
Merged

Conversation

Ma27
Copy link
Member

@Ma27 Ma27 commented Nov 8, 2024

This replaces writeShellApplication with writeShellScriptBin that doesn't perform a shellcheck. This makes it way easier to build postgresql on staging since GHC is super slow to build, even with pretty powerful machines.

Also Haskell updates are currently merged straight into master which means that postgresql and all reverse dependencies require a rebuild on master then[1].

[1] #354270 (comment)

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 24.11 Release Notes (or backporting 23.11 and 24.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

@Ma27 Ma27 requested a review from a team November 8, 2024 16:51
Copy link
Contributor

@wolfgangwalther wolfgangwalther left a comment

Choose a reason for hiding this comment

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

Makes total sense. If we ever come around to doing something like #353490, then the separate .sh file would be checked that way.

@wolfgangwalther
Copy link
Contributor

I'm not sure about the shebang and the shell options that this might change. Do we need to do some set -e etc.? The docs for writeShellApplication suggest that this does a bit more than writeShellScriptBin.

This replaces `writeShellApplication` with `writeShellScriptBin` that
doesn't perform a shellcheck. This makes it way easier to build
postgresql on staging since GHC is super slow to build, even with pretty
powerful machines.

Also Haskell updates are currently merged straight into master which
means that postgresql and all reverse dependencies require a rebuild on
master then[1].

[1] NixOS#354270 (comment)
@Ma27
Copy link
Member Author

Ma27 commented Nov 8, 2024

Good point, added the options that writeShellAPplication added.

@sternenseemann
Copy link
Member

Makes total sense. If we ever come around to doing something like #353490, then the separate .sh file would be checked that way.

It is probably also feasible to add passthru.tests for this. I was considering whether writeShellApplication should maybe have a flag (for in nixpkgs use) that moves the checkPhase to passthru.tests.

@ofborg ofborg bot requested review from globin, ivan and wolfgangwalther November 8, 2024 21:41
@wegank wegank added 12.approvals: 2 This PR was reviewed and approved by two reputable people 12.approved-by: package-maintainer This PR was reviewed and approved by a maintainer listed in the package labels Nov 8, 2024
@Ma27 Ma27 merged commit 8771747 into NixOS:staging Nov 9, 2024
34 of 35 checks passed
@Ma27 Ma27 deleted the pg-no-haskell branch November 9, 2024 13:07
@wolfgangwalther
Copy link
Contributor

Makes total sense. If we ever come around to doing something like #353490, then the separate .sh file would be checked that way.

It is probably also feasible to add passthru.tests for this. I was considering whether writeShellApplication should maybe have a flag (for in nixpkgs use) that moves the checkPhase to passthru.tests.

That would make it possible to check all setup hooks, too, because it avoids the bootstrap-problem. But, we have a few more shell scripts in the repo that are not connected to nix code, imho. So the CI check should be able to work on more files.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
10.rebuild-darwin: 501+ 10.rebuild-darwin: 2501-5000 10.rebuild-linux: 501+ 10.rebuild-linux: 5001+ 11.by: package-maintainer This PR was created by the maintainer of the package it changes 12.approvals: 2 This PR was reviewed and approved by two reputable people 12.approved-by: package-maintainer This PR was reviewed and approved by a maintainer listed in the package
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants