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.

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

enable split-output builds for all haskellPackages #27196

Closed
wants to merge 1 commit into from

Conversation

cleverca22
Copy link
Contributor

for the packages i have tested, data and doc do not have the 1gig chunk of ghc in their closure

Motivation for this change

without this change, any haskell library that references its datadir will cause all programs using it to depend on ghc (over 1gig) at runtime

Things done
  • Tested using sandboxing
    (nix.useSandbox on NixOS,
    or option build-use-sandbox in nix.conf
    on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • Linux
  • 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.

@deepfire
Copy link
Contributor

deepfire commented Jul 7, 2017

Woohoo! : -)

@domenkozar
Copy link
Member

@peti can we merge this to haskell updates and start testing?

@sopvop
Copy link
Contributor

sopvop commented Jul 7, 2017

I think it would be beneficial to also split --dynlibdir and --libdir described here, so you can have dev output with static libs and interface files, and out with runtime dynamically linked libraries.

I think it only works with ghc 8.0+, so it should be conditionally enabled.

@domenkozar
Copy link
Member

I suggest we take this in stages, splitting data will help already, I fear the dynlib/lib split will cause another set of issues :)

@peti
Copy link
Member

peti commented Jul 7, 2017

Pushed in 5e62090696a0e3408c89dd3a694b8fa02de35ac0. Test builds will run at http://hydra.nixos.org/jobset/nixpkgs/haskell-updates.

@nc6
Copy link
Contributor

nc6 commented Jul 7, 2017

Beat me by 12 hours. I've also been working on this - see #27209. Happy to try to work together to get this working!

@cleverca22
Copy link
Contributor Author

looking into this one

cycle detected in the references of ‘/nix/store/ap1zmnpi0xcqmdvyc665iyll6ypyvs89-ghcjs-0.2.0’ from ‘/nix/store/zgs8fi4gcmb9y93jm2y1la3l2lxz3xds-ghcjs-0.2.0-doc’

@cleverca22
Copy link
Contributor Author

cleverca22 commented Jul 8, 2017

ghcjs has references to $out/doc (which doesn't exist) everywhere in the html, which is present in $doc/share/doc, it obeyed the --docdir= for installation, but not for the paths embedded into itself

we will need to either fix ghcjs, or make the doc output optional

@vcunat vcunat added 6.topic: closure size The final size of a derivation, including its dependencies 6.topic: haskell labels Jul 8, 2017
@domenkozar
Copy link
Member

domenkozar commented Jul 8, 2017

About 200 failures, most due to dependencies. Most common cause for failures is 2)

  1. http://hydra.nixos.org/build/55834658/nixlog/1
oving /nix/store/z16da3s1gzygka1hwj3f79b2n0yk12x2-git-annex-6.20170520/share/doc to /nix/store/hqfc9sfn0hkz2lkvryqmcxfxj672jbs8-git-annex-6.20170520-doc/share/doc
rmdir: failed to remove '/nix/store/z16da3s1gzygka1hwj3f79b2n0yk12x2-git-annex-6.20170520/share': Directory not empty
Moving /nix/store/z16da3s1gzygka1hwj3f79b2n0yk12x2-git-annex-6.20170520/share/man to /nix/store/hqfc9sfn0hkz2lkvryqmcxfxj672jbs8-git-annex-6.20170520-doc/share/man
rmdir: failed to remove '/nix/store/z16da3s1gzygka1hwj3f79b2n0yk12x2-git-annex-6.20170520/share': Directory not empty
  1. http://hydra.nixos.org/build/55833992/nixlog/1 http://hydra.nixos.org/build/55834715
builder for ‘/nix/store/pl7xz30jgqizx2716bvv1q9ywkbgxjvx-a50-0.5.drv’ failed to produce output path ‘/nix/store/n1gga3jvsn6mz0m870l4fjhg5xfvmmxa-a50-0.5-doc’
  1. https://nix-cache.s3.amazonaws.com/log/cfaba0ilm8jmr81hzixgvncc28x1w05m-ghc-mod-5.7.0.0.drv
