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

Check that exes that are using TH and profiling are building correctly with a default-dyn GHC #1252

Closed
23Skidoo opened this Issue Mar 21, 2013 · 1 comment

Comments

Projects
None yet
1 participant
@23Skidoo
Copy link
Member

23Skidoo commented Mar 21, 2013

Quoting @dcoutts on IRC (we're discussing my fix for #1207):

13:50 < dcoutts> refold: I think it may need a further fix actually
13:50 < dcoutts> the comment there about dynamic vs static games
13:51 < dcoutts> will it still work for the new ghc that's using dynamic libs by default?
13:52  * dcoutts does not like the current logic for handling that
13:52 < dcoutts> it's rather hacky
13:54 < refold> dcoutts: I don't understand the dynamic vs static games issue completely 
13:55 < dcoutts> refold: ok, so for template haskell, ghc is loading the code, so it has to be the right kind of library and be ABI compatible
13:55 < dcoutts> for example ghc cannot load code that is built for profiling
13:55 < dcoutts> because the abi is different
13:56 < dcoutts> (it might work if ghc itself was built for profiling, but it's not)
13:56 < dcoutts> so you'll see that we arrange so that even if the user asks to *only* build for profling and not vanilla, if the package uses TH then we will always do a vanilla build first
13:57 < dcoutts> so that ghc can load those .o files
13:57 < refold> yes, that's what was causing the test failure
13:57 < dcoutts> now, I think ghc-7.8 is using dynamic libs by default, ie libHSbase-x.y.so rather than libHSbase-x.y.a
13:58 < dcoutts> so we have yet another "dynamic by default" hack
13:58 < dcoutts> and if we're in that mode then we always do a build the dynamic way, even if you asked for static
13:59 < dcoutts> so it's basically the same issue as the profiling, but it depends on the ghc version
13:59 < dcoutts> and it's worse currently because the logic we use to solve it is more convoluted than necessary imho
13:59 < dcoutts> we do things like swapping the order of builds
14:00 < dcoutts> when I think we should do it by defining something like "vanillaOpts" conditionally
14:00 < dcoutts> actually, the term vanilla is now unhelpful
14:01 < dcoutts> we should simply call them static, dynamic and profiling (or maybe that has to be static-profiling and dynamic-profiling)
14:01 < refold> so that's what the ghcDynamic check in buildLib is for
14:01 < dcoutts> right
14:02 < refold> ok
14:02 < dcoutts> we ought to be able to simplify and clarify all this logic
14:02 < refold> but we don't check for ghcDynamic in buildExe
14:03  * dcoutts wonders why that is
14:03  * dcoutts didn't add that code
14:03 < dcoutts> for simplifying the logic I wonder if we could do it by defining a sequence of phases explicitly as a list
14:04 < dcoutts> so we put our conditional logic into the construction of that list of phases
14:04 < dcoutts> and then just sequence_ that list of phases
14:04 < dcoutts> separating deciding what to do from the doing it
14:05 < refold> sounds good
14:05 < dcoutts> ah, that's my comment:
14:05 < dcoutts>      --TODO: do we also need to play the static vs dynamic games here?
14:05 < dcoutts> about the build exe
14:06 < dcoutts> refold: so it would not surprise me at all if this is also needed for the exes
14:06 < dcoutts> the main difference with exes is that currently we only build one way, because we only make one exe at the end, where as for libs we build the lib multiple ways (and optionally can install 
                 multiple ways)
14:08 < refold> I think we can leave that as a TODO for now
14:08 < refold> thanks for the explanation
14:09 < refold> I can file an issue so that we don't forget about it 
14:12 < dcoutts> refold: you mean the TODO/issue of checking that we build exes correctly when TH is being used, for a default-dyn ghc
14:12 < refold> yes
14:12 < refold> dcoutts: surely the test suite will catch this when we'll start to test with GHC 7.8?
14:12 < dcoutts> 'k, thanks
14:12 < dcoutts> refold: it should do, yes
14:13 < dcoutts> refold: it would have been noticed for libs already because ghc uses cabal for building libs
14:13 < dcoutts> so noticed by the ghc devs
14:13 < dcoutts> which no doubt is what prompted the patch

See Distribution.Simple.GHC.buildExe. We should probably check whether GHC is dynamic by default, and if so, do something analogous to what is done in buildLib. The test suite should catch this when we'll start to test with GHC 7.8.

Also, it'd be nice to refactor the logic a bit as @dcoutts suggests above.

@23Skidoo

This comment has been minimized.

Copy link
Member

23Skidoo commented Jun 7, 2013

Related: #1364

@ghost ghost assigned 23Skidoo Jul 11, 2013

23Skidoo added a commit to 23Skidoo/cabal that referenced this issue Jul 11, 2013

Fix building of executables that use TH with dyn-by-default GHC.
Fixes haskell#1252. Makes the test suite pass with GHC HEAD.

(If 'TemplateHaskell/profiling' still fails, you're probably affected by this
GHC bug: http://ghc.haskell.org/trac/ghc/ticket/7978 . Try compiling a more
recent GHC HEAD.)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment