Add explicit import of Monoid <> - #2278

Not surprisingly the build broke due to the unused import warnings. Have used qualified import of Options.Applicative as one alternative (it's unfortunate there are so many functions from OPA). Again, happy to do whatever people find the most pleasant.

Hi, thanks for the report. How did you come across this issue? Were you trying to compile PureScript yourself with cabal-install?

I'm open to alternative viewpoints here, but I think my preferred approach would be to not worry about whether PureScript builds with the latest versions of everything. Ideally everyone would use the exact same install plan; the more possible install plans there are, the more potential there is for problems, as different versions of dependencies could behave in different ways. We have had this kind of problem in the past, before we started using Stack everywhere — I think some people were seeing bugs that we weren't seeing because cabal-install had selected a different version of language-javascript when they built it.

I think we should update CONTRIBUTING.md because it still suggests using cabal-install. We should suggest using Stack instead, I think. Regarding cabal-install, we could explain that you can get a cabal.config file corresponding to the snapshot we're currently using by visiting eg. https://www.stackage.org/lts-6.7/cabal.config, and then you can build PureScript using cabal-install and still get the same install plan. We could also say that building PureScript in any other way is unsupported.

On a different note, I think it would be nice to use an alternative Prelude which re-exports <> (or perhaps the whole Data.Monoid module, or even Data.Semigroup) so that we don't need to import it separately.

I agree that we should have a stackage snapshot under which PS fully builds and only make "guarantees" about that one, but I also think that we should make an effort to try and build with upstream versions greater than the snapshot specifies.

This way we don't end up blocking any releases and provide valuable feedback to the upstream maintainers that we depend on so much.

What releases might we be blocking by not doing that?

I was thinking about stackage nightlies/lts versions in the sense that the latest nightly resolver might not include purescript at all because it has version conflicts

Ah, right, yes. Even though purescript can be used as a library, I think it's really an application (at least, that is how it is used 99.9% of the time). I am beginning to wonder if it makes sense for purescript to be in stackage at all.

We encourage people to install purescript via stack and the latest nightly resolver here:

https://github.com/purescript/purescript/blob/8dab1e848ec97da3d0c4068507dd5e408e95cc45/INSTALL.md#compiling-from-source

Being in stackage is valuable for PureScript since it's the most convenient way to actually notice breaking changes through upstream changes. Doesn't matter if it's a library or an executable, falling behind on dependency versions is never good.

Yeah, we should change INSTALL.md; what it says on the website is much more likely to work: http://www.purescript.org/download/

I agree that we should try to stay up to date with dependency versions, but I worry that being on Stackage could encourage us to place staying up to date too high on our list of priorities. If new versions of things are released and we are breaking Stackage nightly, it's usually not feasible to just update and put a release out, since releases usually need coordination among things like core libraries, pursuit, docs, etc. See the Stackage policy; I don't think these timeframes are really doable for us. Wouldn't staying on the latest LTS snapshot more or less achieve the same?

How did you come across this issue? Were you trying to compile PureScript yourself with cabal-install?

Exactly. We are (currently) building purescript with cabal as part of one of our builds. We're going to stop doing that for lots of reasons, I just raised this PR to avoid it breaking in the future when stackage was bumped. Funnily enough we're still building 0.8.5 and that broke for the exact same reason. At this point the horse has definitely bolted, people won't be able use cabal install any more without providing a growing, custom set of constraints.

I think it would be nice to use an alternative Prelude

Can't agree with this more. We have our own prelude, and it's enabled us to hide all the partial functions and support multiple versions of GHC with ease.

In any case, let me know if there's anything you can do on this PR. I'll stay out of the stack discussion. :)

Even though purescript can be used as a library, I think it's really an application (at least, that is how it is used 99.9% of the time).

Any experiments (eg. linting tools, things like the psc-ide) are likely going to be relying on the purescript package as a library; I'm sketching out a (single-purpose, experimental) linting tool that uses Language.PureScript, for example.

I think supporting the package-as-a-library it should stay as a "first-class" concern (completely aside from detecting compatibility breakages in Stackage or whatever) so as to keep it easy to experiment with.

