Changes necessary to build on GHC 8.4.3 by kritzcreek · Pull Request #3372 · purescript/purescript

kritzcreek · 2018-05-31T09:33:43Z

This isn't ready just yet, because we need to depend on a GIT version of spdx right now. The master for that package already builds, it just hasn't been released to Hackage yet.

kritzcreek · 2018-05-31T10:01:27Z

Reminder, ping @cnd when we merge this

(use upstream PR purescript/purescript#3372)

Cynede · 2018-05-31T10:23:30Z

@kritzcreek thank you! I can verify that this patch works for me with 0.12 release and ghc 8.4.3 on Gentoo 👍

kritzcreek · 2018-05-31T10:47:28Z

Looks like we're running into haskell/network#313 / commercialhaskell/stack#3944 for our AppVeyor build.

Presumably that'll be fixed with network 2.7.0.1 but we'll have to old off until that's released and we tested that it fixes the Windows issues.

ilovezfs · 2018-07-05T07:22:29Z

@kritzcreek https://hackage.haskell.org/package/network-2.7.0.1 was released

angerman · 2018-07-27T13:06:52Z

What's this blocked on? As far as I can tell from travis the jobs just timed out?

kritzcreek · 2018-07-27T13:10:55Z

@angerman We need a release of spdx here: phadej/spdx#12 but the author is unresponsive so we might need to consider forking and maintaining it ourselves.

@hdgarrood I think you introduced spdx to verify a proper LICENSE is set before updating documentation to Pursuit? Can we simplify that check and vendor it or should we fork the package instead?

angerman · 2018-07-27T15:50:32Z

@phadej any advice on how to handle that?

hdgarrood · 2018-07-27T15:59:10Z

I don’t think it’s particularly complex, it’s just not the kind of thing I’d want to vendor if i could avoid to because it could be quite annoying for users if we get it wrong. Although I suppose we might not have much choice.

I don’t have any strong preference for forking vs vendoring except that I suppose the former might imply more of a maintenance commitment which perhaps makes it less desirable. Do you mind if I leave it up to you?

hdgarrood · 2018-07-27T16:04:05Z

I suppose we could also simplify the check to “is the license field present and not equal to AllRightsReserved” or something similar, but I think it’s important to at least have something in place which helps prevent people uploading things to Pursuit without a genuine open source license.

angerman · 2018-07-29T00:34:18Z

Could you use lib:Cabal for this?

http://hackage.haskell.org/package/Cabal-2.2.0.1/docs/Distribution-SPDX.html

/cc @phadej, @hvr

kritzcreek · 2018-07-29T06:17:43Z

I suppose we could also simplify the check to “is the license field present and not equal to AllRightsReserved” or something similar, but I think it’s important to at least have something in place which helps prevent people uploading things to Pursuit without a genuine open source license.

So this is purely a client-side check? Is the check just meant to discourage people from uploading non-open-source licensed library or strictly prevent them? You can just upload things to Pursuit via HTTP so I don't think it's sufficient if we want the latter.

Do you mind if I leave it up to you?

I don't have the capacities to maintain a fork, nor do I know my way around licenses to make a good educated decision, sorry... I'm just going to delete the SPDX dependent code in this PR, and if you can come up with a better solution you can just commit to my branch (collaborators have push access)

phadej · 2018-07-29T17:45:05Z

Why purescript is using spdx, is there something not in Cabal-2.2 http://hackage.haskell.org/package/Cabal-2.2.0.1/docs/Distribution-SPDX.html (note Parsec and Pretty instances).

I'd not recommend to use spdx as is any longer, next version will reuse Cabal definitions anyway.

phadej · 2018-07-29T17:47:44Z

As for why it's not released: I haven't updated spdx license list in spdx since 2017. spdx uses has 2.6, iCabal-2.2 has 3.0, and latest one is Version 3.2 2018-07-10; (there was 3.1 in between)

hdgarrood · 2018-07-29T19:23:52Z

I wasn't aware that the Cabal library provided this; I think that would be a good option as well.

phadej · 2018-07-30T09:37:02Z

@hdgarrood I'm more committed to updating Cabal than spdx. The plan to spdx is to have "legal" stuff, e.g. it have satisfies and equivalent functions to compare SPDX expressions - but those are out of scope of Cabal for now

kritzcreek · 2018-07-31T00:10:04Z

We're currently having trouble with our CI builds timing out and work towards reducing the compilation times (eg. see #3400). So pulling in a dependency on the Cabal library for a check that should really live in Pursuit anyway doesn't excite me.

phadej · 2018-07-31T10:10:45Z

Cabal-2.2 comes with GHC-8.4, you don't need to install it.

kritzcreek · 2018-07-31T10:25:27Z

Do I also not need to compile and link it?

phadej · 2018-07-31T10:38:22Z

You obviously need to link it, but as it comes with GHC you don't need to compile it (the objects files are there already); In the some way you don't need to compile containers or deepseq, for example.

kritzcreek · 2018-07-31T10:49:35Z

That's good to know, thanks!

This reverts commit 9432705.

kritzcreek · 2018-08-05T11:09:17Z

Allright, so I've changed the code to use Distribution.SPDX and added a dependency on Cabal. I'll open another issue about it because I think ideally we'd just not check these licenses inside the compiler, but for now I just want to get this merged so we don't lag behind too far.

Does this work for you @hdgarrood?

hdgarrood · 2018-08-05T13:30:00Z

 --
 isValidSPDX :: String -> Bool
-isValidSPDX = (== 1) . length . SPDX.parseExpression
+isValidSPDX = isJust . SPDX.mkLicenseId


As I understand it, this will attempt to parse a license ID rather than a license expression, which means that e.g. "GPL-2.0+", meaning GPL 2.0 or any later version, would not be accepted. From looking at the docs, I think we need to use the Parsec instance for License, and then check that a) the parse succeeds and b) that the result is not NONE.

