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

Support specialized GHC bindists #530

Closed
3noch opened this Issue Jul 6, 2015 · 25 comments

Comments

Projects
None yet
5 participants
@3noch
Member

3noch commented Jul 6, 2015

Continuing discussion in #495 and related to #399.

stack's understanding of GHC is a bit primitive as it only knows about version, OS, and architecture. It would help to augment this understanding to other [potentially arbitrary] factors. The current use-case is GHC bindists built with integer-simple instead of integer-gmp. Since this concept is orthogonal to version, OS, and architecture, stack has no good way to represent it. Thus, stack cannot currently

  • represent the constraint via the yaml file or command-line arguments
  • install specialized GHCs automatically
  • check for existing presence

Currently the resolver is responsible for determining which version of GHC to use, and something else determines which OS/architecture distribution to use.

@3noch

This comment has been minimized.

Member

3noch commented Jul 6, 2015

When it comes to integer-simple bindists, I don't know if there's any easy way to "check for existing presence." However, it seems that the Cabal system has some knowledge of integer-simple as a dependency, so perhaps that could be used to detect presence.

@3noch

This comment has been minimized.

Member

3noch commented Jul 6, 2015

That said, checking for presence is not very high on the priority list IMHO. If stack can install an integer-simple build itself, then foregoing support for existing builds is less important for now.

@radix

This comment has been minimized.

Contributor

radix commented Jul 6, 2015

is "debugging build" another factor here? I noticed that the blog post at https://www.fpcomplete.com/user/bitonic/perf-for-low-level-profiling mentions that you need the RTS to be built with debug symbols for useful haskell-program debugging. (I kinda think this should be the default... maybe it already is?)

@borsboom

This comment has been minimized.

Contributor

borsboom commented Jul 6, 2015

Specifying a GHC variant on stack.yaml could be tricky, because you may want different variants on different platforms. For example, integer-simple more likely to be wanted on Windows. On Linux, you might want the gmp4 (centos65) variant on some distributions but not on others, and it would be rare to actually specify that rather than autodetect it.

@3noch

This comment has been minimized.

Member

3noch commented Jul 6, 2015

@radix I'm not sure, but I do imagine that there are numerous variants we might want to support.
@manny-fp Certainly it gets complicated, but this takes us closer to that goal, not farther. As it is now, you have to add custom hacks to stack itself to gain these behaviors. If you need something akin to a configure script, you could actually write one that produced a stack.yaml file appropriate for the environment.

@3noch

This comment has been minimized.

Member

3noch commented Jul 6, 2015

@manny-fp Bear in mind we already have the os option, which is not exactly "cross-platform" either. I think the point is that stack.yaml allows you to constrain your build as much as you need it to. Certainly no one is forcing you to use the os field. If you need really complicated constraints on different systems, you could script them, like I said.

@radix

This comment has been minimized.

Contributor

radix commented Jul 6, 2015

it seems like this begs for conditionals in the yaml. scary! :)

@3noch

This comment has been minimized.

Member

3noch commented Jul 6, 2015

Hmm...perhaps something like Setup.hs is needed! ;)

@borsboom

This comment has been minimized.

Contributor

borsboom commented Jul 6, 2015

@3nock Agreed, definitely moving in the right direction. So I'd propose something along these lines to start with:

  1. Create a newtype CompilerVariant = String. It's a free-form string so that we can add a new variant just by adding an entry to https://github.com/fpco/stackage-content/blob/master/stack/stack-setup.yaml rather than needing to change any code. I suppose we could follow the OS/Arch approach of having a sum type with the known types and an additional OtherCompilerVariant String, which would have the benefit of not needing to hard-code strings for cases like the gmp4 variant which is determined programatically.
  2. Create a data PlatformVariant = PlatformVariant Platform (Maybe CompilerVariant). Probably should be a record type, really.
  3. Use PlatformVariant most places that Platform is used now.
  4. Add a --ghc-variant command-line option and corresponding stack.yaml option.
  5. Have Stack.Setup use the CompilerVariant in getOSKey.
  6. Move the gmp4 (libgmp.so.3) detection code to where the default for CompilerVariant is set.
@borsboom

This comment has been minimized.

Contributor

borsboom commented Jul 6, 2015

And let's leave conditionals in stack.yaml as a separate issue :)

@3noch

This comment has been minimized.

Member

3noch commented Jul 7, 2015

As far as I can tell (which is not very far!) that plan looks good. I'm not crazy about the name PlatformVariant since it seems that we're not dealing with variants of the platform but only of the compiler. If we must combine the two ideas, I wonder if something like data Compiler = CompilerFor Platform (Maybe CompilerVariant) is best. But recall that I can't tell too far.

Are you thinking of making the programmatic detection be a subset of CompilerVariant? The Maybe begs the question "What's the default?" Perhaps we could avoid that confusion by embedding every case:

