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

Command-line --ghc-options don't seem to pass through #827

Closed
afcowie opened this Issue Aug 21, 2015 · 14 comments

Comments

Projects
None yet
4 participants
@afcowie
Contributor

afcowie commented Aug 21, 2015

The instructions to build leksah say to do:

cabal build regex-tdfa-text --ghc-options=-XFlexibleContexts

So I tried same with stack:

stack build regex-tdfa-text --ghc-options=-XFlexibleContexts     # and, just in case
stack build --ghc-options=-XFlexibleContexts regex-tdfa-text

And it didn't pick up FlexibleContexts, build fails. #758 gave me the impression this technique should work?

I then discovered you can add ghc-options sections to a stack.yaml, like so:

ghc-options:
  regex-tdfa-text: -XFlexibleContexts

(and then realized the one added to the leksah repo turns out to have had that, but I wasn't then using 'master', was trying to build release) and it built.

Shouldn't --ghc-options=-XBlah have passed -XBlah through to enable the Blah language extension? The stack documentation talks about this being preferable to doing it in stack.yaml so snapshot version isn't impacted (not that anyone else is using this package, but anyway).

[I gather the issue on point is https://github.com/leksah/leksah/issues/156 but I wanted to report my attempted workaround and my impression that it should have worked]

AfC

@snoyberg

This comment has been minimized.

Contributor

snoyberg commented Aug 21, 2015

Consider the implications of applying the --ghc-options to every single compiled package:

  1. If we apply it consistently: we'd need to rebuild every single snapshot package as an extra dep to ensure that the GHC option is used by everything in the snapshot
  2. If we apply it inconsistently, then not-yet-compiled packages would be compiled with it, either polluting the snapshot database with a non-standard flag, or installing into the local database. Either way: the result you get would depend on when you ran it, making it a non reproducible step

The logic we have now is to apply ghc-options to all local targets. This can in fact be confusing, but the alternatives are also all confusing. See https://github.com/commercialhaskell/stack/wiki/FAQ#stack-sometimes-rebuilds-based-on-flag-changes-when-i-wouldnt-expect-it-to-how-come and the linked discussion for more details.

Like flags, I think the best approach is to be explicit about which package you're applying this change to.

The stack documentation talks about this being preferable to doing it in stack.yaml so snapshot version isn't impacted (not that anyone else is using this package, but anyway).

Can you point out where in the documentation you see that?

@afcowie

This comment has been minimized.

Contributor

afcowie commented Aug 21, 2015

@snoyberg spake thus:

Can you point out where in the documentation you see that?

Last sentence of https://github.com/commercialhaskell/stack/wiki/stack.yaml#ghc-options?

@snoyberg

This comment has been minimized.

Contributor

snoyberg commented Aug 21, 2015

The last line is slightly wrong, I've updated it from "targets" to "local targets." That said, I don't think it's telling you "you should do it this way," but telling you the actual trade-offs between the two. The language though is unclear; could you take a hand at improving it based on my description above?

@afcowie

This comment has been minimized.

Contributor

afcowie commented Aug 21, 2015

Sure, happy to. I still don't quite understand why the -X didn't pass through. Is it because it was an explicit build target on the command line, and not an "extra dependency" in stack.yaml? Hm, no that can't be it. Hm. Ok, I guess I don't understand what a "local target" is. If I can figure that out I'll edit the Wiki.

(I get why its hard due to 1 & 2 above)

That said, presumably the right thing to do is amend the help to say "Do not try to use --ghc-options to pass in -X flags, use ghc-options: in stack.yaml instead"? Or could the command line have been constrained to have that option act on that one package only?

(the last time I tried to amend your help it didn't end well, so I'm not doing a pull request unless you say to)

@snoyberg

This comment has been minimized.

Contributor

snoyberg commented Aug 21, 2015

Pull request welcome. You're not changing a highly controversial naming this time ;)

"local targets" refer to things that exist in the packages section of the YAML file, and are referred to on the command line. Those are things that the user is actively working on, and therefore make sense to change settings on easily. Things which are not actively being worked on typically should use the defaults in order to ensure reproducible builds.

@afcowie

This comment has been minimized.

Contributor

afcowie commented Aug 21, 2015

Ok. So when I said stack build blah where blah wasn't the local project, that counted as a dep (extra or otherwise) and thus --ghc-options didn't apply?

@snoyberg

This comment has been minimized.

Contributor

snoyberg commented Aug 21, 2015

Correct. I considered changing it to apply to extra-deps as well. However, that would likely have a very disastrous effect for a command use case. Imagine a stack.yaml such as:

resolver: lts-3.0
extra-deps:
- text-1.2.3
packages: [.]

If you then ran stack build --ghc-options=-XFoo, it would force text and everything that depends on it to be recompiled.

@afcowie

This comment has been minimized.

Contributor

afcowie commented Aug 21, 2015

Ok. I have it now. I'll make the edits this weekend.

AfC

P.S. Why can't I assign this to myself?

@snoyberg

This comment has been minimized.

Contributor

snoyberg commented Aug 21, 2015

Cool, thanks!

On Fri, Aug 21, 2015 at 7:46 AM, Andrew Cowie notifications@github.com
wrote:

Ok. I have it now. I'll make the edits this weekend.

AfC


Reply to this email directly or view it on GitHub
#827 (comment)
.

@snoyberg

This comment has been minimized.

Contributor

snoyberg commented Aug 31, 2015

Bumping

@snoyberg

This comment has been minimized.

Contributor

snoyberg commented Oct 9, 2015

One more bump before closing

@afcowie

This comment has been minimized.

Contributor

afcowie commented Oct 21, 2015

Sorry @snoyberg I will get to this sometime this month but if someone else wants to do it first by all means.

sjakobi added a commit that referenced this issue Jul 22, 2016

@sjakobi

This comment has been minimized.

Contributor

sjakobi commented Jul 22, 2016

I'm trying to fix this issue in #2402.

You may want to check if what I wrote is actually correct. ;)

@mgsloan

This comment has been minimized.

Collaborator

mgsloan commented Jul 26, 2016

Closing as the documentation issue is resolved.

@mgsloan mgsloan closed this Jul 26, 2016

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