I think we should update CONTRIBUTING.md because it still suggests using cabal-install. We should suggest using Stack instead, I think.

Though it's convenient to lock some versions and otherwise throw open the others with any wildcard constraints, it basically means anyone trying to build both current and previous versions needs to buy into Stack and Stackage, and are basically lost to anyone using straight Cabal without a lot of manual work.

If we want to bind ourselves to that, then great, but I really think this should be something to be made as a conscious choice, rather than something that happens to be easy, or because this one library breakage happens to tips us over into works-in-Stack / doesn't-work-in-Cabal territory.

I'd rather import Data.Semigroup (<>)

@hdgarrood

I agree that we should try to stay up to date with dependency versions, but I worry that being on Stackage could encourage us to place staying up to date too high on our list of priorities.

We /really/ should specify upper-bounds to all dependencies, and occasionally run packdeps. Also Stackage do ping us (at least me), and I have no problems with making forward-porting patches. They are usually trivial (language-javascript was the only intrusive, IIRC, and community stepped in to help with it). But optparse-applicative hadn't upper-bound, so nobody got pinged that there is outdated dependency, instead cabal build just broke.

There are benefits on staying on Stackage. It does help people to install purescript. They could stack --resolver=nightly-2016-08-18 purescript (there is purescript-0.9.1 only thought, have to check what blocks it, maybe some lower bound dependency is blocked by something else). Also Stackage have caught few problems with test-suite (node vs. nodejs comes to mind).

optparse-applicative is at least second dependency which bit-rotted purescript (aeson is the first).

I'd rather import Data.Semigroup (<>)

That's what the breakage in optparse-applicative is about - they added Semigroup instances in 0.13.0.0.

Yes, I'd rather import Data.Semigroup ((<>)), and have optparse-applicative: == 0.13.* dependency.

Indeed.

It did not seem right to continue to export a synonym for mempty once we had Semigroup instances.

At this point the horse has definitely bolted, people won't be able use cabal install any more without providing a growing, custom set of constraints.

Hm, I'm not sure I agree with this. Is the set of constraints actually growing, other than in the sense that we very occasionally add dependencies? I'm not sure I'd describe our set of constraints as "custom", either, since it's just Stackage LTS snapshots, which do have a very large buy-in across the ecosystem; it's definitely not just us who are using them.

@damncabbage

I think supporting the package-as-a-library it should stay as a "first-class" concern

Sure, if there's anything making it difficult to do that kind of experiment I'd definitely want to know so that it can be fixed or mitigated.

You have probably worked this out already but I'd recommend using the same snapshot as whichever version of purescript it is that you're depending on does. Pursuit is probably the best (only?) example of this now that psc-ide is in the compiler.

[...] basically lost to anyone using straight Cabal without a lot of manual work.

Perhaps not everyone will agree with me here but I don't think getting a copy of the cabal.config file for whichever Stackage snapshot we were using is particularly onerous. Additionally, even if that exact snapshot doesn't work for you for whatever reason, it's fine to make adjustments to your extra-deps in stack.yaml or to the constraints in a cabal.config file. In fact, I think the fact that we use Stack is a huge benefit even for people who are building PureScript with cabal-install, since the snapshot can be really helpful as a starting point for finding a build plan. I would add, though, that there are probably quite a few people who don't know how to do this so it would be good to explain how in INSTALL.md, I think.

@phadej

We /really/ should specify upper-bounds to all dependencies

👍 I agree that the dependency constraints in our cabal file should reflect our knowledge as much as possible, and that it's better for cabal-install users to get solver errors rather than build failures. I don't think we should be lazy about the constraints in our cabal file just because Stack allows us to.

It does help people to install purescript. They could stack --resolver=nightly-2016-08-18 purescript

This is dangerous and I think we should avoid recommending it. This command will get you, in most cases, an install plan which nobody has ever run the tests against and which few other people are using. I'm planning to update INSTALL.md to replace stack install --resolver=nightly purescript with what it says on the website (stack unpack purescript && cd purescript-* && stack install), which is much much safer. It also has the advantage that you'll always get the latest version; sometimes, the Stackage nightly is a little behind because of dependency conflicts. As you noted: right now, for example ;)

