Skip to content

Fix building with external LLVM#13420

Merged
hahnjo merged 2 commits intoroot-project:masterfrom
stephanlachnit:p-external-llvm-13
Aug 10, 2023
Merged

Fix building with external LLVM#13420
hahnjo merged 2 commits intoroot-project:masterfrom
stephanlachnit:p-external-llvm-13

Conversation

@stephanlachnit
Copy link
Copy Markdown
Contributor

This Pull request:

Changes or fixes:

Ensure that LLVM is always linked statically. While doing so this fixes #12156 and #12152. Tested with both LLVM 13 and LLVM 16.

Checklist:

  • tested changes locally
  • updated the docs (if necessary)

This PR fixes #12156 #12152

@phsft-bot
Copy link
Copy Markdown

Starting build on ROOT-performance-centos8-multicore/soversion, ROOT-ubuntu2204/nortcxxmod, ROOT-ubuntu2004/python3, mac11/noimt, mac12arm/cxx20, windows10/default
How to customize builds

@stephanlachnit
Copy link
Copy Markdown
Contributor Author

@Axel-Naumann is a backport of #12153 and this PR possible for a 6.28.xx release?

@phsft-bot
Copy link
Copy Markdown

Build failed on ROOT-ubuntu2004/python3.
Running on root-ubuntu-2004-3.cern.ch:/home/sftnight/build/workspace/root-pullrequests-build
See console output.

Failing tests:

@github-actions
Copy link
Copy Markdown

github-actions bot commented Aug 8, 2023

Test Results

         9 files           9 suites   1d 19h 23m 30s ⏱️
  2 475 tests   2 470 ✔️ 0 💤 5
21 262 runs  21 257 ✔️ 0 💤 5

For more details on these failures, see this check.

Results for commit 66f562d.

@phsft-bot
Copy link
Copy Markdown

Build failed on mac11/noimt.
Running on macphsft23.dyndns.cern.ch:/Users/sftnight/build/workspace/root-pullrequests-build
See console output.

Failing tests:

@bellenot bellenot self-assigned this Aug 9, 2023
Copy link
Copy Markdown
Member

@vgvassilev vgvassilev left a comment

Choose a reason for hiding this comment

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

That looks like the fix I imagined. LGTM! Thanks!

Copy link
Copy Markdown
Member

@hahnjo hahnjo left a comment

Choose a reason for hiding this comment

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

Also LGTM and CI builds are happy, so let's merge.

@hahnjo hahnjo merged commit 92807f3 into root-project:master Aug 10, 2023
@hahnjo
Copy link
Copy Markdown
Member

hahnjo commented Aug 10, 2023

@Axel-Naumann is a backport of #12153 and this PR possible for a 6.28.xx release?

I think that's a good idea. Can you prepare a backport PR against v6-28-00-patches? If not I can also do it. Then up to Axel if we include it already in the next patch release that is expected ~now or later.

@stephanlachnit
Copy link
Copy Markdown
Contributor Author

@Axel-Naumann is a backport of #12153 and this PR possible for a 6.28.xx release?

I think that's a good idea. Can you prepare a backport PR against v6-28-00-patches? If not I can also do it. Then up to Axel if we include it already in the next patch release that is expected ~now or later.

I was hoping I would be able to do so before I leave, but unfortunately I had to move and don't have access to my computer for a while... So I wouldn't mind if you give it a go, but it's also not super high priority.

@hahnjo
Copy link
Copy Markdown
Member

hahnjo commented Aug 14, 2023

So I wouldn't mind if you give it a go, but it's also not super high priority.

no worries, here we go: #13443
Happy moving 🚀

@hahnjo
Copy link
Copy Markdown
Member

hahnjo commented Aug 14, 2023

Hm, it appears the second commit breaks check-cling from ROOT with builtin_llvm=ON because it doesn't include LLVMConfig.cmake anymore, so TARGET_TRIPLE is not available. I think we can fix this in interpreter/CMakeLists.txt by forwarding the respective variable, let me investigate...

edit: #13446

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Building fails with builtin_llvm=OFF: CommandLine Error: Option 'W' registered more than once!

5 participants