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

Cmake get_directory_property fails on external LLVM #8141

Closed
rarensu opened this issue May 10, 2021 · 8 comments · Fixed by #8766
Closed

Cmake get_directory_property fails on external LLVM #8141

rarensu opened this issue May 10, 2021 · 8 comments · Fixed by #8766

Comments

@rarensu
Copy link

rarensu commented May 10, 2021

  • [check ] Checked for duplicates

Describe the bug

With regard to the file
interpreter/CMakeLists.txt
commit 408de13
moved the line
get_directory_property(LLVM_DEFS DIRECTORY llvm/src COMPILE_DEFINITIONS)
from inside
if (builtin_llvm)
to inside
if (builtin_cling)

Error looks like:

CMake Error at interpreter/CMakeLists.txt:452 (get_directory_property):
get_directory_property DIRECTORY argument provided but requested directory
not found. This could be because the directory argument was invalid or, it
is valid but has not been processed yet.

Expected behavior

The previous method of handling this was to simply skip this procedure. The compile flags from the external LLVM would not be appended to CLING compile flags. The new logic always checks for this dir even if llvm is external. As a result, it no longer works with my external LLVM, which doesn't have a copy of the /src directory in its installation dir. My build system removes the source trees and build dirs to save space on disk. LLVM install dir does contain cmakefiles under lib/cmake/ and those were used to identify it correctly.

To Reproduce

Setup

obtained root from https://root.cern.ch/download/root_v6.24.00.source.tar.gz

Additional context

I am building with cmake and I have builtin_llvm OFF but builtin_cling ON because the internet told me I am not allowed to use my own cling, I have to use the one with root-patches. I am trying very hard to not use the builtin_llvm because it clashes with the other builds on my system so root either fails to build or just crashes, depending on the version. I am an easybuilder and if I can get this to work, lots of other easybuilders will be able to replicate my procedure and build ROOT.

@github-actions
Copy link

Hi @vgvassilev,

It appears this issue is closed, but wasn't yet added to a project. Please add upcoming versions that will include the fix, or 'not applicable' otherwise.

Sincerely,
🤖

7 similar comments
@github-actions
Copy link

Hi @vgvassilev,

It appears this issue is closed, but wasn't yet added to a project. Please add upcoming versions that will include the fix, or 'not applicable' otherwise.

Sincerely,
🤖

@github-actions
Copy link

Hi @vgvassilev,

It appears this issue is closed, but wasn't yet added to a project. Please add upcoming versions that will include the fix, or 'not applicable' otherwise.

Sincerely,
🤖

@github-actions
Copy link

Hi @vgvassilev,

It appears this issue is closed, but wasn't yet added to a project. Please add upcoming versions that will include the fix, or 'not applicable' otherwise.

Sincerely,
🤖

@github-actions
Copy link

Hi @vgvassilev,

It appears this issue is closed, but wasn't yet added to a project. Please add upcoming versions that will include the fix, or 'not applicable' otherwise.

Sincerely,
🤖

@github-actions
Copy link

Hi @vgvassilev,

It appears this issue is closed, but wasn't yet added to a project. Please add upcoming versions that will include the fix, or 'not applicable' otherwise.

Sincerely,
🤖

@github-actions
Copy link

Hi @vgvassilev,

It appears this issue is closed, but wasn't yet added to a project. Please add upcoming versions that will include the fix, or 'not applicable' otherwise.

Sincerely,
🤖

@github-actions
Copy link

Hi @vgvassilev,

It appears this issue is closed, but wasn't yet added to a project. Please add upcoming versions that will include the fix, or 'not applicable' otherwise.

Sincerely,
🤖

pzhristov pushed a commit to alisw/root that referenced this issue Aug 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants