Skip to content

Add support for setting ghcOptions on all packages #1046

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

Merged
merged 4 commits into from
Feb 18, 2021

Conversation

hamishmack
Copy link
Collaborator

No description provided.

@hamishmack hamishmack requested a review from angerman February 17, 2021 10:16
@@ -1,6 +1,15 @@
This file contains a summary of changes to Haskell.nix and `nix-tools`
that will impact users.

## Feb 17, 2021
* `ghcOptions` has been moved from package and is now a list of strings.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why the move? Presumably the idea is that at some point in the future cabal might give us some per-package ghc-options and then putting them on package would make sense, no? Or is the idea that they could come per-component? But I'm not sure cabal will let you set per-component options...

List of strings is great, though.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The move is needed to make it work like the other "global options". We want to support a cabal.project file with:

package *
  ghc-options: -g3

We can't currently convert that as it is not captured in the plan.json output (if it was it would be all good). Since it is not we want to at least support adding a module:

modules = [{ ghcOptions = ["-g3"]; }];

We have code in pace to make this sort of "global option", but they do not end up in the package attribute. Rather than making it a special case somehow it seems better just to move it. It just happens that the "global options" also wind up at the component level.

The option to add them at the component level is not something cabal.project currently supports, however it seems likely that if they are put in the plan.json file they may be stored on the component node (so it might still be useful there).

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I meant moving where the option lives. Can we not do the same thing while still leaving the per-package option in packages.foo.package.ghcOptions rather than packages.foo.ghcOptions? My understanding was that the former represents "stuff we get from cabal"?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The option to add them at the component level is not something cabal.project currently supports, however it seems likely that if they are put in the plan.json file they may be stored on the component node (so it might still be useful there).

That's a good point, though. I guess we don't know where they'll end up so we're guessing regardless.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As far as I can tell the options that can be set global are created here from packageOptions. Those same packageOptions are exposed again here for package level and here for component level. The package section does not seem to include support for setting the options global in this way.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My understanding was that the former represents "stuff we get from cabal"?

The only thing that sets package.ghcOptions is stack-to-nix from the stack.yaml file and user modules. When ghc-options are set in a .cabal file they do not make it into nix at all. Instead they are picked up by setup configure and used by setup build (cabal-to-nix does not do anything with ghcOptions).

In the future the cabal.project file and plan.json might provide GHC options.

hamishmack added a commit to input-output-hk/nix-tools that referenced this pull request Feb 17, 2021
@hamishmack
Copy link
Collaborator Author

bors try

iohk-bors bot added a commit that referenced this pull request Feb 17, 2021
@iohk-bors
Copy link
Contributor

iohk-bors bot commented Feb 17, 2021

@@ -180,10 +182,9 @@ let
++ lib.optionals useLLVM [
"--ghc-option=-fPIC" "--gcc-option=-fPIC"
]
++ map (o: ''--ghc${lib.optionalString (stdenv.hostPlatform.isGhcjs) "js"}-options="${o}"'') ghcOptions
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to be clear, the idea is that by including them in the call to configure we don't need to include them everywhere else like we were doing before?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes I noticed that we passed other --ghc-options arguments (like the enableDWARF one) to configure instead of to build. I can't think of a reason to treat them differently. It was confusing that the list of configure args that we log to the output in the did not include the --ghc-options from this source.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, it seems way better if it works!

@hamishmack hamishmack merged commit 42b1067 into master Feb 18, 2021
@iohk-bors iohk-bors bot deleted the hkm/global-ghc-options branch February 18, 2021 11:38
hamishmack added a commit to input-output-hk/nix-tools that referenced this pull request Feb 25, 2021
booniepepper pushed a commit to booniepepper/haskell.nix that referenced this pull request Feb 4, 2022
)

`ghcOptions` has been moved from package and is now a list of strings.
    old: packages.x.package.ghcOptions = "someGHCoption";
    new: packages.x.ghcOptions = ["someGHCoption"];
To specify ghcOptions for all packages:
    ghcOptions = ["someGHCoption"];
For a single component:
    packages.x.compoents.library.ghcOptions = ["someGHCoption"];
andreabedini pushed a commit to andreabedini/haskell.nix that referenced this pull request Sep 14, 2022
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.

2 participants