llvm-4/clang-4: Build and install man pages #25921

dtzWill · 2017-05-19T14:30:14Z

Motivation for this change

man pages are useful!

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 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.

mention-bot · 2017-05-19T14:30:15Z

@dtzWill, thanks for your PR! By analyzing the history of the files in this pull request, we identified @copumpkin to be a potential reviewer.

LnL7 · 2017-05-19T17:12:09Z

pkgs/development/compilers/llvm/4/clang/default.nix

@@ -13,20 +13,32 @@ let
      mv clang-tools-extra-* $sourceRoot/tools/extra
    '';

-    buildInputs = [ cmake libedit libxml2 llvm python ];
+    buildInputs = [ cmake libedit libxml2 llvm python python.pkgs.sphinx ];


since this is a rebuild for darwin anyway, can you move cmake, ... to nativeBuildInputs

Hmm, okay. I'll push that here although it changes the scope of the PR a bit.. feel free to retitle it if you'd like 👍.

LnL7 · 2017-05-19T17:14:20Z

pkgs/development/compilers/llvm/4/clang/default.nix

    postPatch = ''
      sed -i -e 's/Args.hasArg(options::OPT_nostdlibinc)/true/' lib/Driver/Tools.cpp
      sed -i -e 's/DriverArgs.hasArg(options::OPT_nostdlibinc)/true/' lib/Driver/ToolChains.cpp
+
+      # Patch for standalone doc building
+      sed -i '1s,^,find_package(Sphinx REQUIRED)\n,' docs/CMakeLists.txt
    '';

    outputs = [ "out" "python" ];


perhaps this should include man, since this is part of the darwin stdenv?

Erm, not sure what you mean sorry! Sounds like you're suggesting adding 'man' as an additional output, but I'm not sure what impact that would have on the Darwin stdenv (positive or negative).

yes indeed, adding "man" to the outputs and copying them to $man instead.

without that every darwin user will end up with the manpages on their system, regardless if they installed it in a way that man can even find them.

having a separate output also allows users to install the manpages without polluting their global environment with the cc and clang binaries.

for example:

{ pkgs, ... }: { environment.systemPackages = [ llvm.man clang.man ]; }

Hmm, is there a good way to avoid including the manpages in stdenv but still include them when installing the compiler explicitly?

Looking into it, looks like cc-wrapper propagates the man output which I believe satisfies the "make available when explicitly installed" part of my question above.

Neat re:installing man pages globally without their binaries, I hadn't thought about that and it seems very useful indeed!

I'll make that change shortly, although I don't think i can check whether the Darwin stdenv ends up containing the man page or not.

(rebuilding, will push once it at least builds locally :))

I think meta.outputsToInstall is used when you want to include an output by default when 'installing'. And I'm pretty sure that's not used when it's just a build input.

As for checking the darwin stdenv, I think you can do this and check for clang-*-man.

nix-store -qR $(nix-instantiate -A stdenv --argstr system x86_64-darwin)

Neat! Looks like it's working, then! 👍

dtzWill · 2017-05-19T17:42:11Z

Thanks for the comments/review, updated!

LnL7 · 2017-05-20T11:30:25Z

Looks like something in the stdenv is trying to refer to the man output 😕, also not sure why that breaks the build.

output ‘/nix/store/vrkx52xhl4y54dnvq169g8va5m74786j-stdenv-darwin’ is not allowed to refer to path ‘/nix/store/pmwxqq97xx865pxq64jdgcphfpdzf7gc-clang-4.0.0-man’

LnL7 · 2017-05-21T10:54:29Z

That should do it. I don't think it's possible not to reference the man output unless we change the cc-wrapper to also use multiple outputs.

LnL7 · 2017-05-27T21:35:29Z

FYI. I added support for multiple outputs to the cc-wrapper in #26153

dtzWill · 2017-05-28T19:29:09Z

Woohoo, thanks!

vcunat · 2017-06-01T11:02:35Z

BTW, I'm not convinced that tiny outputs without extra dependencies are worth splitting.

