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.

Sign up for GitHub

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
Merged

Add support for setting ghcOptions on all packages #1046

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
14fa435
Add support for setting ghcOptions on all packages
hamishmack Feb 17, 2021
7570cc5
Remove test code
hamishmack Feb 17, 2021
d5563f3
Use new version of nix-tools
hamishmack Feb 17, 2021
393b601
Update date in changelog
hamishmack Feb 18, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions builder/comp-builder.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,8 @@ let self =
&& !(stdenv.hostPlatform.isMusl && !stdenv.hostPlatform.isx86)
, enableDeadCodeElimination ? component.enableDeadCodeElimination

, ghcOptions ? component.ghcOptions

# Options for Haddock generation
, doHaddock ? component.doHaddock # Enable haddock and hoogle generation
, doHoogle ? component.doHoogle # Also build a hoogle index
Expand Down Expand Up @@ -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!

);

setupGhcOptions = lib.optional (package.ghcOptions != null) '' --ghc${lib.optionalString (stdenv.hostPlatform.isGhcjs) "js"}-options="${package.ghcOptions}"'';

executableToolDepends =
(lib.concatMap (c: if c.isHaskell or false
then builtins.attrValues (c.components.exes or {})
Expand Down Expand Up @@ -258,7 +259,7 @@ let

haddock = haddockBuilder {
inherit componentId component package flags commonConfigureFlags
commonAttrs revision setupGhcOptions doHaddock
commonAttrs revision doHaddock
doHoogle hyperlinkSource quickjump setupHaddockFlags
needsProfiling configFiles preHaddock postHaddock pkgconfig;

Expand Down Expand Up @@ -336,7 +337,7 @@ let
buildPhase = ''
runHook preBuild
# https://gitlab.haskell.org/ghc/ghc/issues/9221
$SETUP_HS build ${haskellLib.componentTarget componentId} -j$(($NIX_BUILD_CORES > 4 ? 4 : $NIX_BUILD_CORES)) ${lib.concatStringsSep " " (setupBuildFlags ++ setupGhcOptions)}
$SETUP_HS build ${haskellLib.componentTarget componentId} -j$(($NIX_BUILD_CORES > 4 ? 4 : $NIX_BUILD_CORES)) ${lib.concatStringsSep " " setupBuildFlags}
runHook postBuild
'';

Expand Down
3 changes: 1 addition & 2 deletions builder/haddock-builder.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
, hyperlinkSource
, quickjump
, setupHaddockFlags
, setupGhcOptions

, needsProfiling
, componentDrv
Expand Down Expand Up @@ -108,7 +107,7 @@ let
${lib.optionalString doHoogle "--hoogle"} \
${lib.optionalString hyperlinkSource "--hyperlink-source"} \
${lib.optionalString quickjump "--quickjump"} \
${lib.concatStringsSep " " (setupHaddockFlags ++ setupGhcOptions)}
${lib.concatStringsSep " " setupHaddockFlags}
}
runHook postHaddock
'';
Expand Down
9 changes: 9 additions & 0 deletions changelog.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,15 @@
This file contains a summary of changes to Haskell.nix and `nix-tools`
that will impact users.

## Feb 18, 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.

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"];

## Feb 8, 2021
* Removed older versions of haskell-language-server from custom-tools
(0.8.0 is in hackage so we can still get that version).
Expand Down
5 changes: 0 additions & 5 deletions modules/package.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,11 +136,6 @@ in {
type = bool;
default = false;
};

ghcOptions = mkOption {
type = nullOr str;
default = null;
};
};

components = let
Expand Down
4 changes: 4 additions & 0 deletions modules/plan.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,10 @@ let
type = listOfFilteringNulls str;
default = (def.hardeningDisable or []);
};
ghcOptions = mkOption {
type = listOfFilteringNulls str;
default = def.ghcOptions or [];
};
};


Expand Down
8 changes: 4 additions & 4 deletions nix/sources.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,16 +66,16 @@
"url_template": "https://github.com/<owner>/<repo>/archive/<rev>.tar.gz"
},
"nix-tools": {
"branch": "master",
"branch": "hkm/global-ghc-options",
"builtin": false,
"description": "Translate Cabals Generic Package Description to a Nix expression",
"homepage": null,
"owner": "input-output-hk",
"repo": "nix-tools",
"rev": "f3148a55adcc9ed1361d524f94f3ccf61e2f1392",
"sha256": "0796gd2mr3pwsh7rsxljsbh6z1irb2rj7vqhas59z6xbgb8jqif7",
"rev": "16e3fe6ce9204bc8ac37125fb49fff71b53e0051",
"sha256": "0ks14660mjkhkwjrg748928b40znza7b754kn1srf7rfnw6dscmp",
"type": "tarball",
"url": "https://github.com/input-output-hk/nix-tools/archive/f3148a55adcc9ed1361d524f94f3ccf61e2f1392.tar.gz",
"url": "https://github.com/input-output-hk/nix-tools/archive/16e3fe6ce9204bc8ac37125fb49fff71b53e0051.tar.gz",
"url_template": "https://github.com/<owner>/<repo>/archive/<rev>.tar.gz"
},
"nixpkgs": {
Expand Down
2 changes: 2 additions & 0 deletions overlays/bootstrap.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -588,6 +588,8 @@ in {
final.haskell-nix.cabalProject ({
name = "nix-tools";
src = final.haskell-nix.sources.nix-tools;
# This is a handy way to use a local git clone of nix-tools when developing
# src = final.haskell-nix.haskellLib.cleanGit { name = "nix-tools"; src = ../../nix-tools; };
index-state = final.haskell-nix.internalHackageIndexState;
cabalProjectLocal = ''
allow-newer: Cabal:base, cryptohash-sha512:base, haskeline:base
Expand Down
10 changes: 9 additions & 1 deletion test/ghc-options/cabal.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,15 @@ let
index-state = "2020-05-25T00:00:00Z";
src = testSrc "ghc-options";
# TODO find a way to get the ghc-options into plan.json so we can use it in plan-to-nix
modules = [ { packages.test-ghc-options.package.ghcOptions = "-DTEST_GHC_OPTION"; } ];
modules = [ {
packages.test-ghc-options.ghcOptions = ["-DTEST_GHC_OPTION"];

# This should also work here
# ghcOptions = ["-DTEST_GHC_OPTION"];
# or this
# packages.test-ghc-options.components.library.ghcOptions = ["-DTEST_GHC_OPTION"];
# packages.test-ghc-options.components.exes.test-ghc-options-exe.ghcOptions = ["-DTEST_GHC_OPTION"];
} ];
};
packages = project.hsPkgs;

Expand Down