The above is the best argument for taking PureScript out of Stackage, I think. People sensibly assume that stack install [--resolver=nightly] purescript is the right thing to do, and end up with either an ancient version (if they didn't use the --resolver option), or if they did, they'll get an install plan that hasn't been tested, and the version of PureScript they get might still be out of date, too.

Also Stackage have caught few problems with test-suite (node vs. nodejs comes to mind).

purescript is in the expected-test-failures list in Stackage right now, and has been for quite a long time, so unless we address that it doesn't look like it's going to catch many others: https://github.com/fpco/stackage/blob/83bbf7648aa907df96c467e285b24556ce482f07/build-constraints.yaml#L3032

Maybe @paf31 can confirm or refute this, but I think a large part of the reason we wanted to standardise on one LTS snapshot (#1974) is to avoid this kind of work because of its (IMO) relatively low value-to-effort ratio.

Finally, if we do want to immediately hear whenever purescript doesn't build with the latest versions of everything (even though it's still not clear to me what we really gain from this) I think we should do that with CI. We already have a GHC 8.0/nightly build, we just need to work out how to build with the latest nightly rather than what we currently have, which is the nightly from the last time the CI scripts were updated. I guess just using sed on the stack-ghc-8.0.yaml file would work.

Sorry for the essay. I'm nearly done. I just wanted to suggest an alternative approach based on the above: immediately after each release, we update to the latest LTS snapshot. I think this is the right balance for us. It would give us plenty of time to test thoroughly with new versions of dependencies whenever we update. It would also mean we could just ignore nightly snapshots. If cabal-install users get build failures, we simply adjust the bounds in purescript.cabal so that they become solver failures.

since the snapshot can be really helpful as a starting point for finding a build plan...

build-depends: should be the primary source of truth for finding a build plan, as long as purescript is a library on Hackage.

build-depends: also communicate the constraints which maintainers think are sufficient. I.e. if there is a build plan which satisfies build-depends: constraints, purescript should be buildable, and also correct. Of course we cannot test all possible combinations of dependencies. We have to trust that dependency maintainers adhere to Hackage semantic contracts (especially don't introduce breaking semantic changes in minor version ranges).

IMHO if people do stack install purescript-0.9.3 or cabal install purescript-0.9.3 (with approriate flags to make install plan verifier happy), they can rightfully expect to get working version of purescript. This means that we should specify correctly both upper-bounds, and also lower-bounds (especially if there is some bugfixes in dependencies which are essential to correctness of purescript).

purescript is in the expected-test-failures

Yes, but as soon as purescript-0.9.3 lands into nightly I'll re-enable the test again (they are broken in 0.9.1). The issue of purescript test failures is resolved already.

I don't think we should be lazy about the constraints in our cabal file just because Stack allows us to.

We are. The Travis CI change to use stack and not use solver at all (thru cabal-install, or stack, or tinc or ...) is the witness of that laziness. servant package-suite experiments with travis cronjobs to verify that solver build plans are correct, that is reasonable compromise: our workflow doesn't suffer from build-failure elsewhere, but we get to know if something is broken periodically.

Another extreme is not to rely on "use stack.yaml" or cabal.config for particular snapshot, why not directly have strict versions in purescript.cabal directly, i.e.

-- We support GHC-7.10.3 and GHC-8.0.1
build-depends:
   base ==4.8.2.0 || ==4.9.0.0,
   aeson ==1.0.0.0,
   base-compat ==0.9.1,
   ...
   bytestring ==0.10.8.1, -- the one came with GHC-8.0.1, 

Note: i'm not really proposing this, as it's a very dumb idea, but that's what use stack essentially is.

Having single-major-version bounds (aeson == 1.0.*) on the other hand is a nice and reasonable compromise between testing yourself, and trusting other developers.

We have to trust that dependency maintainers adhere to Hackage semantic contracts

I guess this is our fundamental disagreement: sometimes library maintainers mess up, and sometimes it's almost impossible to clearly delineate what counts as a bugfix or a semantic change. Ideally yes, we would trust the PVP contracts, but in practice I think purescript users are best served by us taking a more conservative approach here.

Having said that, I totally agree about having our bounds reflect as much as possible of what we know about which versions of dependencies are ok and which are not.

IMHO if people do stack install purescript-0.9.3 or cabal install purescript-0.9.3 (with approriate flags to make install plan verifier happy), they can rightfully expect to get working version of purescript.

The problem with this approach, though, is that we then have to choose between overly conservative upper bounds which forbid even (supposedly) non-breaking changes, or trusting the PVP. Choosing the former would mean we'd be perpetually behind in Stackage, and it probably would make experimental programs using purescript as a library difficult too (for this reason, I also don't think we should go for the 'specify exact versions in purescript.cabal' approach). Choosing the latter means we have no way of protecting against bugfixes or minor changes in dependencies that end up causing bugs in purescript. This dilemma is completely avoidable with stack unpack, so we should just avoid it.

We are. The Travis CI change to use stack and not use solver at all (thru cabal-install, or stack, or tinc or ...) is the witness of that laziness.

We have to make a trade-off between the amount of effort we put in and the value we get out. I'm sorry but I'm not prepared to go back to dealing with cabal-install in the CI scripts. I'm not sure why, but cabal-install is really poorly suited for CI scripts. Having it in the CI scripts makes the value-to-effort ratio just too low. I remember dreading doing anything that might result in me having to do anything with the CI scripts; maintaining them was so horrible that they ended up not being updated. If we can do it easily with Stack then I would be okay with it.

To me, given how much time maintainers are able to devote to PureScript, Travis cron jobs sound like too much effort to be worth doing too, unfortunately. I think we'd be much better served by putting that effort into PureScript in other ways, at least for the time being. Maybe one day there will be a PureScript foundation and people working on the language full-time, and then it would be a better option. :)

If you don't want to specify upper-bounds, then please at least fix/add them after the fact. Currently bounds for purescript 0.6.3 — 0.9.3 are incorrect: http://matrix.hackage.haskell.org/package/purescript
By clicking the version number you can edit some of the metadata (especially build-depends:), if you are the maintainer of the package.

FWIW: also aeson-better-errors has incorrect lower bound on base: http://matrix.hackage.haskell.org/package/aeson-better-errors, thus making bower-json fail too: http://matrix.hackage.haskell.org/package/bower-json

What's the benefit of PureScript being in Stackage? It's so rarely used as a library, and people can add it to extra-deps if need be. I like using Stack for builds, but I never understood the need for us to be in Stackage.

Is there something wrong with the Appveyor build?

Looks like it timed out. I've restarted it.

Is there anything code-related left to do here?

I think the choices are:

• we decide we do want to stay in Stackage and keep purescript building with the latest versions of everything. In that case I think this PR looks good.
• we decide not to have purescript build with the latest versions of everything, so we take purescript out of Stackage, close this PR without merging, and update the bounds in purescript.cabal.

I think the arguments on each side have been more or less completely covered above; at least, I don't have anything else to add. Sorry that it took such a massive amount of text to do it in, though. My preference would of course be the latter.

Regardless of whether we stay in stackage denying a PR that will become a necessity when stackage updates optparse-applicative is just shooting ourselves in the foot. The PR in its current state allows us to build with both versions without any warnings so what's the point in denying it?

Is there anything code-related left to do here?

Not from my perspective

@kritzcreek it's just that I'd rather not set a precedent of trying to support lots of versions of things, since generally it's much, much simpler to stick to one LTS snapshot. The original PR was only a few lines; as we can see here, trying to support both makes it significantly larger.

Making imports qualified/explicit is mandatory in PS because it leads to less fragile code. The fact that this PR allows our codebase to support both versions demonstrates that again. Lets use this PR to set a precedent, not of going out of our way to support multiple versions, but of being more explicit with our imports and reducing the fragility of our codebase.

That's a very good point, I agree.

Thanks @charleso! Let's move the Stackage discussion to an item in the Discussion milestone, yes?

Sounds good to me.