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

Migrated to lts-15.3 #4827

Merged
merged 18 commits into from
Mar 20, 2020
Merged

Migrated to lts-15.3 #4827

merged 18 commits into from
Mar 20, 2020

Conversation

ilyakooo0
Copy link
Contributor

The main difference is that there are far less extra-deps in stack.yaml and code had to be slightly modified to work with megaparsec 8.

@melted
Copy link
Contributor

melted commented Mar 12, 2020

Thanks for the PR! Looks good, let's see what the CI has to say.

@ilyakooo0
Copy link
Contributor Author

It will require some CI script modifications. I will investigate.

@ilyakooo0
Copy link
Contributor Author

It looks like GHC 8.4 was broken for a while (#4611).

Due to megaparsec 8 requiring base 4.11 (and thus GHC 8.4 and higher), the only GHC versions left in CI are 8.6 and 8.8.

@ilyakooo0
Copy link
Contributor Author

I still get libffi errors and I have no idea what causes them or how to fix them:

../../dist/build/idris/idris: error while loading shared libraries: libffi.so.7: cannot open shared object file: No such file or directory

@jfdm
Copy link
Contributor

jfdm commented Mar 12, 2020

The issue with libffi is described here. I think the issue is with the binary stack produces being built sans an option to use a locally available libFFI and to use GHC's internal copy.

@ilyakooo0
Copy link
Contributor Author

CI actually doesn't use stack. It only builds with cabal.

Adding stack CI builds would probably be a good idea as well, but the would be out of the scope of this PR.

@ilyakooo0
Copy link
Contributor Author

This seems related: https://stackoverflow.com/questions/4514997/error-loading-shared-libraries

It's weird that it started happening after a ghc version change

@melted
Copy link
Contributor

melted commented Mar 12, 2020

It's because the PPAs for GHC on Launchpad are built with their own copy of libffi instead of the the one in the Ubuntu packages.

@ilyakooo0
Copy link
Contributor Author

ilyakooo0 commented Mar 13, 2020 via email

@ilyakooo0 ilyakooo0 changed the title Migrated to lts-15.3 [⚠️ WIP] Migrated to lts-15.3 Mar 13, 2020
@jfdm
Copy link
Contributor

jfdm commented Mar 13, 2020

The problem with GHC and libffi is well documented:

https://gitlab.haskell.org/ghc/ghc/issues/15397

When using stack locally, I get the same issue with LTS 15.3 in that stack's binary ghc's are failing at the linking stage. I recently build Idris using the hvr-ppa and GHC v8.8.2 using cabal new. It might be the case that hvr-ppa is using a work around with the flag --use-system-libffi for more recent GHCs or that the fix has not yet been back ported to older ones. I think that the GHCs one uses for other platforms use this flag in the first place, with the issue being Ubuntu and Fedora specific.

@ilyakooo0
Copy link
Contributor Author

ilyakooo0 commented Mar 13, 2020

Luna uses the LD_PRELOAD trick to seeming use the libffi bundled with GHC instead of the system one when running tests:

https://github.com/luna/luna/blob/61e444cd94ff740caf3449d44edea5a3c165880e/.ci/azure-pipelines.yml#L32

It might be worth a shot.

I'm not too familiar with Linux though and am not sure where to find the rts folder from the system GHC installation.

@ilyakooo0
Copy link
Contributor Author

BTW @jfdm if you have a look at the Travis CI build logs, it uses GHC from hvr-ppa, so it is probably not using a work-around:

sudo -E apt-add-repository -y "ppa:hvr/ghc"

@melted
Copy link
Contributor

melted commented Mar 13, 2020

I don't think it was fixed in bionic, which is the freshest ubuntu you can run on travis. I wonder how much work it would be to use docker on travis to run it on a newer version of ubuntu (or maybe just use an arch container).

@melted
Copy link
Contributor

melted commented Mar 13, 2020

I should add that this was the exact issue that was holding us back from using a modern GHC on travis, I snuck in GHC 8.8 for the Idris2 build as I could just build idris without FFI.

So another alternative is to just disable the tests that uses the interpreter and build it without the FFI flag.

@jfdm
Copy link
Contributor

jfdm commented Mar 13, 2020

@melted It would be interesting to see in 8.8.2 works with ffi on travis. This would pretty much be a replication with my current set up.

@ilyakooo0 I know the contents of the travis file, what I meant was that the hvr-ppa would have built their versions of ghc with the correct flag.

@ilyakooo0
Copy link
Contributor Author

ilyakooo0 commented Mar 13, 2020

I think I managed to fix the libffi issue:

https://travis-ci.org/github/idris-lang/Idris-dev/jobs/661951376

It failed for reasons that to me seem unrelated to ffi

More specifically:

make: Entering directory '/home/travis/build/idris-lang/idris-1.3.2/libs'
make -C prelude build
make[1]: Entering directory '/home/travis/build/idris-lang/idris-1.3.2/libs/prelude'
../../dist/build/idris/idris --build prelude.ipkg
Type checking ./Builtins.idr
Type checking ./Prelude/Uninhabited.idr
Type checking ./Prelude/Algebra.idr
Type checking ./Prelude/Bool.idr
Type checking ./Prelude/Basics.idr
Type checking ./Prelude/Interfaces.idr
idris: No trivial solution
Makefile:5: recipe for target 'build' failed
make[1]: *** [build] Error 1
make[1]: Leaving directory '/home/travis/build/idris-lang/idris-1.3.2/libs/prelude'
Makefile:2: recipe for target 'build' failed
make: *** [build] Error 2
make: Leaving directory '/home/travis/build/idris-lang/idris-1.3.2/libs'
The command "LD_PRELOAD=/opt/ghc/${GHCVER}/lib/ghc-${GHCVER}/rts/libffi.so.7 cabal v1-build" exited with 2.

@melted
Copy link
Contributor

melted commented Mar 13, 2020

That looks promising!

@ilyakooo0
Copy link
Contributor Author

GHC 8.6 doesn't pass the tests for some reason (I didn't look into it, but I could reproduce it locally), so I have also removed it from CI.

