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

Applying options to all packages of stack.yaml #1089

Closed
meteficha opened this Issue Oct 2, 2015 · 15 comments

Comments

Projects
None yet
4 participants
@meteficha
Member

meteficha commented Oct 2, 2015

Background:
I have a stack.yaml with many packages. Many times I'll

$ stack test --pedantic foo-package

because I'm not interested in running all of the test suites.

What stack does:
Builds every dependency of the foo-package as if --pedantic wasn't given, then builds foo-package with --pedantic.

What I expect:
Builds dependencies using --pedantic if, and only if, they are specified as a package on stack.yaml. Always builds packages given as argument with --pedantic.

Why this is an issue:
While working on foo-package I'm only interested in running its test suite because that's where I'm writing new tests. However, I may have to change something on one of the other packages. Any small issue that gives an warning will break the CI while still working on my machine.

Furthermore, if you switch between stack test --pedantic and stack test --pedantic foo-package, you'll notice that stack needs to recompile dependencies because their flags changed.

@meteficha

This comment has been minimized.

Member

meteficha commented Oct 2, 2015

I should add that this applies to other options such as --ghc-options. I'll edit the title of the issue.

@meteficha meteficha changed the title from Weird --pedantic behavior to Applying options to all packages of stack.yaml Oct 2, 2015

@bergmark

This comment has been minimized.

Member

bergmark commented Oct 2, 2015

Related

I still want this solved differently as well... I'd like to be able to set this in stack.yaml and have it apply to all local dependencies. As a second step it would also be useful to be able to say "build all test suites and compile with -Werror, except for this package"

Previously I ran stack build --pedantic without arguments but then had to wait through extra compilation time for packages I didn't care about. At the moment stack ghci makes the most efficient path to never add any flags while developing, and then recompiling everything afterwards with --pedantic.

@snoyberg

This comment has been minimized.

Contributor

snoyberg commented Oct 4, 2015

This was already discussed at length, see https://github.com/commercialhaskell/stack/blob/master/doc/faq.md#stack-sometimes-rebuilds-based-on-flag-changes-when-i-wouldnt-expect-it-to-how-come and the linked issue. There are going to stack.yaml changes made to allow setting ghc-options there. But the exact change requested here almost certainly will not become a part of stack, since it breaks reproducibility.

@bergmark

This comment has been minimized.

Member

bergmark commented Oct 4, 2015

@snoyberg to clarify, are you saying stack build --pedantic package-name will stay as is, but if ghc-options is provided in stack.yaml they will be passed to all local packages?

@meteficha

This comment has been minimized.

Member

meteficha commented Oct 5, 2015

For the record, the reproducibility is not an issue. It just exacerbates the real issue of not passing flags the way I expect it.

@snoyberg

This comment has been minimized.

Contributor

snoyberg commented Oct 5, 2015

The problem with this topic is that we can't discuss one piece of it without pulling it many others. I gave this all some thought over the past few days, I'll try to summarize (pinging @borsboom, who's working on some related things).

There are essentially two different questions at play right now:

  1. Should we force a recompile of a package when the GHC options change?
    • Argument in favor: it's the only way to get true reproducibility from Stack, otherwise the order in which stack build is called with different ghc-options affects the result.
    • Argument against: the most common cases of GHC options do not actually affect results in a meaningful way. In particular, changes to -Wall and -Werror don't necessarily change anything, since GHC itself won't recompile modules based on it.
  2. Which packages are affected by the --ghc-options command line option? Currently, we apply them to targets. Perhaps the proposal @meteficha is making here is to apply it to all locals instead. A third option that will be available is explicitly stating which package gets which options, which is the most explicit, but not very convenient. There are two things we need to consider for this:
    • What do users expect?
    • What avoids the most unnecessary recompilation?

I'm willing to reopen the discussion on (1), specifically since experience has shown the argument against to be very strong. I'm also open to discussion on (2), but I have a strong inclination against changing it without also changing (1), as that would break a common workflow (stack build && cd some-subdir && stack build . --pedantic).

@meteficha

This comment has been minimized.

Member

meteficha commented Oct 5, 2015

  1. Yes, packages should be recompiled when GHC options change, regardless of which options were changed. It's better to recompile more than necessary than otherwise.
  2. Yes, that's what I want with this issue :). I'll discuss only about this point from now on.

Being a power user, I don't really care much about the "how" as long as I can automate it (e.g., I can put explicit options on a shell script once and for all). So all of the options you've mentioned work for me except for the status quo.

Obviously you need to work out how Stack should work for everyone else, though :). I honestly have no idea what users expect, outside the fact that the --flag option sets a precedent for package-explicit options.