data CompilerVariant = AvoidLPGL | IntegerSimpleCompiler | IntegerGmpCompiler
                     | OtherCompilerVariant String

(Of course, you could squash some of these into OtherCompilerVariant or reintroduce Maybe, etc. with similar effect.)

If using AvoidLGPL, we could use logic like this

  • If GHC version < 7.12 (hopefully!)
    • On Windows? Use integer-simple.
    • On Linux without libgmp? Use integer-simple.
    • On Linux with libgmp? Use integer-gmp.
  • Otherwise use linker flag or make the user do it herself
@3noch

This comment has been minimized.

Member

3noch commented Jul 7, 2015

This actually gave me another idea: stack could check for licensing problems in your dependency tree. Perhaps it's a non issue since virtually all cabal packages are BSD3, but it could be quite useful to know if one of them wasn't.

@borsboom

This comment has been minimized.

Contributor

borsboom commented Jul 10, 2015

stack could check for licensing problems in your dependency tree

That is a good idea (it's something we've done at FP Complete in other projects), and definitely deserves an issue of its own so that it doesn't get lost.

@borsboom

This comment has been minimized.

Contributor

borsboom commented Jul 11, 2015

I'm not crazy about the name PlatformVariant since it seems that we're not dealing with variants of the platform but only of the compiler

Yeah, I agree it's not a great name, and your suggestion is am improvement.

data CompilerVariant = AvoidLPGL | IntegerSimpleCompiler | IntegerGmpCompiler | OtherCompilerVariant String

I like the sum type, and perhaps adding a DefaultCompiler option rather than using a Maybe. The default (or Nothing) would be the "standard" bindist from the GHC team. When constructing paths, this case would leave the compiler variant out of the path (e.g. x86_64-linux) where the others would include it (e.g. x86_64-linux-integersimple).

I do think the CompilerVariant should identify a particular variant (e.g. IntegerSimple, Gmp4) rather than a "class" of variants (e.g. AvoidLGPL), because we're going to use it to construct paths and should ensure that one variant always ends up with the same string. We could add another type for a user to specify the "class" of compiler and use that after parsing the config/args to decide the CompilerVariant to use.

@borsboom

This comment has been minimized.

Contributor

borsboom commented Jul 11, 2015

Also, is this something you're interested/willing to work on?

@3noch

This comment has been minimized.

Member

3noch commented Jul 11, 2015

I'm on board with all of that. I have limited resources to devote to this, but I am willing to work on it. I would likely need some direction. Also I can only really provide testing for Windows and perhaps a tiny bit of Linux. I can't test Mac at all.

@snoyberg snoyberg modified the milestone: 0.2.0.0 Jul 13, 2015

@borsboom

This comment has been minimized.

Contributor

borsboom commented Jul 13, 2015

I'm not sure how much guidance I can give without taking a stab at it first myself, so I think I'll get started on it and see where things go.

@borsboom borsboom self-assigned this Jul 13, 2015

@3noch

This comment has been minimized.

Member

3noch commented Jul 13, 2015

Great, tell me if/how I can help.

@3noch

This comment has been minimized.

Member

3noch commented Jul 13, 2015

I added an issue for detecting licensing problems: #577

@3noch

This comment has been minimized.

Member

3noch commented Jul 20, 2015

@manny-fp Have you been able to make any progress on this?

@borsboom

This comment has been minimized.

Contributor

borsboom commented Jul 21, 2015

@3noch: Not yet, planning to work on it this week.

@3noch

This comment has been minimized.

Member

3noch commented Jul 31, 2015

@borsboom Since you were asking for integer-simple builds I am curious if this got somewhere.

@rvion

This comment has been minimized.

Contributor

rvion commented Aug 18, 2015

just a quick note: between ghc-musl and integer simple, I don't know what would be the best to push forward

borsboom added a commit that referenced this issue Aug 18, 2015

borsboom added a commit that referenced this issue Aug 18, 2015

@borsboom

This comment has been minimized.

Contributor

borsboom commented Aug 18, 2015

Work in progress PR: #816

@borsboom borsboom referenced this issue Aug 18, 2015

Merged

Support specialized GHC bindists (#530) #816

6 of 8 tasks complete

borsboom added a commit that referenced this issue Aug 22, 2015

borsboom added a commit that referenced this issue Aug 22, 2015

borsboom added a commit that referenced this issue Aug 23, 2015

borsboom added a commit that referenced this issue Sep 18, 2015

Merge pull request #816 from commercialhaskell/wip/530-specialized-gh…
…c-bindists

Support specialized GHC bindists (#530)

@borsboom borsboom closed this Sep 18, 2015

@borsboom

This comment has been minimized.

Contributor

borsboom commented Sep 18, 2015

Merged the PR and closing this issue, but please re-open if you run into any trouble.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment