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

GHC 8.2.1 Compatibility #4529

Closed
wants to merge 8 commits into from
Closed

GHC 8.2.1 Compatibility #4529

wants to merge 8 commits into from

Conversation

hvr
Copy link
Member

@hvr hvr commented May 19, 2017 •
edited
Loading

This PR makes cabal compatible with GHC 8.2.1's libraries.

TODO: add GHC 8.2.1 to Travis-config

@mention-bot
Copy link

@hvr, thanks for your PR! By analyzing the history of the files in this pull request, we identified @ezyang, @23Skidoo and @dcoutts to be potential reviewers.

@hvr
Copy link
Member Author

hvr commented May 20, 2017 •
edited
Loading

@ezyang @23Skidoo unfortunately, the cabal-testsuite exposed a couple of problems w/ GHC 8.2.1RC2:

UNEXPECTED FAIL: 
PackageTests/Backpack/Includes2/setup-internal.test.hs 
PackageTests/Backpack/Includes2/setup-per-component.test.hs 
PackageTests/InternalLibraries/setup-gen-script.test.hs 
PackageTests/InternalLibraries/setup-per-component.test.hs 
PackageTests/InternalLibraries/setup-gen-pkg-config.test.hs 
PackageTests/InternalLibraries/Executable/setup-static.test.hs 
PackageTests/InternalLibraries/Library/setup.test.hs