As it currently stands, the codebase was modified in a backward-incompatible way to support megaparsec 8, which does not compile with GHC 8.2 dues to the required version of base.

If we really wanted to, we could add pragmas to conditionally support either megaparsec 8 or megaparsec 7. This, however, would probably slowly creep all over the codebase and, as this is a compiler and not a library, I see no real benefit in striving to support multiple GHC versions. IMO simplifying the codebase is a bigger priority, than supporting multiple GHC versions.

Tests are currently run in CI only with GHC 8.8.

@ilyakooo0 ilyakooo0 changed the title [⚠️ WIP] Migrated to lts-15.3 Migrated to lts-15.3 Mar 13, 2020
@melted
Copy link
Contributor

melted commented Mar 13, 2020

It's not much of a problem for Mac or Windows users to use a new GHC, the benefit is running with a distro-supplied GHC for Linux users. It wouldn't be nice if we made it so @edwinb couldn't build Idris anymore.

I see debian stable is up to GHC 8.4 now, pretty close. A new Ubuntu LTS will be released next month, I guess that takes care of Ubuntu. And FreeBSD has 8.6 now.

At least I think we should see what's failing on 8.6 and run that on the CI, as that is a very common version and will be for several years.

@jfdm
Copy link
Contributor

jfdm commented Mar 13, 2020

A quick reminder that the documentation and new restrictions (if this is to be merged) will also need to be updated.

@ilyakooo0
Copy link
Contributor Author

@melted if you are curious about the GHC 8.6 issue, then check out this run: https://travis-ci.org/github/idris-lang/Idris-dev/builds/661976175

@ilyakooo0
Copy link
Contributor Author