make: option requires an argument -- 'C'
Usage: make [options] [target] ...
Options:
  -b, -m                      Ignored for compatibility.
  -B, --always-make           Unconditionally make all targets.
  -C DIRECTORY, --directory=DIRECTORY
                              Change to DIRECTORY before doing anything.
  -d                          Print lots of debugging information.
  --debug[=FLAGS]             Print various types of debugging information.
  -e, --environment-overrides
                              Environment variables override makefiles.
  --eval=STRING               Evaluate STRING as a makefile statement.
  -f FILE, --file=FILE, --makefile=FILE
                              Read FILE as a makefile.
  -h, --help                  Print this message and exit.
  -i, --ignore-errors         Ignore errors from recipes.
  -I DIRECTORY, --include-dir=DIRECTORY
                              Search DIRECTORY for included makefiles.
  -j [N], --jobs[=N]          Allow N jobs at once; infinite jobs with no arg.
  -k, --keep-going            Keep going when some targets can't be made.
  -l [N], --load-average[=N], --max-load[=N]
                              Don't start multiple jobs unless load is below N.
  -L, --check-symlink-times   Use the latest mtime between symlinks and target.
  -n, --just-print, --dry-run, --recon
                              Don't actually run any recipe; just print them.
  -o FILE, --old-file=FILE, --assume-old=FILE
                              Consider FILE to be very old and don't remake it.
  -O[TYPE], --output-sync[=TYPE]
                              Synchronize output of parallel jobs by TYPE.
  -p, --print-data-base       Print make's internal database.
  -q, --question              Run no recipe; exit status says if up to date.
  -r, --no-builtin-rules      Disable the built-in implicit rules.
  -R, --no-builtin-variables  Disable the built-in variable settings.
  -s, --silent, --quiet       Don't echo recipes.
  -S, --no-keep-going, --stop
                              Turns off -k.
  -t, --touch                 Touch targets instead of remaking them.
  --trace                     Print tracing information.
  -v, --version               Print the version number of make and exit.
  -w, --print-directory       Print the current directory.
  --no-print-directory        Turn off -w, even if it was turned on implicitly.
  -W FILE, --what-if=FILE, --new-file=FILE, --assume-new=FILE
                              Consider FILE to be infinitely new.
  --warn-undefined-variables  Warn when an undefined variable is referenced.

This program built for x86_64-pc-linux-gnu
Report bugs to <bug-make@gnu.org>

builder for ‘/nix/store/cfaba0ilm8jmr81hzixgvncc28x1w05m-ghc-mod-5.7.0.0.drv’ failed with exit code 2
  1. http://hydra.nixos.org/build/55835843

<no location info>: error:
    <command line>: can't load .so/.DLL for: /nix/store/7kbrifnm1f91fhrw8aw0iby5vqa5fbm4-swagger2-2.1.4/lib/ghc-8.0.2/x86_64-linux-ghc-8.0.2/libHSswagger2-2.1.4-1kRqCiIL0QwKbGKLLqXsWv-ghc8.0.2.so (/nix/store/7kbrifnm1f91fhrw8aw0iby5vqa5fbm4-swagger2-2.1.4/lib/ghc-8.0.2/x86_64-linux-ghc-8.0.2/libHSswagger2-2.1.4-1kRqCiIL0QwKbGKLLqXsWv-ghc8.0.2.so: undefined symbol: containerszm0zi5zi7zi1_DataziSetziBase_zdwzdcp1Data_closure)
  1. http://hydra.nixos.org/build/55834386/nixlog/1/tail
/nix/store/fpyal1fxd0d25l2yiph17mqcvkck03nj-servant-0.9.1.1/lib/ghc-8.0.2/package.conf.d/servant-0.9.1.1.conf
ln: failed to create symbolic link '/nix/store/fpyal1fxd0d25l2yiph17mqcvkck03nj-servant-0.9.1.1/share/doc/servant': No such file or directory

builder for ‘/nix/store/gavw8rhi23cqhq2p6y2sk10k174i0l92-servant-0.9.1.1.drv’ failed with exit code 1
  1. http://hydra.nixos.org/build/55835868/nixlog/1/tail
/nix/store/7r371c6ffpgn6w7yw9sdbw8kcnv1h22c-xmonad-0.13/lib/ghc-8.0.2/package.conf.d/xmonad-0.13.conf
mv: missing destination file operand after '/nix/store/7r371c6ffpgn6w7yw9sdbw8kcnv1h22c-xmonad-0.13/share/man/man1/'
Try 'mv --help' for more information.