@snoyberg

This comment has been minimized.

Contributor

snoyberg commented Oct 5, 2015

To be fair, even the status quo does allow you to do exactly what you're after:

stack build dep1 dep2 ... depN target-package:test-suite1 target-package:test-suite2... --pedantic
@borsboom

This comment has been minimized.

Contributor

borsboom commented Oct 6, 2015

For question (1), my feeling is that once we're passing GHC options on the command-line, we've basically thrown reproducibility out the window anyway, since we could pass all sorts of options in that change the build result. I'd be in favour of Stack not rebuilding non-target packages if the command-line ghc-options change. However, changing ghc-options in stack.yaml should still force a rebuild. I think this would exhibit the behaviour most people would expect.

With that change, (2) probably should be left as-is. Saying that --ghc-options applies to all locals, only to have them ignored in many cases (since they wouldn't trigger a rebuild in non-targets), would just be confusing.

@bergmark

This comment has been minimized.

Member

bergmark commented Oct 6, 2015

This sounds good to me @borsboom!

@borsboom

This comment has been minimized.

Contributor

borsboom commented Oct 6, 2015

It might also be nice to have an extra option (like --ghc-options-all-locals but less awful) that does apply to all locals and does force rebuilds on change. I could see that being useful in scripts, where you really do want reproducibility with a specific set of command-line options.

@borsboom borsboom added this to the P2: Should milestone Oct 6, 2015

@borsboom borsboom self-assigned this Oct 6, 2015

snoyberg added a commit that referenced this issue Oct 8, 2015

@snoyberg

This comment has been minimized.

Contributor

snoyberg commented Oct 8, 2015

I think it's clear that no set of decisions here will make everyone happy, so (following @borsboom's ideas in a chat we had) I've added two new config options:

  • rebuild-ghc-options determines whether a change in options forces a package to be rebuilt. It defaults to false (instead of the current behavior, which is true)
  • apply-ghc-options specifies which packages are affected by the --ghc-options on the command line. The default is now locals (in line with @meteficha's request), with other values of targets (current behavior) and everything (affects snapshot/extradeps too, with a doc caveat that this is almost certainly a bad idea)

I think this is the best we can do right now. @meteficha want to review?

@snoyberg snoyberg assigned snoyberg and unassigned borsboom Oct 8, 2015

@meteficha

This comment has been minimized.

Member

meteficha commented Oct 8, 2015

I'll admit that I don't fully grasp @borsboom's comment. I don't see why reproducibility goes out of the window when you pass explicit GHC options. Thus, my opinion remains that rebuild-ghc-options should be true.

The new apply-ghc-options seems awesome! This is exactly what I wanted :). (BTW, your suggestion of listing more targets works but has the caveat that now the other target's test suite is being run as well. This is a small caveat, though, because its test suite runs while the another package builds.)

@snoyberg

This comment has been minimized.

Contributor

snoyberg commented Oct 8, 2015

(BTW, your suggestion of listing more targets works but has the caveat that now the other target's test suite is being run as well. This is a small caveat, though, because its test suite runs while the another package builds.)

Are you using stack test/stack build --test, or just plain stack build? If you're explicitly stating the packages being used and the test suite to be run, you should just use stack build, which shouldn't run other test suites.

As for the default for rebuild-ghc-options: I know at least I've gotten enough feedback from confused users (not just here) to tip me towards the default being false, which is why I made it (not just this issue).

Anyway, looks like this is tied up now, closing.

@meteficha

This comment has been minimized.

Member

meteficha commented Oct 8, 2015

Thank you, @snoyberg and @borsboom! :shipit:

I'm using stack test. I thought stack build wouldn't run any test suites unless you specified --test. In any case, for my use case it has become moot point with the new apply-ghc-options :).

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