Skip to content
Snippets Groups Projects

Hadrian: fix library install paths in bindist Makefile (#16498)

Merged Hadrian: fix library install paths in bindist Makefile (#16498)
hsyl20/ghc:hsyl20-bindist-makefile-16498 into master
Merged Sylvain Henry requested to merge hsyl20/ghc:hsyl20-bindist-makefile-16498 into master

GHC now works out-of-the-box (i.e. without any wrapper script) by assuming that @bin@ and @lib@ directories sit next to each other. In particular, its RUNPATH uses $ORIGIN-based relative path to find the libraries.

However, to be good citizens we want to support the case where @bin@ and @lib@ directories (respectively BINDIR and LIBDIR) don't sit next to each other or are renamed. To do that the install script simply creates GHC specific @bin@ and @lib@ siblings directories into:

LIBDIR/ghc-VERSION/{bin,lib}

Then it installs wrapper scripts into BINDIR that call the appropriate programs into LIBDIR/ghc-VERSION/bin/.

The issue fixed by this patch is that libraries were not installed into LIBDIR/ghc-VERSION/lib but directly into LIBDIR.

Edited by Sylvain Henry

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • Sylvain Henry added 1 commit

    added 1 commit

    Compare with previous version

  • Alp Mestanogullari
    Alp Mestanogullari @alp ·
    Owner

    Hello :)

    Can you just expand a little bit on what wasn't working before this patch, on which system, etc? Thanks!

  • Sylvain Henry
    Sylvain Henry @hsyl20 ·
    Author Developer

    Hello. It's basically what is mentioned in #16498 (closed):

    • programs are installed with "make install" in PREFIX/lib/ghc-VER/bin/
    • their relative RPATH indicate that libraries are installed in PREFIX/lib/ghc-VER/lib/x86_64-linux-ghc-VER
    • but before this patch they were installed into PREFIX/lib/x86_64-linux-ghc-VER instead, so I fixed this.

    It's failing on x86_64 arch when we "make install" a bindist. If we use the programs from the bindist directly without "make install"ing them, it works because the paths are correct in the bindist.

  • Ben Gamari added hadrian packaging labels

    added hadrian packaging labels

  • Andrey Mokhov
    Andrey Mokhov @snowleopard ·
    Developer

    I find it difficult to understand what's going on in these generated code fragments, so any documentation/note that we could add to the source code would be very helpful.

    @hsyl20 These explanations are great! Could you add them to the commit message, and perhaps incorporate to the code too?

  • Sylvain Henry added 1 commit

    added 1 commit

    Compare with previous version

  • Sylvain Henry changed title from Fix paths in bindist Makefile (#16498 (closed)) to Hadrian: fix library install paths in bindist Makefile (#16498 (closed))

    changed title from Fix paths in bindist Makefile (#16498 (closed)) to Hadrian: fix library install paths in bindist Makefile (#16498 (closed))

  • Sylvain Henry changed the description

    changed the description

  • Sylvain Henry
    Sylvain Henry @hsyl20 ·
    Author Developer

    @snowleopard I have updated the note in the code and the commit/MR description.

    As a side note, I wonder if we still need to use the (undocumented...) "-B" flag to set the "base directory" of ghc in the wrapper script.

  • Andrey Mokhov approved this merge request

    approved this merge request

  • Andrey Mokhov unapproved this merge request

    unapproved this merge request

  • Andrey Mokhov
    Andrey Mokhov @snowleopard ·
    Developer

    @hsyl20 Great, thank you!

    I'll let @alp double-check and approve.

  • Alp Mestanogullari
    Alp Mestanogullari @alp ·
    Owner

    @hsyl20 Out of curiosity, have you tested this on Windows?

  • Sylvain Henry
    Sylvain Henry @hsyl20 ·
    Author Developer

    @alp Sadly no. I have only tested that the wrapper scripts didn't fail on Linux/x86_64.

  • Alp Mestanogullari
    Alp Mestanogullari @alp ·
    Owner

    @hsyl20 Would it be hard for you to try it out on a Windows system? For what it's worth, we already generate binary distributions in CI, so what I'm asking for amounts to adding a command or two to our CI script to install the bindist and test it in the most trivial way. I'm sorry to bother you with this, but since I've been fine-tuning everything to work on both Linux and Windows, I'm trying to make sure we know when some breakage might happen. It wouldn't necessarily block the MR, but at least we'd know about it as soon as it happens.

    Alternatively, I have a branch that adds ./validate --hadrian (https://gitlab.haskell.org/alp/ghc/commits/wip/alp/hadrian-validate-mode) and which combines all those things (binary dist, install, test). Some quirks left on Windows but it does get past the bindist install step.

    Edited by Alp Mestanogullari
  • Alp Mestanogullari approved this merge request

    approved this merge request

  • Alp Mestanogullari
    Alp Mestanogullari @alp ·
    Owner

    If it's too annoying let's merge this and I will see how that changes affects Windows. Approved.

  • Sylvain Henry
    Sylvain Henry @hsyl20 ·
    Author Developer

    @alp It took me quite some time but finally I have been able to build GHC on Windows for the first time \o/

    I have updated the wiki so that other people and future me don't have to go through all this again.

    However, I can't even build a bindist for master branch. It fails with:

    C:\Users\Sylvain\GHC\ghc>hadrian\build.stack.bat -c --flavour=quick binary-dist
WARNING: Ignoring out of range dependency (allow-newer enabled): Cabal-3.0.0.0. happy requires: <2.6
WARNING: Ignoring out of range dependency (allow-newer enabled): shake-0.17.5. hadrian requires: >=0.17.6
| Configure package 'Cabal'
| Configure package 'haskeline'
| Configure package 'transformers'
| Configure package 'mtl'
| Configure package 'ghci'
| Configure package 'ghc'
| Copy package 'rts'
# cabal-copy (for _build/stage1/lib/package.conf.d/rts-1.0.conf)
copyFile: does not exist (Le fichier spécifié est introuvable.)
shakeArgsWith   0.000s    0%
Function shake  0.007s    0%
Database read   0.190s   15%  ====
With database   0.015s    1%
Running rules   1.022s   82%  =========================
Total           1.234s  100%
Error when running Shake build system:
  at src/Main.hs:59:30-42:
* Depends on: binary-dist
  at src/Rules/BinaryDist.hs:97:9-21:
* Depends on: _build/stage1/lib/package.conf.d/rts-1.0.conf
* Raised the exception:
ExitFailure 1

    while the file exists:

    C:\Users\Sylvain\GHC\ghc>dir _build\stage1\lib\package.conf.d\rts-1.0.conf
[...]
01/04/2019  13:13             3 796 rts-1.0.conf

    Should I file a separate ticket?

    (thanks for the approval)

  • Alp Mestanogullari
    Alp Mestanogullari @alp ·
    Owner

    That's odd. Especially given that we already build a bindist on our Hadrian+Windows CI job. @snowleopard Any idea?

  • Sylvain Henry
    Sylvain Henry @hsyl20 ·
    Author Developer

    I wonder if it might be a crlf issue. I'm retrying with a fresh repository.

  • Sylvain Henry
    Sylvain Henry @hsyl20 ·
    Author Developer

    It was indeed a Git CRLF issue. I have updated the wiki again: the info is in hadrian/doc/windows.md but it is harder to find.

    I have tested building, installing and executing the wrapper scripts with this MR applied and it works.

    However the installation is a bit strange because we have:

    • PREFIX/bin: the wrappers
    • PREFIX/lib/bin: ghc.exe ghc-pkg.exe haddock haddock.exe hp2ps.exe hpc.exe hsc2hs.exe runghc.exe
    • PREFIX/lib/lib: the libraries, package.conf.d, etc.
    • PREFIX/lib/lib/bin: touchy.exe unlit.exe
  • Andrey Mokhov approved this merge request

    approved this merge request

  • Andrey Mokhov
    Andrey Mokhov @snowleopard ·
    Developer

    @hsyl20 Thanks for testing on Windows and updating the wiki!

    By "strange" do you mean an unnecessary lib subdirectory? If yes, I agree it does look like a bug.

    Edited by Andrey Mokhov
  • Sylvain Henry
    Sylvain Henry @hsyl20 ·
    Author Developer

    @snowleopard You're welcome! Thank you for your blog post about using Stack to build GHC on Windows and for the link to hadrian/doc/windows.md. Before finding your post I have wasted so much time trying to make the approach without Stack working (without success, configure was crashing...) :'(

    By "strange" do you mean an unnecessary lib subdirectory? If yes, I agree it does look like a bug.

    It seems that it's because "ghclibdir" doesn't contain "ghc-VERSION". When we install the Linux bindist we have: PREFIX/lib/ghc-VERSION/lib/bin/{unlit,ghc-iserv-dyn}

    Edited by Sylvain Henry
  • Andrey Mokhov
    Andrey Mokhov @snowleopard ·
    Developer

    @hsyl20 Glad my blog post and the doc helped!

    It seems that it's because "ghclibdir" doesn't contain "ghc-VERSION".

    Ah, I see. @alp @bgamari I guess this is a bug -- do we need to include ghc-VERSION into Windows installation too?

  • Ben Gamari
    Ben Gamari @bgamari ·
    Maintainer

    @snowleopard I would use the existing Windows bindists as a model. In that case e.g. unlit is installed in $PREFIX/lib/bin/unlit.exe.

  • Alp Mestanogullari
    Alp Mestanogullari @alp ·
    Owner

    That's definitely a bug, yes.

  • Sylvain Henry
    Sylvain Henry @hsyl20 ·
    Author Developer

    Ping. Could we merge this?

  • Ben Gamari approved this merge request

    approved this merge request

  • Ben Gamari assigned to @marge-bot

    assigned to @marge-bot

  • Marge Bot
    Marge Bot @marge-bot ·
    Owner

    I will attempt to batch this MR (!743 (closed))...

  • Marge Bot added 59 commits

    added 59 commits

    Compare with previous version

  • Marge Bot merged

    merged

  • Sylvain Henry mentioned in merge request !956 (closed)

    mentioned in merge request !956 (closed)

Please register or sign in to reply