A way we could proceed is for people developing on Linux to either:

  1. Install the same version of libffi as GHC is using (they are available in a submodule in the GHC repo) (I don't think this would be a horrible solution)
  2. Use the same workaround I used in CI
  3. Add path to the GHC version of the libffi to the LD_... variable
  4. Use a patched version of GHC

When distributing the prebuilt binaries, I think it would be feasible to use a patched version of GHC, so this issue shouldn't affect the end Idris user.

@melted
Copy link
Contributor

melted commented Mar 13, 2020

Looking at that, I don't have high hopes of an easy fix for 8.6.

I think the way forward is to put in a little conditional compilation so it'll work by using megaparsec 7 for 8.4 and 8.6. If you don't feel like doing that, I can implement it.

We definitely want it to work with megaparsec 8 to make it easy for packaging for stack and nix. But we also want it to work with LTS Linux distributions, so we'll just have to put in some ugliness for that.

@ilyakooo0
Copy link
Contributor Author

I get errors like this when I try building with GHC version less than 8:

ar rc libidris_rts.a idris_rts.o idris_heap.o idris_gc.o idris_gmp.o idris_bitstring.o idris_opts.o idris_stats.o idris_utf8.o idris_stdfgn.o idris_buffer.o getline.o idris_net.o mini-gmp.o
ranlib libidris_rts.a

idris                  > copy/register
/Users/iko/Developer/Idris-dev/.stack-work/install/x86_64-osx/461bef767b3dd2476ac556ae62cb86e4c5ae44964bcc2f254a123e39efc02785/8.6.4/share/x86_64-osx-ghc-8.6.4/idris-1.3.2/jsrts/jsbn: copyFile: does not exist (No such file or directory)
Completed 28 action(s).
'cabal copy' failed.  Error message:

--  While building package idris-1.3.2 using:
      /Users/iko/Developer/Idris-dev/.stack-work/dist/x86_64-osx/Cabal-2.4.0.1/setup/setup --builddir=.stack-work/dist/x86_64-osx/Cabal-2.4.0.1 copy
    Process exited with code: ExitFailure 1

Possible causes of this issue:
* No module named "Main". The 'main-is' source file should usually have a header indicating that it's a 'Main' module.
* A cabal file that refers to nonexistent other files (e.g. a license-file that doesn't exist). Running 'cabal check' may point out these issues.
* The Setup.hs file is changing the installation target dir.

Does this look like a Setup.hs bug?

@ilyakooo0
Copy link
Contributor Author

I was able to reproduce it in the master branch.

@ilyakooo0
Copy link
Contributor Author

seems to be related to haskell/cabal#6125

Looking at the produced directory contents, ** is clearly not interpreted as expected:

Screenshot 2020-03-18 at 18 07 58

@melted
Copy link
Contributor

melted commented Mar 18, 2020

Looks like it's something that works on cabal-3.0 and not on 2.4 in the cabal file. Guess we could use 3.0.

@ilyakooo0
Copy link
Contributor Author

Sure, that would be a solution, but stack doesn't currently support cabal 3 and gives an error if I try to explicitly require cabal-version 3.

A thing we could do is manually expand all of the **.

@melted
Copy link
Contributor

melted commented Mar 18, 2020

According to this ** was in 2.4 https://www.haskell.org/cabal/users-guide/file-format-changelog.html So maybe we don't have to go that far.

We used to generate that list in setup.hs before, but that was taken away in 3.0. We had a fully expanded list before then and that was very brittle.

@ilyakooo0
Copy link
Contributor Author

It was added in 2.4, but it is buggy in 2.4.

I didn't dig into when exactly it breaks, but I was able to work around it by manually expanding a small amount of wildcards, which I think will be acceptable.

@melted
Copy link
Contributor

melted commented Mar 18, 2020

Ok, that's fine.

@melted
Copy link
Contributor

melted commented Mar 18, 2020

I found out what the error with 8.6.5 was, I will fix that in another PR.

@ilyakooo0
Copy link
Contributor Author

It might be my local machine weirdness (I don't think it is though), but when running stack test with GHC less than 8.8 (I get it with GHC 8.4 and 8.2) I get this test fail:

  Packages
    pkg001:            OK (22.49s)
    pkg002:            OK (8.01s)
    pkg003:            OK (14.23s)
    pkg004:            OK (0.11s)
    pkg005:            OK (0.06s)
    pkg006:            OK (12.75s)
    pkg007:            OK (23.82s)
    pkg008:            OK (23.66s)
    pkg010:            FAIL (0.05s)
      --- test/pkg010/output	2020-03-19 00:27:41.000000000 +0300
      +++ test/pkg010/expected.out	2020-03-12 21:07:44.000000000 +0300
      @@ -4,28 +4,26 @@
         | ^
       Invalid option `-total'

      -Usage:  ([--nobanner] | [-q|--quiet] | [--ide-mode] | [--ide-mode-socket] |
      -        [--client ARG] | [--log LEVEL] | [--logging-categories CATS] |
      -        [--nobasepkgs] | [--noprelude] | [--nobuiltins] | [--check] |
      -        [-o|--output FILE] | [--interface] | [--typeintype] | [--total] |
      -        [--partial] | [--warnpartial] | [--warnreach] | [--warnipkg] |
      -        [--nocoverage] | [--errorcontext] | [--info] | [--listlogcats] |
      -        [--link] | [--listlibs] | [--libdir] | [--docdir] | [--include] | [--V2]
      -        | [--V1] | [-V|--V0|--verbose] | [--ibcsubdir FILE] |
      -        [-i|--idrispath ARG] | [--sourcepath ARG] | [--warn] |
      -        [-p|--package ARG] | [--port PORT] | [--build IPKG] | [--install IPKG] |
      -        [--repl IPKG] | [--clean IPKG] | [--mkdoc IPKG] | [--installdoc IPKG] |
      -        [--checkpkg IPKG] | [--testpkg IPKG] | [--indent-with INDENT] |
      -        [--indent-clause INDENT] | [--bytecode ARG] | [-S|--codegenonly] |
      -        [-c|--compileonly] | [--dumpdefuns ARG] | [--dumpcases ARG] |
      -        [--codegen TARGET] | [--portable-codegen TARGET] | [--cg-opt ARG] |
      -        [-e|--eval EXPR] | [--execute] | [--exec EXPR] | [-X|--extension EXT] |
      -        [--O3] | [--O2] | [--O1] | [--O0] | [--partial-eval] |
      -        [--no-partial-eval] |
      -        [--optimise-nat-like-types|--optimize-nat-like-types] |
      -        [--no-optimise-nat-like-types|--no-optimize-nat-like-types] |
      -        [-O|--level ARG] | [--target TRIPLE] | [--cpu CPU] | [--color|--colour]
      -        | [--nocolor|--nocolour] | [--consolewidth WIDTH] | [--highlight] |
      -        [--no-tactic-deprecation-warnings] |
      -        [--allow-capitalized-pattern-variables]) [FILES] [-v|--version]
      +Usage:  [--nobanner | (-q|--quiet) | --ide-mode | --ide-mode-socket |
      +          --client ARG | --log LEVEL | --logging-categories CATS |
      +          --nobasepkgs | --noprelude | --nobuiltins | --check |
      +          (-o|--output FILE) | --interface | --typeintype | --total |
      +          --partial | --warnpartial | --warnreach | --warnipkg | --nocoverage |
      +          --errorcontext | --info | --listlogcats | --link | --listlibs |
      +          --libdir | --docdir | --include | --V2 | --V1 | (-V|--V0|--verbose) |
      +          --ibcsubdir FILE | (-i|--idrispath ARG) | --sourcepath ARG | --warn |
      +          (-p|--package ARG) | --port PORT | --build IPKG | --install IPKG |
      +          --repl IPKG | --clean IPKG | --mkdoc IPKG | --installdoc IPKG |
      +          --checkpkg IPKG | --testpkg IPKG | --indent-with INDENT |
      +          --indent-clause INDENT | --bytecode ARG | (-S|--codegenonly) |
      +          (-c|--compileonly) | --dumpdefuns ARG | --dumpcases ARG |
      +          --codegen TARGET | --portable-codegen TARGET | --cg-opt ARG |
      +          (-e|--eval EXPR) | --execute | --exec EXPR | (-X|--extension EXT) |
      +          --O3 | --O2 | --O1 | --O0 | --partial-eval | --no-partial-eval |
      +          (--optimise-nat-like-types|--optimize-nat-like-types) |
      +          (--no-optimise-nat-like-types|--no-optimize-nat-like-types) |
      +          (-O|--level ARG) | --target TRIPLE | --cpu CPU | (--color|--colour) |
      +          (--nocolor|--nocolour) | --consolewidth WIDTH | --highlight |
      +          --no-tactic-deprecation-warnings |
      +          --allow-capitalized-pattern-variables] [FILES] [-v|--version]
       )

@ilyakooo0
Copy link
Contributor Author

If we get the GHC 8.6 issue fixed, then might we drop support for 8.4 and 8.2? (For simplicity’s sake and avoiding excessive conditional compilation)

@melted
Copy link
Contributor

melted commented Mar 20, 2020

8.2 can be dropped, but 8.4 is still common. Looks like it's fine anyway, let's merge this.

@melted melted merged commit c6c928f into idris-lang:master Mar 20, 2020
@jfdm
Copy link
Contributor

jfdm commented Apr 2, 2020

So trying to build idris with cabal v2-install It appears that idris is failing because not all of the source files for libs were copied across. This might be related to the issues with **. I will have a look if expanding the complete list for libs and test fixes this issue.

@jfdm
Copy link
Contributor

jfdm commented Apr 2, 2020

It looks like my hunch is correct, I will adapt the cabal file, and update HEAD.

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

Successfully merging this pull request may close these issues.

3 participants