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

Support specialized GHC bindists (#530) #816

Merged
merged 10 commits into from
Sep 18, 2015

Conversation

borsboom
Copy link
Contributor

@3noch @snoyberg: Here's my work so far on this #530. I have some questions I'd like feedback on before I go further:

  • On systems with libgmp4, it'll append -gmp4 to the dist subdirectory (e.g. x86_64-linux-gmp4), which means people already on such a system will end up having to re-run stack setup and their packages will all be re-built. Is that going to be acceptable? I think in the long-run this is the right thing to do, since it means if they upgrade their system to one with libgmp5, things will work properly rather than it trying to use a libgmp4 GHC/build anyway. (A: this is OK)
  • It goes through the detection of libgmp4 when loading the Config, which seems like a bunch of extra overhead to do every time (it has to shell out to ldd). I tried moving that into BuildConfig instead, so it only has to do so when actually going to build something, but since Stack.Setup needs it (and Stack.Setup doesn't have a BuildConfig) that didn't work. I'm thinking I could add a SetupConfig for that case, or another way might be to use RunOnce to only perform the detection when it's actually needed. Opinion? (A: moved into BuildConfig)
  • The old way of using --os=windowsintegersimple no longer works (you'd use --ghc-variant=integersimple instead). Is it worth having it print a warning for this case? I'm not under the impression that very many people are using --os=windowsintegerimsple. (A: no need to print warning)
  • This does not check whether the system-installed GHC has the correct variant (not even sure how I'd go about that). That means, if using system GHC, the --ghc-variant is effectively ignored. Should --ghc-variant imply --no-system-ghc? (A: yes, should imply --no-system-ghc)
  • I'm not sure how this interacts with the new GHCJS support, so @mgsloan please take a look. Is there an integration test that would catch any problems? (note: currently integration tests on master are failing)

Additional tasks:

  • Support providing URL for bindist.
  • Update stack.yaml docs in Wiki
  • Update ChangeLog

@borsboom borsboom added this to the 0.2.0.0 milestone Aug 18, 2015
@3noch
Copy link
Member

3noch commented Aug 18, 2015

So excited to see work on this! While this might only affect a small slice of the stack user base, it will affect them in a tremendously positive way (especially commercial environments I think).

My 0.02 💵:

  • --os=windowsintegersimple never actually worked. It would download the GHC variant, but it didn't properly configure the builds to be usable. I think it's safe to drop support for it.
  • I think it would be wise for --ghc-variant to imply --no-system-ghc [potentially with the possibility of overriding with some later occurrence of --system-ghc]. The only downside is that users can only use variants that stack knows about. Is there some way to make it more flexible than that, maybe by having some "named" variant that must exist at some path (just like custom snapshots)?

My last thought is how this relates to custom snapshots. Is it possible for a custom snapshot to specify a GHC variant? That would be ideal I think, since the two will often go hand-in-hand, sometimes necessarily (in other words, some variants, like integer-simple, will not work with published snapshots at all, so they essentially require the use of custom snapshots).

@borsboom
Copy link
Contributor Author

@3noch: I admit I'm not familiar with how custom snapshots work, but after looking over #111 it does seem like a integration support between these features makes a lot of sense.

I also agree with the suggestion of supporting a custom variant at a particular URL.

@snoyberg
Copy link
Contributor

  • I'm OK with the -gmp4 change
  • I don't have thoughts on the ldd issue, but I definitely agree that avoiding the call every time would be nice
  • Like @3noch: no objection to removing the old broken stuff
  • I'm OK with implying --no-system-ghc too

@borsboom
Copy link
Contributor Author

My last thought is how this relates to custom snapshots. Is it possible for a custom snapshot to specify a GHC variant? That would be ideal I think, since the two will often go hand-in-hand, sometimes necessarily (in other words, some variants, like integer-simple, will not work with published snapshots at all, so they essentially require the use of custom snapshots).

I'm not sure about this. In some cases they would go hand-in-hand, but in other cases they don't (e.g., a GHC build that fixes a bug but is fully compatible). I don't think GHC variants should be entirely coupled with custom snapshots, but having a snapshot be able to require a particular variant like it requires a GHC version probably does make sense.

@3noch
Copy link
Member

3noch commented Aug 23, 2015

@borsboom Yes, that's exactly what I was thinking too.

@3noch
Copy link
Member

3noch commented Aug 23, 2015

Just to get it on the table, I can conceive of a uniform system for custom things (both GHC variants and snapshots). Let's say it were possible to derive a custom snapshot from some other snapshot. Then the overhead of a custom snapshot to achieve a custom GHC variant would be minimal.

Since snapshots are currently responsible for determining GHC version, this seems like a more consistent model.

For example:

# stack.yaml
resolver
  name: my-resolver1
  location: my-resolver1.yaml
# myresolver1.yaml
compiler:
  name: ghc-bugfix
  version: 7.10.2
  location: ...
packages:
  import: lts-3.1 # not a major fan of this example, but you get the idea

@borsboom
Copy link
Contributor Author

I've pushed initial work on support for downloading custom GHC variant bindists. The ghc-resolver setting in stack.yaml now supports values like this (content-length and sha1 are optional):

ghc-variant:
  name: myintegersimple
  url: "https://s3.amazonaws.com/download.fpcomplete.com/ghc/ghc-7.10.2-i386-unknown-mingw32-integer-simple.tar.xz"
  content-length: 86325092
  sha1: 346a193ac16bf8fc050bb0de4355a3d298dfca51

The --ghc-variant argument supports values like:

--ghc-variant=myintegersimple:https://s3.amazonaws.com/download.fpcomplete.com/ghc/ghc-7.10.2-i386-unknown-mingw32-integer-simple.tar.xz

Questions/comments:

  • Currently, the GHC version of the bindist must exactly match the "wanted" version specified by the resolver. I can think of a few ways to improve this:

    1. Parse the filename of the bindist (this would require them all to start with ghc-x.y.z-) to determine the version contained within.
    2. Require the configuration to specify the version.
    3. Support a subset of the format of stack-setup.yaml in stack.yaml. This has the added benefit of allowing you to specify different bindists for different platforms. In addition, that would make it easier for a custom snapshot to refer to a GHC variant by name.

    Thoughts?

  • I'm not sure how useful it is to support custom GHC variants on the command-line. And for the version issue above, the only practical one that would work on the command-line is parsing the filename (anything else makes an already unwieldy argument even worse).

  • This currently doesn't support file:// URLs. Does that matter?

  • Is there any point to supporting integer-simple out of the box anymore? It doesn't work with the standard snapshots anyway, and most people using it would probably be OK with specifying a custom GHC variant.

@snoyberg
Copy link
Contributor

I very much like the idea of stack-setup.yaml subset in stack.yaml. I could imagine having the stack.yaml contents being automatically merged with the stack-setup.yaml contents, and possibly replacing those contents (nice for companies that want to avoid connecting to an external server).

file:// URLs is a nice-to-have, I don't think it's a blocker.

@snoyberg
Copy link
Contributor

A somewhat tangential idea: perhaps there should be some way of overriding the compiler version when using a snapshot, e.g. "I want to use LTS 3.2, but set the GHC version to 7.10.1 instead." I can especially see that as useful when wanting to play with GHC HEAD. Does that fit in here? If not, I'll put it in a separate issue.

@borsboom
Copy link
Contributor Author

perhaps there should be some way of overriding the compiler version when using a snapshot

I think that's a separate issue.

@3noch
Copy link
Member

3noch commented Aug 23, 2015

Awesome!

Would using the term compiler instead of ghc-variant be more consistent with custom snapshots?

@snoyberg Your tangential idea fits nicely with my hope for drivable snapshots. I realize that "derivative snapshot" might not have simple semantics, but it seems that if feasible it could provide a lot of usefulness.

With our without drivable snapshots, I have a sense that moving all "custom" things to custom snapshots would provide a more uniform design. Essentially I imagine that folks could theoretically build stackage snapshots themselves.

@3noch
Copy link
Member

3noch commented Sep 8, 2015

I hope my "grand vision" comments aren't holding this back from incremental improvements. Any status?

@mgsloan
Copy link
Contributor

mgsloan commented Sep 8, 2015

I don't think this has much interaction with GHCJS, except that we won't support some of the variants. I do plan on adding stack setup support for GHCJS soon, but it seems like it'd be prudent to wait for this PR to be merged.

@borsboom
Copy link
Contributor Author

borsboom commented Sep 9, 2015

@3noch, have you tried the code in this PR to make sure it works for your use case?

@3noch
Copy link
Member

3noch commented Sep 11, 2015

Looks like it's getting close:

  • stack setup does not tell the user that a variant is being downloaded/installed (but the destination folder shows that it is)
  • msys2 does not get installed with integersimple:
> stack build
Continuing despite missing tool: msys2
...
    Configuring network-2.6.2.1...
    Setup.hs: The package has a './configure' script. This requires a Unix
    compatibility toolchain such as MinGW+MSYS or Cygwin.

@3noch
Copy link
Member

3noch commented Sep 11, 2015

By the way, now that stack supports sharing of builds between snapshots, I wonder if users would no longer find much benefit from using a custom snapshot (assuming they just want to follow stackage for the most part). But that is only true if stack also shares libraries from the extra-deps field. It seems that this should be easy to accomplish if it's not already.

For using integersimple, most users will not need to add much more than this:

extra-deps:
# these are in the snapshot but we need to override their flags
- hashable-1.2.3.3
- scientific-0.3.3.8
- text-1.2.1.3
# avoid LPGL dependency (integer-gmp)
flags:
  hashable:
    integer-gmp: false
  scientific:
    integer-simple: true
  text:
    integer-simple: true

@borsboom borsboom force-pushed the wip/530-specialized-ghc-bindists branch from 0f5891a to 58ce1d3 Compare September 15, 2015 23:49
- Don't require separate msys2 installation for each GHC variant (local
  programs directory no longer includes GHC variant, instead each GHC
  variant is installed in a separate subdirectory of local programs)
- Specify additional SetupInfo in stack.yaml, either inline or pointing
  to external file/URL
- Add `stack setup --ghc-bindist` argument, instead of including the
  bindist URL with custom `--ghc-variant`
- `stack setup` messages include GHC variant name
- stack-setup.yaml GHC key generated for any GHC variant, not just
  "known" combinations
@borsboom
Copy link
Contributor Author

@3noch I've pushed fixes to the issues you raised:

stack setup does not tell the user that a variant is being downloaded/installed (but the destination folder shows that it is)

Now it'll indicate the variant in most stack setup messages.

msys2 does not get installed with integersimple

stack build continuing without msys2 is a separate issue. stack setup does install msys2, but if you'd previously run it before it msys2 was added and didn't re-run it, it would be missing. However, in testing this, I noticed that it was requiring a separate msys2 installation for each GHC variant, which I've fixed.

I think this now is at the point where it can be merged to master, which I'd like to do today.

Also: I tested the integersimple with the stack.yaml fragment you pasted above, and that worked for me.

@borsboom borsboom force-pushed the wip/530-specialized-ghc-bindists branch from 9fb651b to 1bab31b Compare September 18, 2015 15:46
@3noch
Copy link
Member

3noch commented Sep 18, 2015

@borsboom Sweet. I can't wait to test it out.

@3noch
Copy link
Member

3noch commented Sep 18, 2015

By msys2 do you mean git?

@borsboom
Copy link
Contributor Author

By msys2 do you mean git?

@3noch: git is one part of what gets installed with msys2.

@borsboom borsboom changed the title WIP: Support specialized GHC bindists (#530) Support specialized GHC bindists (#530) Sep 18, 2015
borsboom added a commit that referenced this pull request Sep 18, 2015
…c-bindists

Support specialized GHC bindists (#530)
@borsboom borsboom merged commit de6cff6 into master Sep 18, 2015
@borsboom borsboom deleted the wip/530-specialized-ghc-bindists branch September 18, 2015 18:22
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.

4 participants