(from https://travis-ci.org/haskell/cabal/jobs/234280545)

PS: Is this fixed by https://ghc.haskell.org/trac/ghc/ticket/13703 getting merged into 8.2?

@ezyang
Copy link
Contributor

ezyang commented May 20, 2017

Yes, this is that upstream bug.

@hvr
Copy link
Member Author

hvr commented May 21, 2017 •
edited
Loading

@ezyang fair enough, in the mean-time a new GHC 8.2.1 snapshot has been built for the PPA which includes ghc/ghc@8054a74 but Travis now runs into a time out (the job gets killed after 50 minutes); right before cabal-testsuite is invoked, the log says whole job took 2549 seconds so far.. Maybe it's time to try out Travis' new staged jobs feature?

see e.g. https://travis-ci.org/haskell/cabal/jobs/234280545

PS: At least one job fails unexpectedly due to

rejecting: custom:setup.Cabal-2.0.0.0/installed-2.0... (conflict: custom => custom:setup.Cabal>=1.10 && <1.25)

as this cannot be satisfied w/ GHC 8.2.1 (there is no version of Cabal-1.24 that's declared compatible w/ GHC 8.2.1 -- in the past, new major versions of GHC also required new major versions of Cabal, so that's been the common pattern in the past if you look at the matrix-builder)

@ezyang
Copy link
Contributor

ezyang commented May 21, 2017

The timeout is probably because we're building deps for the first time; they'll subsequently be cached. So if you keep restarting the build eventually it will succeed.

Test failure needs more investigatoin.

@hvr
Copy link
Member Author

hvr commented May 21, 2017 •
edited
Loading

@ezyang the problem is that the dependency are not cached if the job is killed by timeout. So I now temporarily workarounded this by exit 1 shortly after everything has been built. This is exactly what Travis' new build stages would allow to do w/o such hacks!

Here's an updated list of failed tests w/ the latest GHC 8.2.1RC2 snapshot (see https://travis-ci.org/haskell/cabal/jobs/234575195):

UNEXPECTED FAIL: 
PackageTests/CustomPlain/cabal.test.hs 
PackageTests/CustomWithoutCabalDefaultMain/cabal.test.hs 
PackageTests/BuildTargets/UseLocalPackageForSetup/use-local-package-as-setup-dep.test.hs 
PackageTests/NewBuild/CustomSetup/RemotePackageWithCustomSetup/build-package-from-repo-with-custom-setup.test.hs 
PackageTests/NewBuild/CustomSetup/LocalPackageWithCustomSetup/build-local-package-with-custom-setup.test.hs 
PackageTests/NewBuild/T4450/cabal.test.hs PackageTests/Backpack/T4447/setup.test.hs 
PackageTests/CustomDep/cabal.test.hs

@hvr hvr mentioned this pull request May 23, 2017
Merged
@23Skidoo 23Skidoo mentioned this pull request May 23, 2017
Closed
@ezyang
Copy link
Contributor

ezyang commented Jun 4, 2017

Yes, those bugs are known: https://ghc.haskell.org/trac/ghc/ticket/13703

Should be fixed on next RC.

@hvr
Copy link
Member Author

hvr commented Jun 4, 2017

@ezyang Why do you think the next RC would fix this? The current PPA ghc-8.2.1 snapshot is at ghc/ghc@c0b82c3 (the build job used 8.2.0.20170522+git.0.c0b82c3-2~14.04) which contains the supposed fix ghc/ghc@8054a74

@23Skidoo
Copy link
Member

23Skidoo commented Jun 5, 2017

Should we file a new GHC bug report/reopen 13703?

@ezyang
Copy link
Contributor

ezyang commented Jun 6, 2017

No. Instead, if we apply #4554 to this PR, I think things should be green.

hvr and others added 5 commits June 6, 2017 08:34
@hvr
Add simple GHC 8.2.1 entry to .travis.yml matrix
a2d613e
@hvr
Try to workaround job timeout with -j2
3d9c58d 
GHC 8.2.1 appears to take significantly longer to compile the code
and pushes us over the Travis job time limit.
@hvr
local hack to get Travis CI to commit deps to cache
b3d0f39
@hvr
Revert "local hack to get Travis CI to commit deps to cache"
2e5dc51 
This reverts commit 816ca32.
@ezyang @hvr
Fix test suite wobbliness on GHC 8.2.
ce9dd5e 
Here were the root causes:

- Some tests involving Custom setpu showed MORE output
  (UseLocalPackageForSetup) when run on GHC 8.2.  This is because GHC
  8.2 ships a recent enough version of Cabal to know how to emit
  markers, which means we have started picking up the output.  I hacked
  up these tests to not accept this output, but a more correct thing to
  do is figure out how to NOT request marking of a Setup script which is
  not the inplace install.  This was a little tricky so I bailed.

- GHC 8.2 no longer emits "It is a member of the hidden package".
  This broke CustomWithoutCabalDefaultMain.  Not sure if this
  is a GHC regression but it's pretty harmless.

- While I was at it, I fixed an inexhaustive pattern match in
  cabal-testsuite (though perhaps poorly; I couldn't figure out
  what the new constructor does.)

Signed-off-by: Edward Z. Yang <ezyang@cs.stanford.edu>
@hvr hvr force-pushed the pr/ghc821-compat branch from 34f99ab to ce9dd5e Compare June 6, 2017 06:35
@hvr
Copy link
Member Author

hvr commented Jun 6, 2017

I've rebased this PR and added #4554 on top... now I'm curious :-)

@23Skidoo
Copy link
Member

23Skidoo commented Jun 6, 2017

The failure is due to timeout.

@23Skidoo
Copy link
Member

23Skidoo commented Jun 6, 2017 •
edited
Loading

@ezyang There's still one unexpected failure with GHC 8.2:

UNEXPECTED FAIL: PackageTests/NewBuild/CustomSetup/RemotePackageWithCustomSetup/build-package-from-repo-with-custom-setup.test.hs

@hvr hvr mentioned this pull request Jun 7, 2017
Closed
@23Skidoo 23Skidoo mentioned this pull request Jun 7, 2017
Merged
2 tasks
@ezyang
Copy link
Contributor

ezyang commented Jun 9, 2017

@23Skidoo I pushed a fix to the other PR.

@23Skidoo
Copy link
Member

23Skidoo commented Jun 9, 2017

Yay, it passes now!

@23Skidoo
Copy link
Member

23Skidoo commented Jun 9, 2017

Cherry-picked relevant commits manually.

@23Skidoo 23Skidoo closed this Jun 9, 2017
@hvr hvr deleted the pr/ghc821-compat branch June 9, 2017 16:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@hvr @mention-bot @ezyang @23Skidoo