$ du -hs ./result-man/
12K     ./result-man/

EDIT: well, if there was some use case for having the man pages without the rest, that would be a very good reason. At least in general I fail to see use cases for having documentation without the documented thing itself.

LnL7 · 2017-06-01T18:08:08Z

@vcunat Size isn't the main reason why I suggested it. I think installing compilers in the system/user profile is a bad idea because it makes nix-shell -A unreliable. This makes it straightforward to only install the manpages without having to resort to buildEnv with pathsToLink or something.

vcunat · 2017-06-01T19:38:10Z

Hmm, right, that makes good-enough sense to me.

LnL7 added the 1.severity: mass-darwin-rebuild label May 19, 2017

LnL7 reviewed May 19, 2017

View reviewed changes

dtzWill added 2 commits May 19, 2017 15:37

llvm-4/clang-4: Build and install man pages

9aa3548

llvm-4/*: indicate which build inputs are nativeBuildInputs.

3d1c1c5

dtzWill force-pushed the feature/llvm-manpages branch from ad86fcf to 3d1c1c5 Compare May 19, 2017 21:05

darwin-stdenv: allow clang-unwrapped.man

Verified

This commit was signed with the committer’s verified signature. The key has expired.

LnL7 Daiderd Jordan

GPG key ID: D02435D05B810C96
Expired

Verified
Learn about vigilant mode

1e23b49

LnL7 self-assigned this May 23, 2017

LnL7 changed the base branch from master to staging May 27, 2017 21:30

LnL7 approved these changes May 28, 2017

View reviewed changes

LnL7 merged commit 779ec14 into NixOS:staging May 28, 2017

dtzWill deleted the feature/llvm-manpages branch May 28, 2017 19:29

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

llvm-4/clang-4: Build and install man pages #25921

llvm-4/clang-4: Build and install man pages #25921

dtzWill commented May 19, 2017 •

edited

Loading

mention-bot commented May 19, 2017

LnL7 May 19, 2017

dtzWill May 19, 2017

LnL7 May 19, 2017

dtzWill May 19, 2017

LnL7 May 19, 2017

dtzWill May 19, 2017 •

edited

Loading

LnL7 May 19, 2017 •

edited

Loading

LnL7 May 19, 2017

dtzWill May 19, 2017

dtzWill commented May 19, 2017

LnL7 commented May 20, 2017

LnL7 commented May 21, 2017

LnL7 commented May 27, 2017

dtzWill commented May 28, 2017

vcunat commented Jun 1, 2017 •

edited

Loading

LnL7 commented Jun 1, 2017

vcunat commented Jun 1, 2017

llvm-4/clang-4: Build and install man pages #25921

llvm-4/clang-4: Build and install man pages #25921

Conversation

dtzWill commented May 19, 2017 • edited Loading

Motivation for this change

Things done

mention-bot commented May 19, 2017

LnL7 May 19, 2017

Choose a reason for hiding this comment

dtzWill May 19, 2017

Choose a reason for hiding this comment

LnL7 May 19, 2017

Choose a reason for hiding this comment

dtzWill May 19, 2017

Choose a reason for hiding this comment

LnL7 May 19, 2017

Choose a reason for hiding this comment

dtzWill May 19, 2017 • edited Loading

Choose a reason for hiding this comment

LnL7 May 19, 2017 • edited Loading

Choose a reason for hiding this comment

LnL7 May 19, 2017

Choose a reason for hiding this comment

dtzWill May 19, 2017

Choose a reason for hiding this comment

dtzWill commented May 19, 2017

LnL7 commented May 20, 2017

LnL7 commented May 21, 2017

LnL7 commented May 27, 2017

dtzWill commented May 28, 2017

vcunat commented Jun 1, 2017 • edited Loading

LnL7 commented Jun 1, 2017

vcunat commented Jun 1, 2017

dtzWill commented May 19, 2017 •

edited

Loading

dtzWill May 19, 2017 •

edited

Loading

LnL7 May 19, 2017 •

edited

Loading

vcunat commented Jun 1, 2017 •

edited

Loading