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.

Sign up for GitHub

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

psc-package: Stop using haskellPackages to build #82964

Merged
merged 1 commit into from Apr 1, 2020
Merged

psc-package: Stop using haskellPackages to build #82964

merged 1 commit into from Apr 1, 2020

Conversation

lilyball
Copy link
Member

@lilyball lilyball commented Mar 19, 2020 •
edited

Motivation for this change

The current psc-package doesn't compile anymore, both becasue it's not compatible with GHC 8.8 and because the version of base-compat in haskellPackages is newer than what it supports.

The new approach just downloads a prebuilt binary and patches the linker paths. This means we're using the official supported compiler so we match other installations of psc-package, at the downside of fewer supported platforms. This was the recommended approach from purescript/psc-package#165.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS (with sandbox)
    • macOS (without sandbox)
    • 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.

I tested one package that depends on this (nodePackages.insect). I don't anticipate any issues with dependent packages as this is the same version of psc-package as before, so it should behave the same.

CC @Profpatsch

@lilyball lilyball mentioned this pull request Mar 19, 2020
Closed
@ofborg ofborg bot requested a review from Profpatsch March 19, 2020 21:48
lilyball
lilyball commented Mar 19, 2020
View reviewed changes
pkgs/development/compilers/purescript/psc-package/default.nix
description = "A package manager for PureScript based on package sets";
license = licenses.bsd3;
maintainers = with maintainers; [ Profpatsch ];
platforms = [ "x86_64-darwin" "x86_64-linux" ];
Copy link
Member Author

@lilyball lilyball Mar 19, 2020 •
edited

Choose a reason for hiding this comment

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

https://github.com/justinwoo/easy-purescript-nix/blob/master/psc-package-simple.nix did not contain a platforms declaration so I created this based on the releases. I'm not sure if there are any other platforms besides "x86_64-linux" that work with the linux64 binary download.

@lilyball
Copy link
Member Author

psc-package also contains a win64 release but easy-purescript-nix doesn't have install instructions for it and I don't know how to create my own.

psc-package: Stop using haskellPackages to build
4ebc227 
Switch to the approach taken by
https://github.com/justinwoo/easy-purescript-nix/blob/master/psc-package-simple.nix

This downloads a prebuilt release and patches the linker paths. It
reduces the number of supported platforms, but ensures we're using the
official supported psc-package compiler. The `haskellPackages` approach
wasn't supported and was leading to version conflicts with dependencies.
@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/prs-ready-for-review-may-2019/3032/135

@lilyball lilyball mentioned this pull request Mar 28, 2020
Merged
10 tasks
@cdepillabout
Copy link
Member

@lilyball Looks good to me. A lot of the other PureScript stuff is just installing from binaries, so it makes sense to do the same thing for psc-package.

I'll merge this in, but I'd appreciate if you send a follow-up PR adding yourself as a maintainer if psc-package is something you depend on.

@cdepillabout cdepillabout merged commit 4c23f68 into NixOS:master Apr 1, 2020
@lilyball
Copy link
Member Author

lilyball commented Apr 1, 2020

I didn't add myself as a maintainer because I'm not a direct consumer of this, I just wanted to use a package that itself depends on this. I'd rather not be responsible for psc-package going forward.

@lilyball lilyball deleted the psc-package branch April 1, 2020 23:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Profpatsch Profpatsch Awaiting requested review from Profpatsch

Assignees
No one assigned
Labels
6.topic: haskell 8.has: clean-up 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@lilyball @nixos-discourse @cdepillabout