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

Fix the build and tests with exceptions 0.9.* #54

Closed
gelisam opened this issue Mar 1, 2018 · 16 comments
Closed

Fix the build and tests with exceptions 0.9.* #54

gelisam opened this issue Mar 1, 2018 · 16 comments
Assignees
Milestone

Comments

@gelisam
Copy link
Contributor

gelisam commented Mar 1, 2018

The builds for PRs #52 and #53 both fail with the same error:

During the build phase, there is a warning which wasn't there before, saying that the generalBracket method is missing from a MonadMask instance. Neither PR touches that part of the code, and yet previous builds don't include the warning.

This is only a warning, but alas, both warnings become errors later on, when the unit tests are executed. In fact, each test fails separately with the same error message, indicating that it must be hint which is somehow trying to load its own code and failing because of a warning.

I did write both PR #52 and PR #53, but I don't think it is my changes which have caused this. One reason is that the two PRs touch different parts of the codebase and yet fail in the same way. The other is that while PR #53 configures Travis so that it also attempts to build with ghc-8.4, it doesn't make any change which should impact the build for the other versions of ghc, and yet the build fails for those versions of ghc as well.

My conclusion is that something in the build environment must have changed between the last (successful) build and these ones.

@gelisam
Copy link
Contributor Author

gelisam commented Mar 1, 2018

I can reproduce this locally, although with difficulty. I cannot reproduce the problem when running the tests with ghcid --comman="stack ghci hint:test:unit-tests" --test=main, stack test, nor cabal new-test, but I can reproduce the problem with cabal test. I deleted my ~/.cabal and ~/.ghc folders and carefully run the same commands as Travis, namely cabal update, cabal install --only-dependencies --enable-tests, cabal configure --enable-tests, and finally cabal test, but I don't know if all those steps are necessary to reproduce the problem.

Since I managed to reproduce the problem locally, I don't think that Travis changed their build environment; rather, I think this is the usual issue with cabal builds, namely that someone publishing a new version of a package somewhere can change our build plan and cause the build to fail mysteriously even though no code has changed. I bet a lower-bound is missing somewhere, not necessarily in our list of dependencies but perhaps in theirs.

@mvdan
Copy link
Contributor

mvdan commented Mar 1, 2018

The joy of builds that aren't stable and reproducible.

I'll look into this tomorrow - thanks for bringing it up.

@mvdan
Copy link
Contributor

mvdan commented Mar 3, 2018

I deleted my ~/.cabal and ~/.ghc, ran cabal install and cabal test, and it succeeded. Though, my dependencies originate from the Arch Linux packages, not from cabal. This makes me think that some dependency's minor or patch version broke the build.

I ran the Travis steps, and it still ran fine. So it appears the exact commands are not to blame (and I wouldn't see why, as they've been the same for a long time).

@mvdan
Copy link
Contributor

mvdan commented Mar 3, 2018

My best guess is that this library's 0.9.0 update broke the build: https://hackage.haskell.org/package/exceptions-0.9.0

It defines generalBracket, CI is using 0.9.0, and I'm still on 0.8.3.

@mvdan
Copy link
Contributor

mvdan commented Mar 3, 2018

Bingo - removing the lib from my system and re-running install and test, and the tests are now failing.

@mvdan
Copy link
Contributor

mvdan commented Mar 3, 2018

Success: https://travis-ci.org/mvdan/hint/builds/348662873

I'm going to retitle this issue to remove this temporary dependency restriction for the upcoming release.

mvdan added a commit that referenced this issue Mar 3, 2018
The 0.9.0 update breaks backwards compatibility, which broke the tests.
For now, simply require the version that we support. Fixing CI and
unblocking more work is more important than updating a library.

Updates #54.
@mvdan mvdan changed the title The Travis CI build is failing Fix the build and tests with exceptions 0.9.* Mar 3, 2018
@mvdan mvdan removed their assignment Mar 3, 2018
@mvdan mvdan added this to the 0.8.0 milestone Mar 3, 2018
@mvdan
Copy link
Contributor

mvdan commented Mar 3, 2018

@gelisam
Copy link
Contributor Author

gelisam commented Mar 3, 2018

Please also add the upper-bound for published versions of hint, using hackage's revision mechanism. On a blank system (such as the docker images I have created for #56), cabal install hint successfully installs a broken hint which always fails with a generalBracket-related message.

@mvdan
Copy link
Contributor

mvdan commented Mar 3, 2018

You're right, I forgot about the stable version. Done: https://hackage.haskell.org/package/hint-0.7.0/revisions/

@gelisam
Copy link
Contributor Author

gelisam commented Mar 15, 2018

I have investigated exceptions-0.9.0 and have discovered an important regression. I sent them a fix and they published exceptions-0.10.0, which breaks the API in a slightly different way than exceptions-0.9.0 did.

I can write a MonadMask instance which will work with exceptions-0.10.0, but it won't work with older versions of the exceptions library. Are you okay with requiring exceptions >= 0.10.0?

@mvdan
Copy link
Contributor

mvdan commented Mar 15, 2018

It doesn't seem like we have much of an option in the long term. If anyone is stuck with exceptions 0.8.0, they are likely also stuck with an older version of GHC, in which case using a slightly older version of Hint should be fine.

@gelisam
Copy link
Contributor Author

gelisam commented Mar 15, 2018 via email

@mvdan
Copy link
Contributor

mvdan commented Mar 15, 2018

Is there any such library that won't compile with Hint 0.10.x? Besides hint, that is :)

The set of libraries using exceptions has already been split in two; it's just a matter of what side we want to be on. I'd say go for the newer for the upcoming Hint release, unless there is a good reason not to.

@gelisam gelisam self-assigned this May 2, 2018
@gelisam
Copy link
Contributor Author

gelisam commented May 3, 2018

I'd say go for the newer for the upcoming Hint release

Do you have a timeline on when you plan to perform this release? I still want to implement this, but I haven't had a lot of time to put on this and I don't want to hold you back.

@gelisam
Copy link
Contributor Author

gelisam commented May 3, 2018

Never mind, I found the time for it this morning :)

@mvdan
Copy link
Contributor

mvdan commented May 4, 2018

I wanted to do the release sometime in April or May, but I was briefly on holiday and now travelling :) I'll look at your patch now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants