Skip to content

Fix FORM's RNTuple backend build against ROOT 6.40 - #749

Merged
gemmeren merged 1 commit into
Framework-R-D:mainfrom
olantwin:fix-form-rntuple-root640
Jul 22, 2026
Merged

Fix FORM's RNTuple backend build against ROOT 6.40#749
gemmeren merged 1 commit into
Framework-R-D:mainfrom
olantwin:fix-form-rntuple-root640

Conversation

@olantwin

@olantwin olantwin commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

ROOT 6.40 moved RRawPtrWriteEntry from ROOT::Experimental::Detail to ROOT::Detail. Select the namespace with a ROOT version check so the RNTuple storage backend (FORM_USE_RNTUPLE_STORAGE=ON) compiles against both ROOT 6.38 and 6.40.

Fixes #712.

  • Code
    • Added ROOT version-aware handling for RRawPtrWriteEntry.
    • Uses ROOT::Detail with ROOT 6.40+ and ROOT::Experimental::Detail for ROOT 6.38.
    • Updated the m_entry member to use the compatibility alias.
  • Compatibility
    • Enables the RNTuple storage backend to compile with ROOT 6.38 and newer, including ROOT 6.40.

ROOT 6.40 moved RRawPtrWriteEntry from ROOT::Experimental::Detail to
ROOT::Detail. Select the namespace with a ROOT version check so the
RNTuple storage backend (FORM_USE_RNTUPLE_STORAGE=ON) compiles against
both ROOT 6.38 and 6.40.

Fixes Framework-R-D#712.
@coderabbitai

coderabbitai Bot commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 960cddab-4792-4cbf-9e90-37c40b1fc420

📥 Commits

Reviewing files that changed from the base of the PR and between 799bb5a and c6ffead.

📒 Files selected for processing (1)
  • form/root_storage/root_rntuple_write_container.hpp
📜 Recent review details
⏰ Context from checks skipped due to timeout. (4)
  • GitHub Check: build (gcc, none)
  • GitHub Check: Analyze cpp with CodeQL
  • GitHub Check: clang-tidy-check
  • GitHub Check: coverage
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{cpp,cc,cxx,h,hpp}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

**/*.{cpp,cc,cxx,h,hpp}: Use clang-format tool for all C++ code formatting (VS Code auto-formats on save); configuration defined in .clang-format with 100-character line limit and 2-space indentation
Follow clang-tidy recommendations defined in .clang-tidy

Files:

  • form/root_storage/root_rntuple_write_container.hpp
**/*.{hpp,cpp}

📄 CodeRabbit inference engine (AGENTS.md)

**/*.{hpp,cpp}: Use .hpp for header files, .cpp for implementation, and *_test.cpp for test files in C++
Enforce 100-character line limit and 2-space indentation in C++ code via .clang-format
Use QualifierAlignment: Right (east-const) style: int const x not const int x in C++
Use PointerAlignment: Left in C++ (pointer * attached to type, not variable name)
All C++ identifiers must use lower_case naming: namespaces, classes, structs, enums, functions, variables, parameters, members, and constants
Exception to C++ naming: template parameters use CamelCase
Exception to C++ naming: macros use UPPER_CASE
Private, protected, and constant members in C++ must have a trailing underscore (_), no trailing underscore on anything else
Use enum class preferred over plain enum in C++
Use std::shared_ptr for shared ownership, std::unique_ptr for exclusive ownership, raw pointers for non-owning references only in C++
Use functors with agent-noun pattern: ModelEvaluator evaluate_model(...) in C++
Apply .clang-tidy checks for bugprone, cert, clang-analyzer, concurrency, cppcoreguidelines, misc, modernize, performance, portability, and readability as defined in the .clang-tidy configuration file
Use phlex:: namespace for core code, phlex::experimental:: for experimental features in C++

Files:

  • form/root_storage/root_rntuple_write_container.hpp
**/*.hpp

📄 CodeRabbit inference engine (AGENTS.md)

Avoid boolean parameters in C++ interfaces; prefer enumerations instead

Files:

  • form/root_storage/root_rntuple_write_container.hpp
🔇 Additional comments (2)
form/root_storage/root_rntuple_write_container.hpp (2)

8-9: LGTM!

Also applies to: 19-30


19-39: 🎯 Functional Correctness

Compile both ROOT namespace branches in CI.

A single ROOT build cannot exercise the other preprocessor branch. ROOT 6.38 uses ROOT::Experimental::Detail::RRawPtrWriteEntry, while ROOT 6.40’s writer API uses ROOT::Detail::RRawPtrWriteEntry; confirm FORM_USE_RNTUPLE_STORAGE=ON builds against both versions. (root.cern)


📝 Walkthrough

Walkthrough

The RNTuple write container now supports ROOT versions before and from 6.40 by conditionally selecting the namespace of RRawPtrWriteEntry.

Changes

ROOT RNTuple compatibility

Layer / File(s) Summary
Version-aware RRawPtrWriteEntry type
form/root_storage/root_rntuple_write_container.hpp
Includes RVersion.h, selects the ROOT namespace based on ROOT_VERSION_CODE, and uses the resulting alias for m_entry.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

  • Framework-R-D/phlex#635: Introduced the RNTuple write container member declaration that this change makes ROOT-version-aware.

Suggested reviewers: wwuoneway

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly states the main change: fixing the RNTuple backend build for ROOT 6.40.
Linked Issues check ✅ Passed The change matches #712 by handling the RRawPtrWriteEntry namespace move while preserving ROOT 6.38 compatibility.
Out of Scope Changes check ✅ Passed The diff appears narrowly scoped to the ROOT version-conditional type fix needed for the linked issue.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@codecov

codecov Bot commented Jul 22, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

@@            Coverage Diff             @@
##             main     #749      +/-   ##
==========================================
- Coverage   83.85%   83.84%   -0.02%     
==========================================
  Files         172      172              
  Lines        7198     7198              
  Branches      845      845              
==========================================
- Hits         6036     6035       -1     
- Misses        882      883       +1     
  Partials      280      280              
Flag Coverage Δ
scripts 80.13% <ø> (ø)
unittests 85.71% <ø> (-0.03%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.
see 2 files with indirect coverage changes


Continue to review full report in Codecov by Harness.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 799bb5a...c6ffead. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@gemmeren
gemmeren requested a review from aolivier23 July 22, 2026 16:15

@gemmeren gemmeren left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks Oliver, LGTM, but @aolivier23 should have a look.

@aolivier23 aolivier23 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good to me. Thank you for pushing this upstream @olantwin @gemmeren

@gemmeren
gemmeren merged commit f4f7e3c into Framework-R-D:main Jul 22, 2026
38 checks passed
@olantwin
olantwin deleted the fix-form-rntuple-root640 branch July 23, 2026 07:34
@olantwin

Copy link
Copy Markdown
Contributor Author

Thanks for the reviews and for accepting the fix!

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.

FORM's RNTuple storage backend does not compile against ROOT ≥ 6.40 (RRawPtrWriteEntry namespace move)

3 participants