Skip to content

Remove header copying in core/clingutils#13054

Merged
hahnjo merged 1 commit into
root-project:masterfrom
hahnjo:clingutils-headers
Jun 21, 2023
Merged

Remove header copying in core/clingutils#13054
hahnjo merged 1 commit into
root-project:masterfrom
hahnjo:clingutils-headers

Conversation

@hahnjo
Copy link
Copy Markdown
Member

@hahnjo hahnjo commented Jun 21, 2023

No description provided.

@hahnjo hahnjo requested a review from Axel-Naumann June 21, 2023 06:30
@hahnjo hahnjo self-assigned this Jun 21, 2023
@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/cxx14
How to customize builds

@Axel-Naumann
Copy link
Copy Markdown
Member

Standalone cling doesn't use modules; does it work even without copying headers, in a relocatable way? We might be able to turn this off for ROOT though.

@hahnjo
Copy link
Copy Markdown
Member Author

hahnjo commented Jun 21, 2023

Standalone cling doesn't use modules; does it work even without copying headers, in a relocatable way? We might be able to turn this off for ROOT though.

Unless I'm missing something obvious here, this isn't used for Cling standalone at all - core/clingutils/CMakeLists.txt is a ROOT thing, no?

@Axel-Naumann
Copy link
Copy Markdown
Member

🤦

@github-actions
Copy link
Copy Markdown

Test Results

       11 files         11 suites   2d 2h 12m 23s ⏱️
  2 469 tests   2 469 ✔️ 0 💤 0
25 483 runs  25 483 ✔️ 0 💤 0

Results for commit cff6b1b.

@Axel-Naumann
Copy link
Copy Markdown
Member

You should also test with -Druntime_cxxmodules=Off, please.

@hahnjo
Copy link
Copy Markdown
Member Author

hahnjo commented Jun 21, 2023

@phsft-bot build with flags -Druntime_cxxmodules=Off -DCTEST_TEST_EXCLUDE_NONE=On

@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/cxx14 with flags -Druntime_cxxmodules=Off -DCTEST_TEST_EXCLUDE_NONE=On
How to customize builds

@phsft-bot
Copy link
Copy Markdown

@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:

@hahnjo
Copy link
Copy Markdown
Member Author

hahnjo commented Jun 21, 2023

Build failed on ROOT-performance-centos8-multicore/cxx17.

These failure are pre-existing, I opened #13058.

I briefly discussed with Axel, and we decided to merge this for now. In the worst case, somebody building their own ROOT binaries without C++ runtime modules will complain when installing on a system with non-matching headers, in which case we'll investigate (hopefully all experiments enable C++ runtime modules by now).

@hahnjo hahnjo marked this pull request as ready for review June 21, 2023 14:46
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 reasonable to me.

@hahnjo hahnjo merged commit 02ef417 into root-project:master Jun 21, 2023
@hahnjo hahnjo deleted the clingutils-headers branch June 21, 2023 15:46
maksgraczyk pushed a commit to maksgraczyk/root that referenced this pull request Jun 28, 2023
Most headers have include guards these days and copying them from the
build system doesn't seem necessary anymore.
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.

4 participants