@@ -323,6 +328,11 @@ stdenv.mkDerivation ({
done
''}

for x in $doc/share/doc/html/src/*.html; do
Copy link
Member

Choose a reason for hiding this comment

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

Missing mkdir -p $doc

@domenkozar
Copy link
Member

Another thing to check is if stack still works with multiple outputs changes.

@peti
Copy link
Member

peti commented Jul 9, 2017

I pushed the updated version to haskell-updates. It's test-built at http://hydra.nixos.org/eval/1374171.

peti added a commit to NixOS/cabal2nix that referenced this pull request Jul 11, 2017
peti added a commit to NixOS/cabal2nix that referenced this pull request Jul 11, 2017
@domenkozar
Copy link
Member

@peti 3 and 5 addressed in peti#69

peti added a commit to NixOS/cabal2nix that referenced this pull request Jul 11, 2017
@domenkozar
Copy link
Member

@peti you seem to be the maintainer of https://nix-cache.s3.amazonaws.com/log/aclka6bq595zkcar1w8hbzwm2j5lk29h-hopenssl-2.2.1.drv - any tips how to fix that one?

@peti
Copy link
Member

peti commented Jul 11, 2017

The hopenssl errors have been there before; that is an issue with the new version requiring Cabal 1.24.x or later; it's not related to the changes made in this PR.

@domenkozar
Copy link
Member

domenkozar commented Jul 11, 2017

There are two leftover packages failing related to this change:

(1) ghcjs: for this @cleverca22 suggests we just disable doc splitting as ghcjs created a reference cycle (bug in the code)

(2) git-annex fails with


builder for ‘/nix/store/yx4hgnmdbs8fhl0lsr3c2gxnlaiwain1-git-annex-6.20170520.drv’ failed to produce output path ‘/nix/store/396fwi9lp7yjdk0idqscrhpmcsb5x58k-git-annex-6.20170520-data’

@domenkozar
Copy link
Member

domenkozar commented Jul 11, 2017

@peti
Copy link
Member

peti commented Jul 12, 2017

@domenkozar, git-annex's build system is so profoundly weird that I see no realistic option to adapt it to multiple outputs without investing plenty of time and effort into re-writing and/or patching its Makefile -- time that I don't have. IMHO, multiple outputs should simply be disabled for that particular build.

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.

We need a reasonably straight-forward way to disable multiple outputs for a given build, i.e. becaus the build uses a strange build system that's not capable of distinguishing binary, data, and doc installations paths.

@domenkozar
Copy link
Member

Agreed :)

@cleverca22 cleverca22 force-pushed the haskell-split-output branch from 6949b55 to 2bbe993 Compare July 14, 2017 21:07
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.

I don't think that this change will do the trick. It assumes that we always have a $doc output, but at least in the case of git-annex this is not the case: there is neither doc nor data, so really need to disable split outputs completely.

hxt-unicode = hasNoDataOutput super.hxt-unicode;
primitive = hasNoDataOutput super.primitive;
regex-base = hasNoDataOutput super.regex-base;
logict = hasNoDataOutput super.logict;
Copy link
Contributor

Choose a reason for hiding this comment

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

Can't we infer this automatically somehow?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

you would have to read the contents of the cabal file, and potentially the Setup.hs if they have modified it

the only other option (which i have temporarily turned back on to skip ahead), is to just make an empty data output always

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, so perhaps cabal2nix could generate it automatically? (most packages don't use a custom Setup.hs)

@peti
Copy link
Member

peti commented Jul 20, 2017

This branch is in a confusing state. :-)

@domenkozar
Copy link
Member

domenkozar commented Jul 20, 2017

@peti if I understand correctly we just need to fix ghcjs (according to your fork and hydra). Then this branch can be closed.

@@ -53,6 +53,8 @@
, coreSetup ? false # Use only core packages to build Setup.hs.
, useCpphs ? false
, hardeningDisable ? lib.optional (ghc.isHaLVM or false) "all"
, hasDataDir ? true
Copy link
Member

Choose a reason for hiding this comment

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

10933 out of 11949 Haskell packages have hasDataDir = false since they don't install any data files. Therefore, false should be the default -- not true -- so that we omit this attribute in 91% of all Haskell Nix expressions.

0d862e9f09a5fc4bf8414fa405df22f0d7c6984e adds that attribute to the package set for all expressions -- regardless of whether it's true or false -- so that we can switch the default in the generic builder easily.

@@ -53,6 +53,8 @@
, coreSetup ? false # Use only core packages to build Setup.hs.
, useCpphs ? false
, hardeningDisable ? lib.optional (ghc.isHaLVM or false) "all"
, hasDataDir ? true
, hasDocDir ? true
Copy link
Member

Choose a reason for hiding this comment

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

What exactly makes a Haskell build have a doc dir? Am I correct in assuming that this equivalent to doHaddock || extra-doc-files /= []?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

if it installs anything into the directory specified by --docdir

@@ -24,15 +24,15 @@ self: super: {
cabal-install = super.cabal-install.overrideScope (self: super: { Cabal = self.Cabal_1_24_2_0; });

# Link statically to avoid runtime dependency on GHC.
jailbreak-cabal = (disableSharedExecutables super.jailbreak-cabal).override { Cabal = self.Cabal_1_20_0_4; };
jailbreak-cabal = hasNoDataOutput ((disableSharedExecutables super.jailbreak-cabal).override { Cabal = self.Cabal_1_20_0_4; });
Copy link
Member

Choose a reason for hiding this comment

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

I believe that those hasNoDataOutput overrides can be safely removed now.

@peti
Copy link
Member

peti commented Jul 20, 2017

if I understand correctly we just need to fix ghcjs (according to your fork and hydra). Then this branch can be closed.

Yes, ghcjs is the only remaining build failure to be fixed. The other errors were there before this effort started and are probably not related to multiple outputs.

peti added a commit to NixOS/cabal2nix that referenced this pull request Jul 24, 2017
@cleverca22 cleverca22 force-pushed the haskell-split-output branch 2 times, most recently from 6b593ff to 964510c Compare July 25, 2017 11:21
@Fuuzetsu
Copy link
Member

Hi, can I do something to help push this forward? Is ghcjs the only remaining broken thing? We're awaiting this quite eagerly.

@domenkozar
Copy link
Member

@peti & @cleverca22: I think there is some miscommunication going on here.

a) hasDataDir is now set by cabal2nix, so the generic builder default is false. Good.
b) hasDocDir is also default false, but it's never really set. @peti can we also detect that in cabal2nix or should be just turn it off for ghcjs and leave it on by default?

peti added a commit to NixOS/cabal2nix that referenced this pull request Jul 26, 2017
@cleverca22 cleverca22 force-pushed the haskell-split-output branch from 964510c to 9062565 Compare July 26, 2017 14:08
@cleverca22 cleverca22 force-pushed the haskell-split-output branch from 9062565 to a39cad9 Compare July 26, 2017 14:10
peti added a commit to NixOS/cabal2nix that referenced this pull request Jul 26, 2017
peti added a commit to NixOS/cabal2nix that referenced this pull request Jul 26, 2017
peti added a commit to NixOS/cabal2nix that referenced this pull request Jul 26, 2017
@cleverca22
Copy link
Contributor Author

ghcjs is failing inside a dep with Setup: haddock-api.cabal: This package requires at least Cabal version 2.0

@peti
Copy link
Member

peti commented Jul 27, 2017

Merged to master in be63b19.

@peti peti closed this Jul 27, 2017
@shlevy
Copy link
Member

shlevy commented Jul 27, 2017

@peti Can/should this be backported?

@peti
Copy link
Member

peti commented Jul 27, 2017

I would prefer to leave this out of release-17.03 since it's a pretty intrusive change with a high risk of breaking user environments or builds. It might be too much for a "stable release branch".

@domenkozar
Copy link
Member

@peti I agree, but that means you won't be able to update cabal2nix there since it now passes the extra flag to packages.

@peti
Copy link
Member

peti commented Jul 27, 2017 via email

@fpletz fpletz added this to the 17.09 milestone Jul 27, 2017
@globin
Copy link
Member

globin commented Jul 27, 2017

Yeah I'd prefer not backporting this. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
6.topic: closure size The final size of a derivation, including its dependencies 6.topic: haskell
Projects
None yet
Development

Successfully merging this pull request may close these issues.