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

rename package to spago-legacy to help NixPkgs #3

Merged
merged 4 commits into from
Jul 28, 2024
Merged

rename package to spago-legacy to help NixPkgs #3

merged 4 commits into from
Jul 28, 2024

Conversation

peterbecich
Copy link
Contributor

@peterbecich peterbecich commented Jul 6, 2024 •
edited
Loading

Description of the change

This package is currently spago in NixPkgs:
https://search.nixos.org/packages?channel=unstable&from=0&size=50&sort=relevance&type=packages&query=spago

The Nix configuration for that is generated from this Cabal file. Renaming to spago-legacy will help clear the path for NixPkgs update to the new Spago.

I think it's unnecessary for the spago binary to be renamed to spago-legacy.

If this PR is merged, NixPkgs will be updated to the latest commit in this repo spago-legacy.

cabal build --enable-tests

succeeds with GHC 9.2

stack build succeeds.

Please merge this PR and make a fix version/release 0.21.1 or maybe even 0.21.0.1. This is probably necessary for Nix to grab the latest commit, I don't think it works directly off the branch.

Checklist:

  • Added the change to the "Unreleased" section of the changelog
  • Added some example of the new feature to the README
  • Added a test for the contribution (if applicable)

P.S.: the above checks are not compulsory to get a change merged, so you may skip them. However, taking care of them will result in less work for the maintainers and will be much appreciated 😊

@peterbecich
rename package to spago-legacy
02f95a4 
This package is currently `spago` in NixPkgs:
https://search.nixos.org/packages?channel=unstable&from=0&size=50&sort=relevance&type=packages&query=spago

The Nix configuration for that is generated from this Cabal file.
Renaming to `spago-legacy` will help NixPkgs update to the new Spago.

I think it's unnecessary for the `spago` binary to be renamed.
@peterbecich peterbecich marked this pull request as ready for review July 6, 2024 02:09
This was referenced Jul 6, 2024
Draft
Closed
@f-f
Copy link
Member

f-f commented Jul 6, 2024 •
edited
Loading

We don't have release CI set up on this repo, so it might take more work to build the binaries. Will nixpkgs need the binaries or will it build them off the branch? In other words: will just cutting the release tag enough for nixpkgs?

@peterbecich
Copy link
Contributor Author

NixPkgs will not need the binaries.

From what I can understand about this script
https://github.com/NixOS/nixpkgs/blob/9860009d87706af95eb94bc5369f86741e047135/pkgs/development/tools/purescript/spago-legacy/update.sh#L22-L31
the release bundle is necessary but you can omit the binaries. In fact, the release bundle can probably be empty. A tag without a release is insufficient. The script is going to grab the latest item in https://api.github.com/repos/purescript/spago-legacy/releases, parse the tag from that, and then cabal2nix will fetch that tag.

Thanks

@f-f
Copy link
Member

f-f commented Jul 7, 2024

Thanks, that makes sense. I can merge this PR and cut a new tag, but we'll need to get CI going first - the macOS build fails with the purescript installer failing Check if a prebuilt 0.15.8 binary is provided for darwin-arm64. Which is strange because the build is not running on an ARM machine.

@peterbecich
Copy link
Contributor Author

peterbecich commented Jul 13, 2024 •
edited
Loading

I can disable the unnecessary jobs in .github in this PR, leaving only build ubuntu-latest and Cabal build, is that acceptable?

@peterbecich
Copy link
Contributor Author

peterbecich commented Jul 13, 2024 •
edited
Loading

I don't think the Mac build is necessary to support the Darwin platform of the Nix build:

https://search.nixos.org/packages?channel=unstable&show=spago&from=0&size=50&sort=relevance&type=packages&query=spago
Platforms

[aarch64-darwin](https://hydra.nixos.org/job/nixos/trunk-combined/nixpkgs.spago.aarch64-darwin)
[aarch64-linux](https://hydra.nixos.org/job/nixos/trunk-combined/nixpkgs.spago.aarch64-linux)
[i686-linux](https://hydra.nixos.org/job/nixos/trunk-combined/nixpkgs.spago.i686-linux)
[x86_64-darwin](https://hydra.nixos.org/job/nixos/trunk-combined/nixpkgs.spago.x86_64-darwin)
[x86_64-linux](https://hydra.nixos.org/job/nixos/trunk-combined/nixpkgs.spago.x86_64-linux)

It is going to build Spago directly from source, given the release package.

peterbecich
peterbecich commented Jul 13, 2024
View reviewed changes
.github/workflows/build.yml
@@ -39,7 +39,7 @@ jobs:
- uses: actions/checkout@v2
- name: Setup Haskell (macOS and Windows)
if: runner.os != 'Linux'
uses: haskell/actions/setup@v1
uses: haskell-actions/setup@v2
Copy link
Contributor Author

Choose a reason for hiding this comment

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

https://github.com/haskell-actions/setup

@peterbecich
Copy link
Contributor Author

@f-f please review again

f-f
f-f reviewed Jul 14, 2024
View reviewed changes
.github/workflows/release.yml Outdated Show resolved Hide resolved
spago-legacy.cabal Outdated Show resolved Hide resolved
@peterbecich peterbecich requested a review from f-f July 16, 2024 05:34
@peterbecich
Copy link
Contributor Author

@f-f please review again

@f-f f-f merged commit 2790261 into purescript:master Jul 28, 2024
2 checks passed
@f-f
Copy link
Member

f-f commented Jul 28, 2024

I merged and cut the new release. Please keep me posted on the progress in nixpkgs!

I think it's unnecessary for the spago binary to be renamed to spago-legacy.

Let's do the renaming, it's quite confusing to have the two things share the same binary name.

peterbecich added a commit to peterbecich/nixpkgs that referenced this pull request Jul 28, 2024
@peterbecich
haskellPackages.spago: rename to spago-legacy
b10e5fa 
The Spago package in NixPkgs has moved from
https://github.com/purescript/spago to https://github.com/purescript/spago-legacy.

I believe the new `spago` https://github.com/purescript/spago is a rewrite from Haskell to PureScript. To support this
new Spago in NixPkgs will probably require rewriting all of the Nix configuration. I.m.o. the easiest way to
clear the path for this is to rename the old Nix configuration from `spago` to `spago-legacy`.

Because part of this is generated by `cabal2nix`, changes to the `spago-legacy` Cabal file are needed: purescript/spago-legacy#3
@peterbecich
Copy link
Contributor Author

Thanks @f-f , here is the WIP NixOS/nixpkgs#324944

I will rename the binary to spago-legacy. This may require another PR and release, t.b.d.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@f-f f-f f-f approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@peterbecich @f-f