Closed alpmestan closed 6 years ago
@alpmestan Thanks a lot! This all sounds good to me, although it feels a bit strange that the dynamic
way is treated specially. For example, are we going to also need extra-profiling-library-flavours
later?
Hope all three patches (Cabal, GHC and Hadrian) will land soon!
No, dynamic
is special because it yields shared libs. Profiled libraries can be either static or dynamic, so we're never going to need some special support as long as extra[-dynamic]-library-flavours
let us get non-trivial flavours of static or shared libraries.
I'm not ecstatic about this solution but it was either that or keep just extra-library-flavours
and treat anything with dyn
it in in a special way. That was judged too hacky, which is why I went for the new field.
omg. what box have I opened with extra-library-flavours
.
@snowleopard I doubt we'll need specialization for profiling. From a systems perspective there are only static and dynamic libraries; where the extra-library-flavours
was mostly concerned with allowing multiple static libraries in a cabal package (without cabal knowing how to actually build those libraries).
Eventually, I suppose some of hadrian's logic for building object files for many complex flavours could be carried over in Cabal, and then we would have a nicer approach to ask for a bunch of ways to be built. What's a little troubling is that this is only ever going to be useful for libHSrts
, since any other Haskell library ever only comes in vanilla, profiled or dynamic(/shared) flavour, they just don't have any other flavour to offer.
Would it make sense to generalize this notion of flavour so that not only libHSrts could offer its current menu of flavours to its customers, but other libraries could perhaps make use of this too? I don't know. All I know right now is that without the Cabal patch above, we simply can't have both sentences be true:
@alpmestan @angerman Thank you! It makes sense to me now. Let's go ahead with the proposed solution. I guess other libraries might start building various flavours once this is supported and documented.
When doing a clean hadrian build (flavour=quick) from my branch (https://github.com/DavidEichmann/ghc/commit/b7ca1a1413dd1a6f369f6fbd652182fc94b73474), I see that the GenericPackageDescription for the rts has extra-lib-flavours, but no extra-dyn-lib-flavours. I added debug output to Hadrian.Haskell.Cabal.Parser.resolveContextData
:
when (package == rts) . liftIO . putStrLn $ " @@@ generic condLibrary: " ++ show (C.condLibrary gpd)
And got the output:
@DavidEichmann Perhaps, we haven't yet implemented the new parsing functionality for extra-dyn-lib-flavours
in Hadrian?
I think the relevant function is:
https://github.com/snowleopard/hadrian/blob/master/src/Hadrian/Haskell/Cabal/Parse.hs#L173
Yes we've been looking at this function, but I personally fail to understand why things go fine for extraLibFlavours but not extraDynLibFlavours. Why one is preserved but not the other. It was all working fine before I rebased and got the last few commits (among which the caching ones?), so this is quite confusing.
@DavidEichmann This output is expected. This is "by default", without taking into account all the flags etc. In particular, you can notice how extraLibFlavours
only has _thr
while we in fact end up building and even installing a lot more static flavours than that. _thr
is there because we always build at least vanilla + threaded static libs for the RTS, whatever the flavour. On the other hand, the dyn lib flavours is empty because by default we don't want to produce any shared lib, only when the dynamic
cabal flag is passed when configuring the rts lib. Makes sense? So we need to look further down the road. At some point, extraLibFlavours
will be augmented with loads of things, and, quite likely, extraDynLibFlavours
won't, I suppose?
@alpmestan Actually, it appears that we don't even record anything flavour-related in ContextData
:
https://github.com/snowleopard/hadrian/blob/master/src/Hadrian/Haskell/Cabal/Type.hs#L40
Should we? If rts.cabal
contains this information, then maybe we need to parse it and put into ContextData.extraLibFlavours
or something like that?
Yeah maybe. Yet, it still works for static library flavours (extraLibFlavours
field in Cabal's BuildInfo
type), even though we don't have that information in ContextData
. So there's something odd going on, we're completely missing a piece of the puzzle, and we definitely need to figure it out. It is not completely crazy to think that even if we were storing that data in ContextData
, we still would not get Cabal to install our extra dynamic flavours. I actually consider this to be the most likely outcome (for what it's worth... since I've got no clue at this point about what's going on).
It looks like the empty extraDynLibFlavours was caused by my branch (DavidEichmann/ghc@b7ca1a1 being based on too old of a version of cabal (2.4). Hence the default value of []
is used as per this line of code. Commenting that line out leads to a successful build with ghc build with -dynamic and ghc --info
gives ("GHC Dynamic","YES")
.
I've noticed a further issue with the dynamically linked ghc binary (e.g. in the perf flavour). When executing it from the a directory other than the ghc project root, it fails to find the needed .so files. one can see that the "Library runpath" is set relative to the current directory:
when it should be set relative to the ghc binray as is done with the make built ghc binary:
The command to build the ghc binary from the make build system contains explicit options setting these paths (e.g. -optl-Wl,-rpath -optl-Wl,'$ORIGIN/../haskeline-0.7.4.3'
):
"inplace/bin/ghc-stage1" -o ghc/stage2/build/tmp/ghc-stage2 -hisuf dyn_hi -osuf dyn_o -hcsuf dyn_hc -fPIC -dynamic -O -H64m -Wall -hide-all-packages -i -ighc/. -ighc/stage2/build -Ighc/stage2/build -ighc/stage2/build/ghc/autogen -Ighc/stage2/build/ghc/autogen -optP-DGHCI -optP-include -optPghc/stage2/build/ghc/autogen/cabal_macros.h -package-id array-0.5.2.0 -package-id base-4.12.0.0 -package-id bytestring-0.10.8.2 -package-id containers-0.6.0.1 -package-id deepseq-1.4.4.0 -package-id directory-1.3.3.0 -package-id filepath-1.4.2.1 -package-id ghc-8.7 -package-id ghc-boot-8.7 -package-id ghc-prim-0.5.3 -package-id ghci-8.7 -package-id haskeline-0.7.4.3 -package-id process-1.6.3.0 -package-id time-1.8.0.2 -package-id transformers-0.5.5.0 -package-id unix-2.7.2.2 -Wall -Wnoncanonical-monad-instances -Wnoncanonical-monadfail-instances -Wnoncanonical-monoid-instances -fno-warn-name-shadowing -threaded -XHaskell2010 -XNoImplicitPrelude -O2 -Wcpp-undef -no-hs-main -threaded -no-user-package-db -rtsopts -Wnoncanonical-monad-instances -odir ghc/stage2/build -hidir ghc/stage2/build -stubdir ghc/stage2/build -fPIC -dynamic -O -H64m -Wall -hide-all-packages -i -ighc/. -ighc/stage2/build -Ighc/stage2/build -ighc/stage2/build/ghc/autogen -Ighc/stage2/build/ghc/autogen -optP-DGHCI -optP-include -optPghc/stage2/build/ghc/autogen/cabal_macros.h -package-id array-0.5.2.0 -package-id base-4.12.0.0 -package-id bytestring-0.10.8.2 -package-id containers-0.6.0.1 -package-id deepseq-1.4.4.0 -package-id directory-1.3.3.0 -package-id filepath-1.4.2.1 -package-id ghc-8.7 -package-id ghc-boot-8.7 -package-id ghc-prim-0.5.3 -package-id ghci-8.7 -package-id haskeline-0.7.4.3 -package-id process-1.6.3.0 -package-id time-1.8.0.2 -package-id transformers-0.5.5.0 -package-id unix-2.7.2.2 -Wall -Wnoncanonical-monad-instances -Wnoncanonical-monadfail-instances -Wnoncanonical-monoid-instances -fno-warn-name-shadowing -threaded -XHaskell2010 -XNoImplicitPrelude -O2 -Wcpp-undef -no-hs-main -threaded -no-user-package-db -rtsopts -Wnoncanonical-monad-instances -fno-use-rpaths -optl-Wl,-rpath -optl-Wl,'$ORIGIN/../haskeline-0.7.4.3' -optl-Wl,-rpath -optl-Wl,'$ORIGIN/../stm-2.5.0.0' -optl-Wl,-rpath -optl-Wl,'$ORIGIN/../ghc-8.7' -optl-Wl,-rpath -optl-Wl,'$ORIGIN/../terminfo-0.4.1.2' -optl-Wl,-rpath -optl-Wl,'$ORIGIN/../process-1.6.3.0' -optl-Wl,-rpath -optl-Wl,'$ORIGIN/../hpc-0.6.0.3' -optl-Wl,-rpath -optl-Wl,'$ORIGIN/../ghci-8.7' -optl-Wl,-rpath -optl-Wl,'$ORIGIN/../transformers-0.5.5.0' -optl-Wl,-rpath -optl-Wl,'$ORIGIN/../template-haskell-2.14.0.0' -optl-Wl,-rpath -optl-Wl,'$ORIGIN/../pretty-1.1.3.6' -optl-Wl,-rpath -optl-Wl,'$ORIGIN/../ghc-heap-8.7' -optl-Wl,-rpath -optl-Wl,'$ORIGIN/../ghc-boot-8.7' -optl-Wl,-rpath -optl-Wl,'$ORIGIN/../ghc-boot-th-8.7' -optl-Wl,-rpath -optl-Wl,'$ORIGIN/../directory-1.3.3.0' -optl-Wl,-rpath -optl-Wl,'$ORIGIN/../unix-2.7.2.2' -optl-Wl,-rpath -optl-Wl,'$ORIGIN/../time-1.8.0.2' -optl-Wl,-rpath -optl-Wl,'$ORIGIN/../filepath-1.4.2.1' -optl-Wl,-rpath -optl-Wl,'$ORIGIN/../binary-0.8.6.0' -optl-Wl,-rpath -optl-Wl,'$ORIGIN/../containers-0.6.0.1' -optl-Wl,-rpath -optl-Wl,'$ORIGIN/../bytestring-0.10.8.2' -optl-Wl,-rpath -optl-Wl,'$ORIGIN/../deepseq-1.4.4.0' -optl-Wl,-rpath -optl-Wl,'$ORIGIN/../array-0.5.2.0' -optl-Wl,-rpath -optl-Wl,'$ORIGIN/../base-4.12.0.0' -optl-Wl,-rpath -optl-Wl,'$ORIGIN/../integer-gmp-1.0.2.0' -optl-Wl,-rpath -optl-Wl,'$ORIGIN/../ghc-prim-0.5.3' -optl-Wl,-rpath -optl-Wl,'$ORIGIN/../rts' -optl-Wl,-zorigin ghc/stage2/build/Main.dyn_o ghc/stage2/build/GHCi/Leak.dyn_o ghc/stage2/build/GHCi/UI.dyn_o ghc/stage2/build/GHCi/UI/Info.dyn_o ghc/stage2/build/GHCi/UI/Monad.dyn_o ghc/stage2/build/GHCi/UI/Tags.dyn_o ghc/stage2/build/hschooks.dyn_o
The make logic can be found here. In the case of hadrian, we currently do not specify such linker options, but should. As hadrian installs all .so files in the same directory, we just need to use e.g. this option when building the ghc binary:
-optl-Wl,-rpath -optl-Wl,'$ORIGIN/../lib/x86_64-linux-ghc-8.7.20181018'
https://ghc.haskell.org/trac/ghc/ticket/15837 is now where we discuss/report updates about our past and upcoming fixes for this problem.
It turns out hadrian can produce any flavour we want of the RTS library, but Cabal just won't see them as its code just expects one shared lib, for the "just dynamic" way. This in particular keeps us from building the stage 2 GHC with
-dynamic
, like the make build system does, at least whenever we build a flavour that contains at least one dynamic-enabled way. Why? Because the stage 2 GHC is also meant to be built with-threaded
, 99% of the time, which requires thethr_dyn
way of the RTS library, that Cabal doesn't install.I have a Cabal patch that adds
extra-dynamic-library-flavours
, which seemed like the least bad solution when I discussed this problem with @hvr. See https://github.com/haskell/cabal/pull/5606.This then of course also requires a change to
rts.cabal.in
:that I will submit to GHC, along with a Cabal submodule bump, once I get the feature merged in Cabal.
Finally, I have some hadrian patch here that makes use of that new
dynamic
flag. The result is that with all those elements, we not only produce all the dynamic ways we want, as before, but Cabal sees them and installs them all in the package database, unlike before.@DavidEichmann and I stumbled upon this problem while investigating a test failure, where David noticed the "GHC Dynamic" bit of the
ghc --info
output differed between hadrian built GHCs and make built GHCs. On top of all those patches he will submit a PR that builds GHC with-dynamic
whenever we're building a flavour that contains one dynamic-enabled way, or something along those lines.