Skip to content

[rootcling] Construct options after resetting parser#13372

Closed
stephanlachnit wants to merge 1 commit intoroot-project:masterfrom
stephanlachnit:p-rootcling-construct-options-main
Closed

[rootcling] Construct options after resetting parser#13372
stephanlachnit wants to merge 1 commit intoroot-project:masterfrom
stephanlachnit:p-rootcling-construct-options-main

Conversation

@stephanlachnit
Copy link
Copy Markdown
Contributor

This Pull request:

Construct the llvm::cl options in the main function and reset the parser before. This removes any predefined options registered by LLVM (see llvm::cl docs).

Changes or fixes:

Allows building with vanilla LLVM, see also #12156 (comment)

Checklist:

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

This PR fixes #12156

Note on clang-format: the entire file is very clang-format unclean. I tried my best to make it look readable, if you want to clang-format it I would propose to do it separately as it changes almost every line in the file.

@phsft-bot
Copy link
Copy Markdown

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

@phsft-bot
Copy link
Copy Markdown

Build failed on ROOT-ubuntu2004/python3.
See console output.

@stephanlachnit stephanlachnit force-pushed the p-rootcling-construct-options-main branch from 4801498 to 33df5ac Compare August 2, 2023 09:24
@phsft-bot
Copy link
Copy Markdown

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

@phsft-bot
Copy link
Copy Markdown

Build failed on ROOT-ubuntu2004/python3.
See console output.

@phsft-bot
Copy link
Copy Markdown

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

Failing tests:

@github-actions
Copy link
Copy Markdown

github-actions bot commented Aug 3, 2023

Test Results

         7 files           7 suites   1d 20h 58m 43s ⏱️
  2 476 tests   2 475 ✔️ 0 💤 1
16 583 runs  16 582 ✔️ 0 💤 1

For more details on these failures, see this check.

Results for commit 33df5ac.

@Axel-Naumann
Copy link
Copy Markdown
Member

Aren't we relying on some LLVM options that rootcling does not declare explicitly, and that we would lose by resetting the options, iiuc? @vgvassilev ?

@vgvassilev
Copy link
Copy Markdown
Member

As I have mentioned in the original issue, this is not a direction I’d like to go. It does not fix the original issue and seems to deviate from the way llvm suggests using the command line options.

@phsft-bot
Copy link
Copy Markdown

Build failed on ROOT-debian10-i386/soversion.
Running on pcepsft11.dyndns.cern.ch:/home/sftnight/build/workspace/root-pullrequests-build
See console output.

Failing tests:

@Axel-Naumann
Copy link
Copy Markdown
Member

@stephanlachnit given your progress in understanding the root cause in the other issue are you okay with closing this PR?

@stephanlachnit
Copy link
Copy Markdown
Contributor Author

@stephanlachnit given your progress in understanding the root cause in the other issue are you okay with closing this PR?

I haven't yet checked this - I will close it once I confirmed it works with static system libs as well.

@stephanlachnit
Copy link
Copy Markdown
Contributor Author

@stephanlachnit given your progress in understanding the root cause in the other issue are you okay with closing this PR?

I haven't yet checked this - I will close it once I confirmed it works with static system libs as well.

Confirmed that with #13420 this is not needed anymore.

@stephanlachnit stephanlachnit deleted the p-rootcling-construct-options-main branch August 8, 2023 21:20
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!

4 participants