I see, thanks for checking. How about now?

Looks good now, thanks!

kritzcreek · 2018-08-09T08:27:38Z

@LiamGoodacre or @garyb could I get a review on this?

LiamGoodacre

Looks great!

kritzcreek · 2018-08-09T09:20:49Z

Thanks!

kritzcreek · 2018-08-09T09:21:25Z

Ping @cnd as promised ;)

* changes necessary to build on GHC 8.4.2 * remove redundant Monoid imports * bumps to a 8.4.3 resolver * Updates for LTS-12 * Work around cabal/stack bug * Update node and stack versions on appveyor * removes spdx dependent code * Revert "removes spdx dependent code" This reverts commit 9432705. * use Cabal's SPDX parsing * use Cabal's Parsec class to parse licenses

changes necessary to build on GHC 8.4.2

d7e1618

kritzcreek added the needs more work label May 31, 2018

This was referenced May 31, 2018

Fixes ambiguous occurrence ‘<>’ with Prelude.Compat #3370

Closed

Restrict base for GHC < 8.4.1 #3294

Merged

Release current master to Hackage so we can build against 8.4.2 phadej/spdx#12

Closed

remove redundant Monoid imports

339f3c1

Cynede added a commit to gentoo-haskell/gentoo-haskell that referenced this pull request May 31, 2018

dev-lang/purescript: fix compilation with 8.4

e15fcf8

(use upstream PR purescript/purescript#3372)

Cynede added a commit to gentoo-haskell/gentoo-haskell that referenced this pull request May 31, 2018

dev-lang/purescript: fix compilation with 8.4

a9681bc

(use upstream PR purescript/purescript#3372)

ilovezfs mentioned this pull request Jun 3, 2018

Formulae using GHC older than 8.4 Homebrew/homebrew-core#25568

Closed

19 tasks

bumps to a 8.4.3 resolver

914f87d

kritzcreek changed the title ~~Changes necessary to build on GHC 8.4.2~~ Changes necessary to build on GHC 8.4.3 Jun 18, 2018

kritzcreek added 3 commits July 10, 2018 11:24

Updates for LTS-12

c0e7935

Work around cabal/stack bug

342f73f

Update node and stack versions on appveyor

8ca76a2

removes spdx dependent code

9432705

kritzcreek added 2 commits August 5, 2018 12:24

Revert "removes spdx dependent code"

8e8165a

This reverts commit 9432705.

use Cabal's SPDX parsing

4190752

hdgarrood reviewed Aug 5, 2018

View reviewed changes

use Cabal's Parsec class to parse licenses

1d604bb

kritzcreek force-pushed the ghc-8.4.2 branch from 0175cc3 to 1d604bb Compare August 5, 2018 18:52

kritzcreek added status: ready for review and removed needs more work labels Aug 5, 2018

LiamGoodacre approved these changes Aug 9, 2018

View reviewed changes

kritzcreek merged commit e184fca into purescript:master Aug 9, 2018

kritzcreek deleted the ghc-8.4.2 branch August 9, 2018 09:20

paluh mentioned this pull request Sep 18, 2018

Purescript fail to build on darwin NixOS/nixpkgs#42833

Closed

paluh mentioned this pull request Oct 4, 2018

Add nix config to stack.yaml #3435

Merged

natefaubion mentioned this pull request Nov 9, 2018

0.12.1 Release Checklist #3451

Closed

Conversation

kritzcreek commented May 31, 2018

Uh oh!

kritzcreek commented May 31, 2018

Uh oh!

Cynede commented May 31, 2018

Uh oh!

kritzcreek commented May 31, 2018

Uh oh!

ilovezfs commented Jul 5, 2018

Uh oh!

angerman commented Jul 27, 2018

Uh oh!

kritzcreek commented Jul 27, 2018

Uh oh!

angerman commented Jul 27, 2018

Uh oh!

hdgarrood commented Jul 27, 2018

Uh oh!

hdgarrood commented Jul 27, 2018

Uh oh!

angerman commented Jul 29, 2018

Uh oh!

kritzcreek commented Jul 29, 2018

Uh oh!

phadej commented Jul 29, 2018

Uh oh!

phadej commented Jul 29, 2018

Uh oh!

hdgarrood commented Jul 29, 2018

Uh oh!

phadej commented Jul 30, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kritzcreek commented Jul 31, 2018

Uh oh!

phadej commented Jul 31, 2018

Uh oh!

kritzcreek commented Jul 31, 2018

Uh oh!

phadej commented Jul 31, 2018

Uh oh!

kritzcreek commented Jul 31, 2018

Uh oh!

kritzcreek commented Aug 5, 2018

Uh oh!

hdgarrood Aug 5, 2018

Choose a reason for hiding this comment

Uh oh!

kritzcreek Aug 5, 2018

Choose a reason for hiding this comment

Uh oh!

hdgarrood Aug 5, 2018

Choose a reason for hiding this comment

Uh oh!

kritzcreek commented Aug 9, 2018

Uh oh!

LiamGoodacre left a comment

Choose a reason for hiding this comment

Uh oh!

kritzcreek commented Aug 9, 2018

Uh oh!

kritzcreek commented Aug 9, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

phadej commented Jul 30, 2018 •

edited

Loading