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.

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

Jump to bottom

haskellPackages.ghcWithPackages: Wrap haddock with GHC lib dir #37132

Merged
merged 1 commit into from Mar 19, 2018
Merged

haskellPackages.ghcWithPackages: Wrap haddock with GHC lib dir #37132

merged 1 commit into from Mar 19, 2018

Conversation

srhb
Copy link
Contributor

@srhb srhb commented Mar 15, 2018 •
edited

Motivation for this change

#36976

haddock isn't wrapped up in ghcWithPackages and friends. I think it's safe to do so, but I'm unsure whether this is the right approach, so I'm looking for feedback.

Things done

Wrapped up haddock with only -B$NIX_${ghcCommandCaps}_LIBDIR. I think this is sufficient, but maybe it doesn't hurt to simply use the wrapper used for eg. the ghc executable, which adds more env vars?

  • Tested using sandboxing (nix.useSandbox on NixOS, or option build-use-sandbox in nix.conf on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Fits CONTRIBUTING.md.

@srhb srhb requested a review from peti as a code owner March 15, 2018 18:11
peti
peti approved these changes Mar 15, 2018
View reviewed changes
Copy link
Member

@peti peti left a comment

Choose a reason for hiding this comment

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

The change looks good to me. Did you (or the person who reported this problem originally) test whether this fix actually solves the problem?

An alternative suggestion would be to try and use the haskellPackages.haddock program rather than the one bundled with ghc, because that tool should work automatically due to the patches we apply to ghc-paths.

peti
peti reviewed Mar 15, 2018
View reviewed changes
pkgs/development/haskell-modules/with-packages-wrapper.nix Outdated
@@ -95,6 +95,14 @@ symlinkJoin {
makeWrapper ${ghc}/bin/$prg $out/bin/$prg --add-flags "${packageDBFlag}=${packageCfgDir}"
fi
done

if [[ -x "${ghc}/bin/haddock" ]]; then
Copy link
Member

@peti peti Mar 15, 2018 •
edited

Choose a reason for hiding this comment

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

Oh, could you please add a reference to #36976 into the source code as a comment? That makes it easier for people who are reading the source code to grok why that wrapper is necessary.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oops, github got confused by my (too eager) squash. Here's the comment:

# haddock was referring to the base ghc, https://github.com/NixOS/nixpkgs/issues/36976

@peti peti added this to the 18.03 milestone Mar 15, 2018
@srhb
Copy link
Contributor Author

srhb commented Mar 15, 2018

Here's what I tested, inspired by @mrkkrp

~/nixpkgs nixos-unstable
❯ nix-shell -p "pkgs.haskellPackages.ghcWithPackages (pkgs: with pkgs; [ lens ])" --run 'haddock --optghc -package --optghc lens; echo $?'
<command line>: cannot satisfy -package lens
    (use -v for more information)
1

❯ git checkout haddock-wrapped

~/nixpkgs haddock-wrapped
❯ nix-shell -p "pkgs.haskellPackages.ghcWithPackages (pkgs: with pkgs; [ lens ])" --run 'haddock --optghc -package --optghc lens; echo $?'
0

@srhb srhb changed the title WIP: haskellPackages.ghcWithPackages: Wrap haddock with GHC lib dir haskellPackages.ghcWithPackages: Wrap haddock with GHC lib dir Mar 15, 2018
@mrkkrp mrkkrp mentioned this pull request Mar 19, 2018
Closed
@grahamc grahamc merged commit 81da63f into NixOS:master Mar 19, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@peti peti peti approved these changes

Assignees
No one assigned
Labels
0.kind: bug 6.topic: haskell 10.rebuild-darwin: 1-10 10.rebuild-linux: 11-100
Projects
None yet
Milestone
18.03
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@srhb @peti @grahamc @GrahamcOfBorg