Skip to content

[x3d] make the C library a static private standalone builtin independent of ROOT - #23151

Merged
linev merged 6 commits into
root-project:masterfrom
ferdymercury:bx3d2
Aug 27, 2026
Merged

[x3d] make the C library a static private standalone builtin independent of ROOT#23151
linev merged 6 commits into
root-project:masterfrom
ferdymercury:bx3d2

Conversation

@ferdymercury

@ferdymercury ferdymercury commented Aug 26, 2026

Copy link
Copy Markdown
Collaborator

This Pull request:

Changes or fixes:

Addresses intertwining concerns raised in #23138 (comment)

In my opinion, if x3d is going to be removed soon, this extra PR is not needed, intertwining will disappear by itself, and having things in builtins is better towards Debian porting even if a bit wired during the temporary deprecation/removal phase.

If it's not going to be removed, this helps, since Debian packagers might think of packaging this variant of x3d separately rather than having a single monolithic ROOT build system.

Checklist:

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

@ferdymercury ferdymercury added clean build Ask CI to do non-incremental build on PR skip code analysis Skip the code analysis CI steps for this PR, including verifying clang-formatting and running Ruff. labels Aug 26, 2026
@ferdymercury ferdymercury reopened this Aug 26, 2026
@github-actions

github-actions Bot commented Aug 26, 2026

Copy link
Copy Markdown

Test Results

    23 files      23 suites   3d 13h 22m 17s ⏱️
 3 855 tests  3 853 ✅ 0 💤 2 ❌
78 574 runs  78 572 ✅ 0 💤 2 ❌

For more details on these failures, see this check.

Results for commit 36d4eec.

♻️ This comment has been updated with latest results.

@linev

linev commented Aug 26, 2026

Copy link
Copy Markdown
Member

Maybe name for library x3d is too short.
We should try to avoid use of such short names. Something like ROOTx3d will be better.

Comment thread builtins/x3d/CMakeLists.txt Outdated
Comment thread graf3d/g3d/CMakeLists.txt Outdated
Comment thread graf3d/TGeometry/CMakeLists.txt Outdated
Comment thread graf3d/x3d/CMakeLists.txt Outdated
Comment thread montecarlo/eg/CMakeLists.txt Outdated
Comment thread builtins/x3d/CMakeLists.txt Outdated
as suggested by linev

Co-authored-by: ferdymercury <ferdymercury@users.noreply.github.com>
@ferdymercury ferdymercury changed the title [x3d] make the C library a private standalone builtin independent of ROOT [x3d] make the C library a static private standalone builtin independent of ROOT Aug 26, 2026
@ferdymercury
ferdymercury marked this pull request as ready for review August 26, 2026 11:43
Comment thread builtins/x3d/CMakeLists.txt Outdated

@pcanal pcanal left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM (beside the minor issue of the library name)

Comment thread builtins/x3d/CMakeLists.txt Outdated
Comment thread builtins/x3d/CMakeLists.txt Outdated
Comment thread builtins/x3d/CMakeLists.txt Outdated
Comment thread graf3d/g3d/CMakeLists.txt Outdated
Comment thread graf3d/TGeometry/CMakeLists.txt Outdated
Comment thread graf3d/x3d/CMakeLists.txt Outdated
Comment thread montecarlo/eg/CMakeLists.txt Outdated
ferdymercury and others added 2 commits August 27, 2026 00:33
to prevent ambiguity in case insensitive file systems if we later rename X3d to ROOTX3D
as suggested by pcanal

Co-authored-by: Philippe Canal <pcanal@fnal.gov>
Co-authored-by: ferdymercury <ferdymercury@users.noreply.github.com>
@ferdymercury

Copy link
Copy Markdown
Collaborator Author

Good point, renaming applied. Thanks!

@linev
linev merged commit a229b06 into root-project:master Aug 27, 2026
@ferdymercury
ferdymercury deleted the bx3d2 branch August 27, 2026 05:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

clean build Ask CI to do non-incremental build on PR pr:squash on merge skip code analysis Skip the code analysis CI steps for this PR, including verifying clang-formatting and running Ruff.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants