Thanks for the review! I resolved everything except for two outstanding discussions.

Okay, second round of review responses is done. There are 3 outstanding discussions, about

• unpacking code
• the right version number for cling
• a question about licenses

Resolved the unpacking code one.

Okay, all feedback has been addressed now.

Oh, I just realized that after changing installPhase to postInstall this is going to pick up a bunch more binaries, since we're basically running cmake on all of LLVM:

ls cling/bin
bugpoint      clang-cl               cling             lli-child-target  llvm-cov      llvm-dis        llvm-lib       llvm-modextract  llvm-PerfectShuffle  llvm-size        llvm-xray  scan-build
c-index-test  clang-cpp              count             llvm-ar           llvm-c-test   llvm-dlltool    llvm-link      llvm-mt          llvm-profdata        llvm-split       not        scan-view
clang         clang-format           FileCheck         llvm-as           llvm-cvtres   llvm-dsymutil   llvm-lto       llvm-nm          llvm-ranlib          llvm-stress      obj2yaml   verify-uselistorder
clang++       clang-import-test      git-clang-format  llvm-bcanalyzer   llvm-cxxdump  llvm-dwarfdump  llvm-lto2      llvm-objdump     llvm-readelf         llvm-strings     opt        yaml2obj
clang-5.0     clang-offload-bundler  llc               llvm-cat          llvm-cxxfilt  llvm-dwp        llvm-mc        llvm-opt-report  llvm-readobj         llvm-symbolizer  sancov     yaml-bench
clang-check   clang-rename           lli               llvm-config       llvm-diff     llvm-extract    llvm-mcmarkup  llvm-pdbutil     llvm-rtdyld          llvm-tblgen      sanstats

This is probably not desirable, maybe we should go back to overriding installPhase ?

What would you put in the installPhase? Right now it calls "make install", before you had "cmake --build . --target install", which should do the same thing.

Oh you're right, this problem existed before that change.

I'm not sure...we definitely don't want cling bringing its own special versions of clang and llvm stuff along with it.

Furthermore, the output currently contains other folders like include and lib. I'm not sure if any of these pose pollution concerns as well (I don't use NixOS).

But for all I know the cling binary expects to find some of those other binaries/includes/libraries.

What do you think about creating another derivation that wraps this one and only outputs bin/cling? That's the only solution I can think of.

I think cling doesn't need llvm or clang binaries to operate, so we could strip those away.

When we build out clang and llvm in separate derivations, first build llvm, then use that to build clang. We should be able to do the same thing with the patched versions of llvm and clang that cling requires.

@Axel-Naumann, if you know, could you please tell us, is it possible to build cling out-of-tree against already installed copy of llvm and clang?

cling needs a patched clang but can work on an original llvm. It's very sensitive to versions though - cling requires llvm 5.0. An upgrade to a newer llvm will happen still this year, but that, too, will then require a specific llvm version. We don't have the resources to maintain compatibility across llvm versions.

@Axel-Naumann Thank you for your answer. We are not planning to compile cling with potentially incompatible versions of clang or llvm, this will only confuse and inconvenience our cling users. The actual question we have is whether we could make separate builds for llvm, clang and cling. Basically, we already know that we can do

cd /src/llvm
cmake -DCMAKE_INSTALL_PATH=/foo/llvm .
make install

and then use this installation of llvm when building clang:

cd /src/clang
PATH=/foo/llvm/bin:$PATH cmake -DCMAKE_INSTALL_PATH=/foo/clang .
make install`

Now the question is: is it okay to also build cling in a separate build? I would imagine it will work something like:

cd /src/cling
PATH=/foo/llvm/bin:/foo/clang/bin:$PATH cmake -DCMAKE_INSTALL_PATH=/foo/cling .
make install

The problem with building cling in-tree is that it will install llvm and clang binaries/headers/libraries, which we would not want to have in the same path. We would also like to have separate builds to improve caching, e.g. we don't want to rebuild llvm on any minor change to cling.

Thanks, now I understand. Yes that should work: we support this for building cling as part of ROOT, against already built llvm and clang. Please let me know if things don't work as expected, or better yet: let me know which changes to CMake you needed to make it work :-)

@Axel-Naumann Thanks again for your insight!

@thomasjm Are you interested in implementing this? I think we could just reuse clang_5 and llvm_5 and override their src to the "cling-patches" versions. This should simplify most problems we are dealing with here. Or we could merge this as is, but this way the cling will probably have conflicts with stdenv on darwin and stdenvClang on linux.

I don't suppose there's some way to parse the output of a derivation to get a list of strings back as a Nix expression? Would be nice to add compilerIncludeFlags back into flags...

I don't suppose there's some way to parse the output of a derivation to get a list of strings back as a Nix expression? Would be nice to add compilerIncludeFlags back into flags...

There is, it's called Import From Derivation (aka IFD), but we generally don't use this feature in nixpkgs and Hydra will not evaluate that at all.

Gotcha. Okay, I don't think there's anything else outstanding here.

@GrahamcOfBorg build cling

Thanks everyone, especially @thomasjm, @Axel-Naumann and @stephane-rolland for their efforts! This is a great work and I'm happy to see this finally getting in. Feel free to open follow up Issues and PR's if we missed something.

thanks all! i'm super excited this is available now 🎉

Hooray! Thanks so much to everyone who reviewed and commented :)

Wooohooo!!