root-project
build: Allow building Cling using the Clang shared library.
The officially supported way to build LLVM/Clang as a shared library is via the LLVM_BUILD_LLVM_DYLIB and LLVM_LINK_LLVM_DYLIB CMake options (see: https://llvm.org/docs/BuildingADistribution.html). When built this way, the whole of Clang API is exposed as a shared library (libclang-cpp.so).
- CMakeLists.txt: Query if we're in shared mode via llvm-config, and register the result as LLVM_LIB_IS_SHARED. [LLVM_LIB_IS_SHARED] <target_link_libraries>: Use the PUBLIC interface of the LLVM shared library.
- lib/Interpreter/CMakeLists.txt [LLVM_LIB_IS_SHARED]: When defined, replace the individual Clang components by clang-cpp.
- lib/MetaProcessor/CMakeLists.txt: Likewise.
- lib/Utils/CMakeLists.txt: Likewise.
- tools/Jupyter/CMakeLists.txt: Likewise.
- tools/driver/CMakeLists.txt: Likewise.
- tools/libcling/CMakeLists.txt: Likewise.
Fixes: https://github.com/root-project/cling/issues/430
- [x ] tested changes locally -> Yes, using the Guix packages for building cling.
This PR fixes https://github.com/root-project/cling/issues/430
Doesn't https://github.com/root-project/cling/issues/430 require a simpler fix by adding libSerialization to the list of linked libraries?
Test Results
12 files 12 suites 2d 16h 35m 44s :stopwatch: 2 642 tests 2 642 :white_check_mark: 0 :zzz: 0 :x: 29 977 runs 29 977 :white_check_mark: 0 :zzz: 0 :x:
Results for commit 094176e6.
:recycle: This comment has been updated with latest results.
![github-actions[bot] avatar](https://avatars.githubusercontent.com/in/15368?v=4 "Published by a pub.dev verified publisher"){kind=small}
Note: We are intentionally building LLVM and Clang as static library to avoid conflict with other uses of the LLVM and Clang libraries (i.e. by keeping the usage as internal as possible). Using shared library will require all other usage of those libraries within a process to be using our version of LLVM and Clang, this has shown to be an impossible requirement.
Note: We are intentionally building
LLVMandClangas static library to avoid conflict with other uses of theLLVMandClanglibraries (i.e. by keeping the usage as internal as possible). Using shared library will require all other usage of those libraries within a process to be using our version ofLLVMandClang, this has shown to be an impossible requirement.
For Guix purposes, I'm building a llvm-cling variant of llvm that reuses most of its definition but uses the Cling's llvm-project sources instead. clang-cling-runtime and clang-cling then cling are the only users. I suppose if a project wanted to link to both LLVM and Cling (libcling) it'd be a problem, but we don't have any such package at the moment, and it wouldn't be too difficult for users to work within a guix shell llvm-cling cling cmake environment, say, to make it possible.
Doesn't root-project/cling#430 require a simpler fix by adding libSerialization to the list of linked libraries?
That wouldn't be sufficient at least for linking to a shared library LLVM.so. When LLVM is built this way (-DLLVM_BUILD_LLVM_DYLIB=ON, which is what LLVM recommends over BUILD_SHARED_LIBS=ON these days, the many lib*.so of LLVM are all gone and everything is in a single LLVM.so file.
libclangSerialization is not part of llvm but clang and won’t be part of llvm.so
For Guix purposes, ...
For other purposes :) we should continue to make sure we can also build LLVM and CLANG in static mode.
For Guix purposes, ...
For other purposes :) we should continue to make sure we can also build LLVM and CLANG in static mode.
The shared library mode is more rigorous of detecting missing link-time dependencies. If we enumerate them all that won't hurt and won't change the way we build ROOT/cling if we want to build it is static mode.
libclangSerialization is not part of llvm but clang and won’t be part of llvm.so
You're right, I answered too quickly. But rereading my original report I had managed to add link directives and get cling to build, but it wouldn't run, so this PR is still necessary/useful to build Cling using shared libraries.
For Guix purposes, ...
For other purposes :) we should continue to make sure we can also build LLVM and CLANG in static mode.
The shared library mode is more rigorous of detecting missing link-time dependencies. If we enumerate them all that won't hurt and won't change the way we build ROOT/cling if we want to build it is static mode.
I agree this change shouldn't break the current way of linking cling statically. The CI suggests this change didn't impact that.
libclangSerialization is not part of llvm but clang and won’t be part of llvm.so
You're right, I answered too quickly. But rereading my original report I had managed to add link directives and get cling to build, but it wouldn't run, so this PR is still necessary/useful to build Cling using shared libraries.
Are you building cling standalone or you are building root?
I am not sure what is the use-case here but generally building something that has an execution engine and llvm in it as a shared library is a bad idea. This is because the symbols of llvm get incorporated in the binary/library and if that binary loads something like libMessa which contains a copy of llvm becomes a huge mess, unless you provide some sort of symbol versioning.
Please think twice about your use-case before building as a shared library. You can get cling as a shared library through the CppInterOp project.
As part of this PR we can probably accept changes in terms of missing dependencies. That is, adding to the list of dependent libraries (such as your change about libclangSerialization).
Standalone.
If I may elaborate, this is what the Guix package definitions for Cling look like:
(define llvm-cling
;; To determine which version of LLVM a given release of Cling should use,
;; consult the
;; https://raw.githubusercontent.com/root-project/cling/master/LastKnownGoodLLVMSVNRevision.txt
;; file.
(let ((base llvm-15)) ;for a DYLIB build
(package/inherit base
(name "llvm-cling")
(version "13-20240318-01")
(source
(origin
(inherit (package-source base))
(method git-fetch)
(uri (git-reference
(url "https://github.com/root-project/llvm-project")
(commit (string-append "cling-llvm" version))))
(file-name (git-file-name "llvm-cling" version))
(sha256
(base32
"1zh6yp8px9hla7v9i67a6anbph140f8ixxbsz65aj7fizksjs1h3"))
(patches (search-patches "clang-cling-13-libc-search-path.patch")))))))
(define clang-cling-runtime
(let ((base clang-runtime-13))
(package/inherit base
(name "clang-cling-runtime")
(version (package-version llvm-cling))
(source (package-source llvm-cling))
(arguments
(substitute-keyword-arguments (package-arguments base)
((#:phases phases '%standard-phases)
#~(modify-phases #$phases
(add-after 'unpack 'change-directory
(lambda _
(chdir "compiler-rt")))
(add-after 'install 'delete-static-libraries
;; This reduces the size from 22 MiB to 4 MiB.
(lambda _
(for-each delete-file (find-files #$output "\\.a$"))))))))
(inputs (modify-inputs (package-inputs base)
(replace "llvm" llvm-cling))))))
(define clang-cling
(let ((base clang-13))
(package/inherit base
(name "clang-cling")
(version (package-version llvm-cling))
(source (package-source llvm-cling))
(arguments
(substitute-keyword-arguments (package-arguments base)
((#:phases phases '%standard-phases)
#~(modify-phases #$phases
(add-after 'unpack 'change-directory
(lambda _
(chdir "clang")))
(add-after 'install 'delete-static-libraries
;; This reduces the size by half, from 220 MiB to 112 MiB.
(lambda _
(for-each delete-file (find-files #$output "\\.a$"))))))))
(propagated-inputs
(modify-inputs (package-propagated-inputs base)
(replace "llvm" llvm-cling)
(replace "clang-runtime" clang-cling-runtime))))))
(define-public cling
(package
(name "cling")
(version "1.0")
(source (origin
(method git-fetch)
(uri (git-reference
(url "https://github.com/root-project/cling")
(commit (string-append "v" version))))
(file-name (git-file-name name version))
(sha256
(base32
"17n66wf5yg1xjc94d6yb8g2gydjz0b8cj4a2pn6xrygdvhh09vv1"))
;; Patch submitted upstream here:
;; https://github.com/root-project/cling/pull/433.
(patches (search-patches "cling-use-shared-library.patch"))))
(build-system cmake-build-system)
(arguments
(list
#:build-type "Release" ;keep the build as lean as possible
#:tests? #f ;FIXME: 78 tests fail (out of ~200)
#:test-target "check-cling"
#:configure-flags
#~(list (string-append "-DCLING_CXX_PATH="
(search-input-file %build-inputs "bin/g++"))
;; XXX: The AddLLVM.cmake module expects LLVM_EXTERNAL_LIT to
;; be a Python script, not a shell executable.
(string-append "-DLLVM_EXTERNAL_LIT="
(search-input-file %build-inputs "bin/.lit-real")))
#:phases
#~(modify-phases %standard-phases
(add-after 'unpack 'set-version
(lambda _
(make-file-writable "VERSION")
(call-with-output-file "VERSION"
(lambda (port)
(format port "~a~%" #$version)))))
(add-after 'unpack 'patch-paths
(lambda* (#:key inputs #:allow-other-keys)
(substitute* "lib/Interpreter/CIFactory.cpp"
(("\\bsed\\b")
(which "sed"))
;; This ensures that the default C++ library used by Cling is
;; that of the compiler that was used to build it, rather
;; than that of whatever g++ happens to be on PATH.
(("ReadCompilerIncludePaths\\(CLING_CXX_RLTV")
(format #f "ReadCompilerIncludePaths(~s"
(search-input-file inputs "bin/g++")))
;; Cling uses libclang's CompilerInvocation::GetResourcesPath
;; to resolve Clang's library prefix, but this fails on Guix
;; because it is relative to the output of cling rather than
;; clang (see:
;; https://github.com/root-project/cling/issues/434). Fully
;; shortcut the logic in this method to return the correct
;; static location.
(("static std::string getResourceDir.*" all)
(string-append all
" return std::string(\""
#$(this-package-input "clang-cling")
"/lib/clang/"
#$(first
(take (string-split
(package-version clang-cling) #\-)
1)) ".0.0" ;e.g. 13.0.0
"\");")))
;; Check for the 'lit' command for the tests, not 'lit.py'
;; (see: https://github.com/root-project/cling/issues/432).
(substitute* "CMakeLists.txt"
(("lit.py")
"lit"))))
(add-after 'unpack 'adjust-lit.cfg
;; See: https://github.com/root-project/cling/issues/435.
(lambda _
(substitute* "test/lit.cfg"
(("config.llvm_tools_dir \\+ '")
"config.cling_obj_root + '/bin"))))
(add-after 'install 'delete-static-libraries
;; This reduces the size from 17 MiB to 5.4 MiB.
(lambda _
(for-each delete-file (find-files #$output "\\.a$")))))))
(native-inputs (list python python-lit))
(inputs (list clang-cling llvm-cling))
(home-page "https://root.cern/cling/")
(synopsis "Interactive C++ interpreter")
(description "Cling is an interactive C++17 standard compliant
interpreter, built on top of LLVM and Clang. Cling can be used as a
read-eval-print loop (REPL) to assist with rapid application development.
Here's how to print @samp{\"Hello World!\"} using @command{cling}:
@example
cling '#include <stdio.h>' 'printf(\"Hello World!\\n\");'
@end example")
(license license:lgpl2.1+))) ;for the combined work
As a distribution (Guix), building as a shared library makes sense to 1. reduce disk size and 2. inherit the base llvm/clang package definitions mostly as-is (which should reduce maintenance).
While linking to different LLVM could be a problem, Guix provides the tools to manipulate the package graph, should someone want to link libcling with Mesa or some other projects that links to the stock LLVM, so that it'd instead link with llvm-cling rather than llvm. Guix and Nix basically give you full control in a sandbox manner of what goes into your build.
Please think twice about your use-case before building as a shared library. You can get cling as a shared library through the CppInterOp project.
Is CppInterOp the future of Cling (that is, will it eventually obsolete it?). I remember reading about some effort integrating Cling or some clang-repl into the LLVM project itself, which would be simplest for users/distributors down the line.
As for my use-case, I tried expounding on the rationale here: https://github.com/root-project/root/pull/15563#issuecomment-2125969791
We have had the same discussions for the conda package. You are trading disk space for correctness and that’s something we should not do.
You can take a look for more details for example here https://github.com/Axel-Naumann/llvm-mesa-reproducer
CppInterOp is a library that enables incremental compilation for third party clients. It won’t replace cling per se but eventually it will provide everything that clings provides using the clang-repl backend.
We have had the same discussions for the conda package. You are trading disk space for correctness and that’s something we should not do.
Do you have a link to that past issue/discussion? It seems to me that this change under review is adding more flexibility without taking anything away. We can also think that at some point in the future, the LLVM project would integrate all the Cling patches, and a shared LLVM would make sense even on classic FHS distributions.
Do you have a link to that past issue/discussion?
https://github.com/conda-forge/cppyy-cling-feedstock/pull/56#issuecomment-1800307065
It seems to me that this change under review is adding more flexibility without taking anything away.
Indeed, if we are adding only the missing dependencies. The current PR does more than that and I am not sure if that is strictly needed.
We can also think that at some point in the future, the LLVM project would integrate all the Cling patches, and a shared LLVM would make sense even on classic FHS distributions.
Yes, that project is called clang-repl. Unfortunately, I do not anticipate all patches to land in llvm because some hardly would make sense there.
@Apteryks, I think I am lost. Are you building cling as a standalone project and you rely on the llvm-config rather than integrating it properly as an external project to llvm?
I think I am lost. Are you building cling as a standalone project and you rely on the llvm-config rather than integrating it properly as an external project to llvm?
Cling is built as a standalone project, in an environment that contains the patched LLVM and Clang built using the -DLLVM_BUILD_LLVM_DYLIB=ON and -DCLANG_LINK_CLANG_DYLIB=ON CMake options (shared library mode), among others. So yes, the LLVM (built from the sources at https://github.com/root-project/llvm-project) is found via llvm-config and the build system (with this proposed change) automatically detects that LLVM/Clang is a shared library and links to libLLVM.so / libclang-cpp.so in that case.
I think llvm discourages that setup generally. Can you use -DLLVM_EXTERNAL_PROJECTS="cling" -DLLVM_EXTERNAL_CLING_SOURCE_DIR=/src/cling? That should be able to carry the right build information to cling, too.
I think llvm discourages that setup generally
Discourage what setup? If you mean -DLLVM_BUILD_LLVM_DYLIB=ON and -DCLANG_LINK_CLANG_DYLIB=ON, these are the recommended switches for a shared library build of LLVM/Clang (see: https://llvm.org/docs/CMake.html). It's used by most distributions nowadays (e.g. on Arch Linux: https://gitlab.archlinux.org/archlinux/packaging/packages/llvm/-/blob/main/PKGBUILD?ref_type=heads#L82, Debian (see: https://metadata.ftp-master.debian.org/changelogs//main/l/llvm-toolchain-13/llvm-toolchain-13_13.0.1-11_changelog), nixpkgs (see: https://github.com/NixOS/nixpkgs/blob/master/pkgs/development/compilers/llvm/common/llvm/default.nix#L326), etc.).
I haven't tried -DLLVM_EXTERNAL_PROJECTS="cling" -DLLVM_EXTERNAL_CLING_SOURCE_DIR=/src/cling, but this suggests I'd need to combine the build of of both cling and llvm as part of the same build, which in Guix we try to avoid as a policy (so that build units are smaller, can be patched individually, and offer more visibility in terms of dependencies, etc.)
I think llvm discourages that setup generally
Discourage what setup? If you mean
-DLLVM_BUILD_LLVM_DYLIB=ONand-DCLANG_LINK_CLANG_DYLIB=ON, these are the recommended switches for a shared library build of LLVM/Clang (see: https://llvm.org/docs/CMake.html). It's used by most distributions nowadays (e.g. on Arch Linux: https://gitlab.archlinux.org/archlinux/packaging/packages/llvm/-/blob/main/PKGBUILD?ref_type=heads#L82, Debian (see: https://metadata.ftp-master.debian.org/changelogs//main/l/llvm-toolchain-13/llvm-toolchain-13_13.0.1-11_changelog), nixpkgs (see: https://github.com/NixOS/nixpkgs/blob/master/pkgs/development/compilers/llvm/common/llvm/default.nix#L326), etc.).
No, relying on the llvm-config binary.
I haven't tried
-DLLVM_EXTERNAL_PROJECTS="cling" -DLLVM_EXTERNAL_CLING_SOURCE_DIR=/src/cling, but this suggests I'd need to combine the build of of both cling and llvm as part of the same build, which in Guix we try to avoid as a policy (so that build units are smaller, can be patched individually, and offer more visibility in terms of dependencies, etc.)
In that case we should use find_package instead. You can see how that is done in the clad project https://github.com/vgvassilev/clad
Unfortunately, I do not understand how the current changes help our project. I can claim they make our cmake more complicated and unreadable. I can see how this helps your package manager but this is the first time I hear about it.
To move forward here, I propose to replace the llvm-config part with find_package for clang and llvm. Most of these changes you inserted will be handled automatically for you.
To move forward here, I propose to replace the llvm-config part with find_package for clang and llvm. Most of these changes you inserted will be handled automatically for you.
Just to make sure, do you mean wholly replacing the llvm-config part (which already exists), or just the newly added --shared-mode probing added in this change?
To move forward here, I propose to replace the llvm-config part with find_package for clang and llvm. Most of these changes you inserted will be handled automatically for you.
Just to make sure, do you mean wholly replacing the llvm-config part (which already exists), or just the newly added
--shared-modeprobing added in this change?
Replacing should be fine.
Replacing should be fine.
I've pushed a new version which implements the change suggested. I was able to use this patch to build cling against a shared library LLVM fine, so it seems to work as intended! Thanks for the suggestion, that's indeed cleaner.
That looks pretty good. Can you confirm you can build and package cling standalone this way?
That looks pretty good. Can you confirm you can build and package cling standalone this way?
Yes. I built the cling package definition shown in this thread using this new patch, against a system provided LLVM shared library (the llvm-project provided by root for cling).