From f1ae7c4ea9e5c569cdff7e37891a67f2ad4c80d1 Mon Sep 17 00:00:00 2001 From: Philippe Canal Date: Sun, 8 Feb 2026 17:34:45 +0100 Subject: [PATCH 01/21] io: Correct handling of byte count pos in TBufferFile::ReadObjectAny --- io/io/src/TBufferFile.cxx | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/io/io/src/TBufferFile.cxx b/io/io/src/TBufferFile.cxx index 92bacafdcd231..fbe038bff88e5 100644 --- a/io/io/src/TBufferFile.cxx +++ b/io/io/src/TBufferFile.cxx @@ -2593,7 +2593,8 @@ void *TBufferFile::ReadObjectAny(const TClass *clCast) InitMap(); // before reading object save start position - UInt_t startpos = UInt_t(fBufCur-fBuffer); + ULong64_t startpos = static_cast(fBufCur-fBuffer); + ULong64_t cntpos = startpos <= kMaxCountPosition ? startpos : kOverflowPosition; // attempt to load next object as TClass clCast UInt_t tag; // either tag or byte count @@ -2613,7 +2614,7 @@ void *TBufferFile::ReadObjectAny(const TClass *clCast) Error("ReadObject", "got object of wrong class! requested %s but got %s", clCast->GetName(), clRef->GetName()); - CheckByteCount(startpos, tag, (TClass *)nullptr); // avoid mis-leading byte count error message + CheckByteCount(cntpos, tag, (TClass *)nullptr); // avoid mis-leading byte count error message return 0; // We better return at this point } baseOffset = 0; // For now we do not support requesting from a class that is the base of one of the class for which there is transformation to .... @@ -2628,7 +2629,7 @@ void *TBufferFile::ReadObjectAny(const TClass *clCast) //we cannot mix a compiled class with an emulated class in the inheritance Error("ReadObject", "trying to read an emulated class (%s) to store in a compiled pointer (%s)", clRef->GetName(),clCast->GetName()); - CheckByteCount(startpos, tag, (TClass *)nullptr); // avoid mis-leading byte count error message + CheckByteCount(cntpos, tag, (TClass *)nullptr); // avoid mis-leading byte count error message return 0; } } @@ -2640,7 +2641,7 @@ void *TBufferFile::ReadObjectAny(const TClass *clCast) obj = (char *) (Longptr_t)fMap->GetValue(startpos+kMapOffset); if (obj == (void*) -1) obj = nullptr; if (obj) { - CheckByteCount(startpos, tag, (TClass *)nullptr); + CheckByteCount(cntpos, tag, (TClass *)nullptr); return (obj + baseOffset); } } @@ -2652,7 +2653,7 @@ void *TBufferFile::ReadObjectAny(const TClass *clCast) MapObject((TObject*) -1, startpos+kMapOffset); else MapObject((void*)nullptr, nullptr, fMapCount); - CheckByteCount(startpos, tag, (TClass *)nullptr); + CheckByteCount(cntpos, tag, (TClass *)nullptr); return 0; } @@ -2711,7 +2712,7 @@ void *TBufferFile::ReadObjectAny(const TClass *clCast) // let the object read itself clRef->Streamer( obj, *this, clOnfile ); - CheckByteCount(startpos, tag, clRef); + CheckByteCount(cntpos, tag, clRef); } return obj+baseOffset; From 48ada53c26a8046b941b29c55aab89a197fd403f Mon Sep 17 00:00:00 2001 From: Philippe Canal Date: Wed, 11 Feb 2026 15:22:34 +0100 Subject: [PATCH 02/21] io: Fix up clarify byte count related limits --- io/io/src/TBufferFile.cxx | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/io/io/src/TBufferFile.cxx b/io/io/src/TBufferFile.cxx index fbe038bff88e5..69d35e703ad9f 100644 --- a/io/io/src/TBufferFile.cxx +++ b/io/io/src/TBufferFile.cxx @@ -347,7 +347,8 @@ void TBufferFile::WriteCharStar(char *s) void TBufferFile::SetByteCount(ULong64_t cntpos, Bool_t packInVersion) { - assert( cntpos <= kOverflowPosition && (sizeof(UInt_t) + cntpos) < static_cast(fBufCur - fBuffer) + assert( (cntpos == kOverflowPosition || + (cntpos < kOverflowPosition && (sizeof(UInt_t) + cntpos) < static_cast(fBufCur - fBuffer))) && (fBufCur >= fBuffer) && static_cast(fBufCur - fBuffer) <= std::numeric_limits::max() && "Byte count position is after the end of the buffer"); From 6ff2c262f1c007fbbb7e7a2a4b533e3fd378a2b8 Mon Sep 17 00:00:00 2001 From: Philippe Canal Date: Thu, 12 Feb 2026 23:29:16 +0100 Subject: [PATCH 03/21] io: In TBufferFile::ReadClass fix recording of byte count pos. This fixes TBufferFile::ReadObjectAny handling of long range byte counts --- io/io/src/TBufferFile.cxx | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/io/io/src/TBufferFile.cxx b/io/io/src/TBufferFile.cxx index 69d35e703ad9f..ff72db6ba2dc8 100644 --- a/io/io/src/TBufferFile.cxx +++ b/io/io/src/TBufferFile.cxx @@ -2599,6 +2599,7 @@ void *TBufferFile::ReadObjectAny(const TClass *clCast) // attempt to load next object as TClass clCast UInt_t tag; // either tag or byte count + // ReadClass will push on fByteCountStack if needed. TClass *clRef = ReadClass(clCast, &tag); TClass *clOnfile = nullptr; Int_t baseOffset = 0; @@ -2826,7 +2827,7 @@ TClass *TBufferFile::ReadClass(const TClass *clReq, UInt_t *objTag) // When objTag is not used, the caller is not interested in the byte // count and will not (can not) call CheckByteCount. if (objTag) - fByteCountStack.push_back(fBufCur - fBuffer); + fByteCountStack.push_back(fBufCur - fBuffer - sizeof(UInt_t)); startpos = UInt_t(fBufCur-fBuffer); *this >> tag; } From 5fefdae80c5a3d3254b077de409835bfc74066f7 Mon Sep 17 00:00:00 2001 From: Philippe Canal Date: Wed, 11 Feb 2026 13:19:29 +0100 Subject: [PATCH 04/21] io: compile testLargeByteCounts --- io/io/test/CMakeLists.txt | 8 ++++++-- io/io/test/{testByteCount.C => testByteCount.cxx} | 10 +++++++++- io/io/test/testByteCount.ref | 1 + 3 files changed, 16 insertions(+), 3 deletions(-) rename io/io/test/{testByteCount.C => testByteCount.cxx} (96%) diff --git a/io/io/test/CMakeLists.txt b/io/io/test/CMakeLists.txt index b65a1886a6d17..61f381acf0274 100644 --- a/io/io/test/CMakeLists.txt +++ b/io/io/test/CMakeLists.txt @@ -41,6 +41,10 @@ endif() # ROOT_ADD_TEST(io-io-test-TMapFileTest COMMAND TMapFileTest complete) #endif() +ROOTTEST_COMPILE_MACRO(testByteCount.cxx + FIXTURES_SETUP io-io-tobj-fixture) + ROOTTEST_ADD_TEST(testLargeByteCounts - MACRO testByteCount.C - OUTREF testByteCount.ref) + MACRO testByteCount.cxx+ + OUTREF testByteCount.ref + FIXTURES_REQUIRED io-io-tobj-fixture) diff --git a/io/io/test/testByteCount.C b/io/io/test/testByteCount.cxx similarity index 96% rename from io/io/test/testByteCount.C rename to io/io/test/testByteCount.cxx index b152e3700aa75..07a6966786e56 100644 --- a/io/io/test/testByteCount.C +++ b/io/io/test/testByteCount.cxx @@ -1,6 +1,14 @@ +#include "TBufferFile.h" +#include "TExMap.h" + +#include +#include + + +int testByteCount() { int errors = 0; - int expectedByteCounts = 1; + unsigned int expectedByteCounts = 1; // TBufferFile currently reject size larger than 2GB. // SetBufferOffset does not check against the size, diff --git a/io/io/test/testByteCount.ref b/io/io/test/testByteCount.ref index a5a2b78b46929..2ac0fa3121d7c 100644 --- a/io/io/test/testByteCount.ref +++ b/io/io/test/testByteCount.ref @@ -1,2 +1,3 @@ Processing io/io/test/testByteCount.C... The end. +(int) 0 From bdf54a5152b20407483ed350dd580ffcc6d5e376 Mon Sep 17 00:00:00 2001 From: Philippe Canal Date: Mon, 16 Feb 2026 11:26:16 -0600 Subject: [PATCH 05/21] io: Add missing CheckByteCount in TStreamerElements. Those were essentially replaced by a 'set the cursor to the end' call in commit 7d25b75b8877a03ee579e97d91ee8f719721d6d5. It is dubious whether these changes are needed or were an heurestic to ignore (temporarily !?) race conditions. --- core/meta/src/TStreamerElement.cxx | 14 +++++++++++++- io/io/src/TStreamerInfo.cxx | 4 ++++ 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/core/meta/src/TStreamerElement.cxx b/core/meta/src/TStreamerElement.cxx index 561fca629178d..ef74518c37948 100644 --- a/core/meta/src/TStreamerElement.cxx +++ b/core/meta/src/TStreamerElement.cxx @@ -556,9 +556,12 @@ void TStreamerElement::Streamer(TBuffer &R__b) if (R__v > 3) { if (TestBit(kHasRange)) GetRange(GetTitle(),fXmin,fXmax,fFactor); } - //R__b.CheckByteCount(R__s, R__c, TStreamerElement::IsA()); R__b.ClassEnd(TStreamerElement::Class()); + // Dubious reset to the expected end it was added as part the commit + // log "Several protections added in case of multiple files being read/updated + // in parallel." R__b.SetBufferOffset(R__s+R__c+sizeof(UInt_t)); + R__b.CheckByteCount(R__s, R__c, TStreamerElement::Class()); ResetBit(TStreamerElement::kCache); ResetBit(TStreamerElement::kWrite); @@ -832,6 +835,7 @@ void TStreamerBase::Streamer(TBuffer &R__b) } R__b.ClassEnd(TStreamerBase::Class()); R__b.SetBufferOffset(R__s+R__c+sizeof(UInt_t)); + R__b.CheckByteCount(R__s, R__c, TStreamerBase::Class()); } else { R__b.WriteClassBuffer(TStreamerBase::Class(),this); } @@ -1001,7 +1005,11 @@ void TStreamerBasicPointer::Streamer(TBuffer &R__b) R__b >> fCountVersion; fCountName.Streamer(R__b); fCountClass.Streamer(R__b); + // Dubious reset to the expected end it was added as part the commit + // log "Several protections added in case of multiple files being read/updated + // in parallel." R__b.SetBufferOffset(R__s+R__c+sizeof(UInt_t)); + R__b.CheckByteCount(R__s, R__c, TStreamerBasicPointer::Class()); } else { R__b.WriteClassBuffer(TStreamerBasicPointer::Class(),this); } @@ -1105,7 +1113,11 @@ void TStreamerLoop::Streamer(TBuffer &R__b) R__b >> fCountVersion; fCountName.Streamer(R__b); fCountClass.Streamer(R__b); + // Dubious reset to the expected end it was added as part the commit + // log "Several protections added in case of multiple files being read/updated + // in parallel." R__b.SetBufferOffset(R__s+R__c+sizeof(UInt_t)); + R__b.CheckByteCount(R__s, R__c, TStreamerLoop::Class()); } else { R__b.WriteClassBuffer(TStreamerLoop::Class(),this); } diff --git a/io/io/src/TStreamerInfo.cxx b/io/io/src/TStreamerInfo.cxx index 3900b54b37cf8..4fb7814c48b8c 100644 --- a/io/io/src/TStreamerInfo.cxx +++ b/io/io/src/TStreamerInfo.cxx @@ -5503,7 +5503,11 @@ void TStreamerInfo::Streamer(TBuffer &R__b) R__b.ClassMember("fElements","TObjArray*"); R__b >> fElements; R__b.ClassEnd(TStreamerInfo::Class()); + // Dubious reset to the expected end it was added as part the commit + // log "Several protections added in case of multiple files being read/updated + // in parallel." R__b.SetBufferOffset(R__s+R__c+sizeof(UInt_t)); + R__b.CheckByteCount(R__s, R__c, TStreamerInfo::Class()); ResetBit(kIsCompiled); ResetBit(kBuildOldUsed); ResetBit(kBuildRunning); From ca9ac34a4d5be6eb90b1d891b57dcff2489e6367 Mon Sep 17 00:00:00 2001 From: Philippe Canal Date: Mon, 16 Feb 2026 11:30:58 -0600 Subject: [PATCH 06/21] io/roofit: Correct CheckByteCount placement. Due to the need for using a stack of bytecount position, we can no longer support redundant calls to CheckByteCount. --- roofit/roofitcore/src/RooCategory.cxx | 4 ++-- roofit/roofitcore/src/RooDataHist.cxx | 1 - 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/roofit/roofitcore/src/RooCategory.cxx b/roofit/roofitcore/src/RooCategory.cxx index 054f44d760e61..4b416087b426d 100644 --- a/roofit/roofitcore/src/RooCategory.cxx +++ b/roofit/roofitcore/src/RooCategory.cxx @@ -445,6 +445,7 @@ void RooCategory::Streamer(TBuffer &R__b) installLegacySharedProp(props); // props was allocated by I/O system, we cannot delete here in case it gets reused + R__b.CheckByteCount(R__s, R__c, RooCategory::IsA()); } else if (R__v == 2) { RooAbsCategoryLValue::Streamer(R__b); @@ -453,6 +454,7 @@ void RooCategory::Streamer(TBuffer &R__b) props->Streamer(R__b); installLegacySharedProp(props.get()); + R__b.CheckByteCount(R__s, R__c, RooCategory::IsA()); } else { // Starting at v3, ranges are shared using a shared pointer, which cannot be read by ROOT's I/O. // Instead, ranges are written as a normal pointer, and here we restore the sharing. @@ -461,8 +463,6 @@ void RooCategory::Streamer(TBuffer &R__b) _rangesPointerForIO = nullptr; } - R__b.CheckByteCount(R__s, R__c, RooCategory::IsA()); - } else { // Since we cannot write shared pointers yet, assign the shared ranges to a normal pointer, // write, and restore. diff --git a/roofit/roofitcore/src/RooDataHist.cxx b/roofit/roofitcore/src/RooDataHist.cxx index 73fd5b60e5e0b..6c0a8112c65af 100644 --- a/roofit/roofitcore/src/RooDataHist.cxx +++ b/roofit/roofitcore/src/RooDataHist.cxx @@ -2352,7 +2352,6 @@ void RooDataHist::Streamer(TBuffer &R__b) { if (R__v > 2) { R__b.ReadClassBuffer(RooDataHist::Class(),this,R__v,R__s,R__c); - R__b.CheckByteCount(R__s, R__c, RooDataHist::IsA()); initialize(nullptr, false); } else { From 4429eb6b49c8be47754d7c800418b6638951987d Mon Sep 17 00:00:00 2001 From: Philippe Canal Date: Mon, 16 Feb 2026 12:51:43 -0600 Subject: [PATCH 07/21] NFC io: tighten local variable definition --- io/io/src/TBufferFile.cxx | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/io/io/src/TBufferFile.cxx b/io/io/src/TBufferFile.cxx index ff72db6ba2dc8..96ce2d5ce3f4c 100644 --- a/io/io/src/TBufferFile.cxx +++ b/io/io/src/TBufferFile.cxx @@ -2811,11 +2811,9 @@ TClass *TBufferFile::ReadClass(const TClass *clReq, UInt_t *objTag) R__ASSERT(IsReading()); // read byte count and/or tag (older files don't have byte count) - TClass *cl; if (fBufCur < fBuffer || fBufCur > fBufMax) { fBufCur = fBufMax; - cl = (TClass*)-1; - return cl; + return (TClass*)-1; } UInt_t bcnt, tag, startpos = 0; *this >> bcnt; @@ -2838,6 +2836,7 @@ TClass *TBufferFile::ReadClass(const TClass *clReq, UInt_t *objTag) return 0; } + TClass *cl; if (tag == kNewClassTag) { // got a new class description followed by a new object From 2988699445c0ccaaa1c5ee917a712364147aab36 Mon Sep 17 00:00:00 2001 From: Philippe Canal Date: Mon, 16 Feb 2026 12:52:37 -0600 Subject: [PATCH 08/21] io: CheckByteCount need class pointer/name as argument --- io/io/src/TStreamerInfoReadBuffer.cxx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/io/io/src/TStreamerInfoReadBuffer.cxx b/io/io/src/TStreamerInfoReadBuffer.cxx index 8135f9416fc82..9bf5d4eaf9a1e 100644 --- a/io/io/src/TStreamerInfoReadBuffer.cxx +++ b/io/io/src/TStreamerInfoReadBuffer.cxx @@ -1221,7 +1221,7 @@ Int_t TStreamerInfo::ReadBuffer(TBuffer &b, const T &arr, } } } - b.CheckByteCount(start,count,aElement->GetFullName()); + b.CheckByteCount(start, count, aElement->GetClass()); continue; } if (pstreamer == 0) { From e02252df5adb98a79d5c87686d1b322bf641fec7 Mon Sep 17 00:00:00 2001 From: Philippe Canal Date: Mon, 16 Feb 2026 12:58:33 -0600 Subject: [PATCH 09/21] io: Add debugging utility for mismatch Set/CheckByteCount --- io/io/inc/TBufferFile.h | 9 +++-- io/io/src/TBufferFile.cxx | 73 ++++++++++++++++++++++++++++++++------- 2 files changed, 68 insertions(+), 14 deletions(-) diff --git a/io/io/inc/TBufferFile.h b/io/io/inc/TBufferFile.h index 143f8ac374a69..bada9600751ac 100644 --- a/io/io/inc/TBufferFile.h +++ b/io/io/inc/TBufferFile.h @@ -53,7 +53,12 @@ class TBufferFile : public TBufferIO { InfoList_t fInfoStack; ///< Stack of pointers to the TStreamerInfos using ByteCountLocator_t = std::size_t; // This might become a pair if we implement chunked keys - using ByteCountStack_t = std::vector; + struct ByteCountLocationInfo { + ByteCountLocator_t locator; ///< Position where the byte count value is stored + const TClass* cl; ///< Class for which the byte count is reserved + const TClass* alt; + }; + using ByteCountStack_t = std::vector; ByteCountStack_t fByteCountStack; ///GetName() && strcmp(passedClassName, stackClass->GetName()) == 0); + }; if (startpos == kOverflowPosition) { // The position is above 4GB but was cached using a 32 bit variable. - startpos = fByteCountStack.back(); + startpos = fByteCountStack.back().locator; + // See below + R__ASSERT((fByteCountStack.back().cl == nullptr || clss == fByteCountStack.back().cl) + && "Class on the byte count position stack does not match the passed class"); + } else { + // This assert allows to reject cases that used to be valid (missing or + // redundant call to SetByteCount) but are now an error because the + // position is passed through a stack (and redundantly via the argument + // for 'small' buffers). + const auto stackClass = fByteCountStack.back().cl; + const auto altStackClass = fByteCountStack.back().alt; + const bool classMatches = classMatcher(stackClass, clss, classname) || + classMatcher(altStackClass, clss, classname); + if (startpos != fByteCountStack.back().locator) { + const char *stackClassName = stackClass ? stackClass->GetName() : "not specified"; + const char *passedClassName = clss ? clss->GetName() : classname ? classname : "not specified"; + Fatal("CheckByteCount", + "Incorrect Streamer Function.\n\tByte count position stack mismatch: expected %zu but got %llu.\n\tClass on stack is %s, passed class is %s", + fByteCountStack.back().locator, startpos, + stackClassName ? stackClassName : "nullptr", passedClassName ? passedClassName : "nullptr"); + } else if (!classMatches) { + const char *stackClassName = stackClass ? stackClass->GetName() : nullptr; + const char *passedClassName = clss ? clss->GetName() : classname; + if (classMatcher(stackClass, clss, classname)) { + // In case the mismatch comes from the alt class. + stackClassName = altStackClass ? altStackClass->GetName() : nullptr; + } + Fatal("CheckByteCount", + "Incorrect Streamer Function.\n\tClass mismatch for byte count position %llu: expected %s but got %s", + startpos, stackClassName ? stackClassName : "nullptr", passedClassName ? passedClassName : "nullptr"); + } } if (bcnt == kOverflowCount) { // in case we are checking a byte count for which we postponed @@ -2770,7 +2812,7 @@ void TBufferFile::WriteObjectClass(const void *actualObjectStart, const TClass * } // reserve space for leading byte count - UInt_t cntpos = ReserveByteCount(); + UInt_t cntpos = ReserveByteCount(actualClass); // write class of object first Int_t mapsize = fMap->Capacity(); // The slot depends on the capacity and WriteClass might induce an increase. @@ -2825,7 +2867,8 @@ TClass *TBufferFile::ReadClass(const TClass *clReq, UInt_t *objTag) // When objTag is not used, the caller is not interested in the byte // count and will not (can not) call CheckByteCount. if (objTag) - fByteCountStack.push_back(fBufCur - fBuffer - sizeof(UInt_t)); + // Note: the actual class is set later on. + fByteCountStack.push_back({fBufCur - fBuffer - sizeof(UInt_t), clReq, nullptr}); startpos = UInt_t(fBufCur-fBuffer); *this >> tag; } @@ -2884,7 +2927,11 @@ TClass *TBufferFile::ReadClass(const TClass *clReq, UInt_t *objTag) } // return bytecount in objTag - if (objTag) *objTag = (bcnt & ~kByteCountMask); + if (objTag) { + *objTag = (bcnt & ~kByteCountMask); + if (cl) + fByteCountStack.back().alt = cl; + } // case of unknown class if (!cl) cl = (TClass*)-1; @@ -3017,7 +3064,7 @@ Version_t TBufferFile::ReadVersion(UInt_t *startpos, UInt_t *bcnt, const TClass // before reading object save start position auto full_startpos = fBufCur - fBuffer; *startpos = full_startpos <= kMaxCountPosition ? UInt_t(full_startpos) : kOverflowPosition; - fByteCountStack.push_back(full_startpos); + fByteCountStack.push_back({(size_t)full_startpos, cl, nullptr}); } // read byte count (older files don't have byte count) @@ -3156,7 +3203,8 @@ Version_t TBufferFile::ReadVersionNoCheckSum(UInt_t *startpos, UInt_t *bcnt) // before reading object save start position auto full_startpos = fBufCur - fBuffer; *startpos = full_startpos < kMaxCountPosition ? UInt_t(full_startpos) : kOverflowPosition; - fByteCountStack.push_back(full_startpos); + // TODO: Extend ReadVersionNoCheckSum to take the class pointer. + fByteCountStack.push_back({(size_t)full_startpos, nullptr, nullptr}); } // read byte count (older files don't have byte count) @@ -3281,7 +3329,7 @@ UInt_t TBufferFile::WriteVersion(const TClass *cl, Bool_t useBcnt) UInt_t cntpos = 0; if (useBcnt) { // reserve space for leading byte count - cntpos = ReserveByteCount(); + cntpos = ReserveByteCount(cl); } Version_t version = cl->GetClassVersion(); @@ -3310,7 +3358,7 @@ UInt_t TBufferFile::WriteVersionMemberWise(const TClass *cl, Bool_t useBcnt) UInt_t cntpos = 0; if (useBcnt) { // reserve space for leading byte count - cntpos = ReserveByteCount(); + cntpos = ReserveByteCount(cl); } Version_t version = cl->GetClassVersion(); @@ -3466,14 +3514,15 @@ UInt_t TBufferFile::CheckObject(UInt_t offset, const TClass *cl, Bool_t readClas /// Reserve space for a leading byte count and return the position where to /// store the byte count value. /// +/// \param[in] cl The class for which we are reserving the byte count, used for error reporting. /// \return The position (cntpos) where the byte count should be stored later, /// or kOverflowPosition if the position exceeds kMaxCountPosition -UInt_t TBufferFile::ReserveByteCount() +UInt_t TBufferFile::ReserveByteCount(const TClass *cl) { // reserve space for leading byte count - auto full_cntpos = fBufCur - fBuffer; - fByteCountStack.push_back(full_cntpos); + size_t full_cntpos = fBufCur - fBuffer; + fByteCountStack.push_back({full_cntpos, cl, nullptr}); *this << (UInt_t)kByteCountMask; // placeholder for byte count return full_cntpos <= kMaxCountPosition ? full_cntpos : kOverflowPosition; } From 59bb8ad0ea8886c3ed978847da083bf645e973c9 Mon Sep 17 00:00:00 2001 From: Philippe Canal Date: Mon, 16 Feb 2026 20:17:54 -0600 Subject: [PATCH 10/21] io/tree: Remove redundant CheckByteCount --- tree/tree/src/TBranch.cxx | 2 -- 1 file changed, 2 deletions(-) diff --git a/tree/tree/src/TBranch.cxx b/tree/tree/src/TBranch.cxx index 768434017ea86..e0cfeb01c36b2 100644 --- a/tree/tree/src/TBranch.cxx +++ b/tree/tree/src/TBranch.cxx @@ -3096,10 +3096,8 @@ void TBranch::Streamer(TBuffer& b) fBasketSeek [fWriteBasket] = fBasketSeek [fWriteBasket-1]; } - // Check Byte Count is not needed since it was done in ReadBuffer if (!fSplitLevel && fBranches.GetEntriesFast()) fSplitLevel = 1; gROOT->SetReadingObject(false); - b.CheckByteCount(R__s, R__c, TBranch::IsA()); if (IsA() == TBranch::Class()) { if (fNleaves == 0) { fReadLeaves = &TBranch::ReadLeaves0Impl; From f7dc66016eb792d52de1b0d980d767fb27663210 Mon Sep 17 00:00:00 2001 From: Philippe Canal Date: Tue, 17 Feb 2026 12:29:46 -0600 Subject: [PATCH 11/21] io: byte count stack handling in skip object --- core/base/inc/TBuffer.h | 17 +++++++++++++++++ io/io/inc/TBufferFile.h | 1 + io/io/inc/TBufferJSON.h | 1 + io/io/src/TBufferFile.cxx | 21 ++++++++++++++++++++- io/io/src/TBufferJSON.cxx | 1 + io/io/src/TStreamerInfoReadBuffer.cxx | 2 +- io/sql/inc/TBufferSQL2.h | 1 + io/sql/src/TBufferSQL2.cxx | 8 ++++++++ io/xml/inc/TBufferXML.h | 1 + io/xml/src/TBufferXML.cxx | 9 +++++++++ 10 files changed, 60 insertions(+), 2 deletions(-) diff --git a/core/base/inc/TBuffer.h b/core/base/inc/TBuffer.h index d6cae935560ed..71c2a214dbd1e 100644 --- a/core/base/inc/TBuffer.h +++ b/core/base/inc/TBuffer.h @@ -135,6 +135,23 @@ class TBuffer : public TObject { virtual void *ReadObjectAny(const TClass* cast) = 0; virtual void SkipObjectAny() = 0; + /** \brief Skip, based on a known start position and byte count, to the end of the current object. + * + * \warning Advanced use only. + * + * This overload exists primarily for error handling within a Streamer. + * + * A typical use case is a Streamer with a flow like: + * - Read version and bytecount + * - Start reading the data + * - Detect an error (e.g. missing some information for a CollectionProxy) + * - Properly set the cursor to the end of the object to allow reading to continue. + * + * Because the actual byte count information for \e large byte counts is kept only on the + * internal byte-count stack, there are only two viable options to support this use case: + * provide this overload, or make the stack accessible (e.g. via a getter). + */ + virtual void SkipObjectAny(Long64_t start, UInt_t bytecount) = 0; virtual void TagStreamerInfo(TVirtualStreamerInfo* info) = 0; virtual void IncrementLevel(TVirtualStreamerInfo* info) = 0; diff --git a/io/io/inc/TBufferFile.h b/io/io/inc/TBufferFile.h index bada9600751ac..e6f69c726793c 100644 --- a/io/io/inc/TBufferFile.h +++ b/io/io/inc/TBufferFile.h @@ -102,6 +102,7 @@ class TBufferFile : public TBufferIO { void *ReadObjectAny(const TClass* cast) override; void SkipObjectAny() override; + void SkipObjectAny(Long64_t start, UInt_t bytecount) override; void IncrementLevel(TVirtualStreamerInfo* info) override; void SetStreamerElementNumber(TStreamerElement*,Int_t) override {} diff --git a/io/io/inc/TBufferJSON.h b/io/io/inc/TBufferJSON.h index 8ab2d521e6a1d..6f7a8ca944c84 100644 --- a/io/io/inc/TBufferJSON.h +++ b/io/io/inc/TBufferJSON.h @@ -109,6 +109,7 @@ class TBufferJSON final : public TBufferText { void *ReadObjectAny(const TClass *clCast) final; void SkipObjectAny() final; + void SkipObjectAny(Long64_t start, UInt_t bytecount) final; // these methods used in streamer info to indicate currently streamed element, void IncrementLevel(TVirtualStreamerInfo *) final; diff --git a/io/io/src/TBufferFile.cxx b/io/io/src/TBufferFile.cxx index 231121dcdb42c..994a01c73a543 100644 --- a/io/io/src/TBufferFile.cxx +++ b/io/io/src/TBufferFile.cxx @@ -2612,7 +2612,26 @@ void TBufferFile::SkipObjectAny() { UInt_t start, count; ReadVersion(&start, &count); - SetBufferOffset(start+count+sizeof(UInt_t)); + if (count == kOverflowPosition) + SetBufferOffset(start+ fByteCountStack.back().locator + sizeof(UInt_t)); + else + SetBufferOffset(start+count+sizeof(UInt_t)); + // The byte count location is pushed on the stack only if there is + // actually a byte count. + if (count) + fByteCountStack.pop_back(); +} + +//////////////////////////////////////////////////////////////////////////////// +/// Skip any kind of object from buffer + +void TBufferFile::SkipObjectAny(Long64_t start, UInt_t count) +{ + if (count == kOverflowPosition) + SetBufferOffset(start + fByteCountStack.back().locator + sizeof(UInt_t)); + else + SetBufferOffset(start + count + sizeof(UInt_t)); + fByteCountStack.pop_back(); } //////////////////////////////////////////////////////////////////////////////// diff --git a/io/io/src/TBufferJSON.cxx b/io/io/src/TBufferJSON.cxx index de880d809b583..63dc652cee39e 100644 --- a/io/io/src/TBufferJSON.cxx +++ b/io/io/src/TBufferJSON.cxx @@ -2589,6 +2589,7 @@ void *TBufferJSON::ReadObjectAny(const TClass *expectedClass) /// Skip any kind of object from buffer void TBufferJSON::SkipObjectAny() {} +void TBufferJSON::SkipObjectAny(Long64_t, UInt_t) {} //////////////////////////////////////////////////////////////////////////////// /// Write object to buffer. Only used from TBuffer diff --git a/io/io/src/TStreamerInfoReadBuffer.cxx b/io/io/src/TStreamerInfoReadBuffer.cxx index 9bf5d4eaf9a1e..611abf936bacb 100644 --- a/io/io/src/TStreamerInfoReadBuffer.cxx +++ b/io/io/src/TStreamerInfoReadBuffer.cxx @@ -1279,7 +1279,7 @@ Int_t TStreamerInfo::ReadBuffer(TBuffer &b, const T &arr, // and the collection is always empty, // So let's skip the rest (which requires the StreamerInfo of the valueClass ... which we do not have) - b.SetBufferOffset(start+count+sizeof(UInt_t)); + b.SkipObjectAny(start, count); continue; } diff --git a/io/sql/inc/TBufferSQL2.h b/io/sql/inc/TBufferSQL2.h index 1c7c7e696adae..1217c4f370c57 100644 --- a/io/sql/inc/TBufferSQL2.h +++ b/io/sql/inc/TBufferSQL2.h @@ -149,6 +149,7 @@ class TBufferSQL2 final : public TBufferText { void *ReadObjectAny(const TClass *clCast) final; void SkipObjectAny() final; + void SkipObjectAny(Long64_t start, UInt_t bytecount) final; void IncrementLevel(TVirtualStreamerInfo *) final; void SetStreamerElementNumber(TStreamerElement *elem, Int_t comp_type) final; diff --git a/io/sql/src/TBufferSQL2.cxx b/io/sql/src/TBufferSQL2.cxx index eae8309d44560..c0d5d59dea4cc 100644 --- a/io/sql/src/TBufferSQL2.cxx +++ b/io/sql/src/TBufferSQL2.cxx @@ -862,6 +862,14 @@ void TBufferSQL2::SkipObjectAny() { } +//////////////////////////////////////////////////////////////////////////////// +/// Skip any kind of object from buffer +/// Actually skip only one node on current level of xml structure + +void TBufferSQL2::SkipObjectAny(Long64_t, UInt_t) +{ +} + //////////////////////////////////////////////////////////////////////////////// /// Write object to buffer. Only used from TBuffer diff --git a/io/xml/inc/TBufferXML.h b/io/xml/inc/TBufferXML.h index 4210e1eb159dc..319b0aa2e454e 100644 --- a/io/xml/inc/TBufferXML.h +++ b/io/xml/inc/TBufferXML.h @@ -77,6 +77,7 @@ class TBufferXML final : public TBufferText, public TXMLSetup { void *ReadObjectAny(const TClass *clCast) final; void SkipObjectAny() final; + void SkipObjectAny(Long64_t start, UInt_t bytecount) final; void IncrementLevel(TVirtualStreamerInfo *) final; void SetStreamerElementNumber(TStreamerElement *elem, Int_t comp_type) final; diff --git a/io/xml/src/TBufferXML.cxx b/io/xml/src/TBufferXML.cxx index 6aafce756b643..fdfa96d3a2bcd 100644 --- a/io/xml/src/TBufferXML.cxx +++ b/io/xml/src/TBufferXML.cxx @@ -1510,6 +1510,15 @@ void TBufferXML::SkipObjectAny() ShiftStack("skipobjectany"); } +//////////////////////////////////////////////////////////////////////////////// +/// Skip any kind of object from buffer +/// Actually skip only one node on current level of xml structure + +void TBufferXML::SkipObjectAny(Long64_t, UInt_t) +{ + ShiftStack("skipobjectany"); +} + //////////////////////////////////////////////////////////////////////////////// /// Write object to buffer. Only used from TBuffer From 95fa20f9172c4f3a7b9d4da3a7e85d8ed03cedaf Mon Sep 17 00:00:00 2001 From: Philippe Canal Date: Tue, 17 Feb 2026 17:22:48 -0600 Subject: [PATCH 12/21] io: Add TBuffer::ByteCountWriter RAII. This simplify the implementation of calling `ReserveByteCount` and then `SetByteCount` and could also simplify removing the use of the ByteCountStack in some/most cases. Note: `ReserveByteCount` is protected and the existing code that does the same semantic action was only recording the position in a 32 bits integer. That code needs to be updated by either making `ReserverByteCount` public (making removal of external use of the ByteCountStack more difficult) or by introducing a RAII that hides the size of the integer. --- core/base/inc/TBuffer.h | 24 ++++++++++++++++++++++++ io/io/inc/TBufferFile.h | 2 +- io/io/inc/TBufferText.h | 2 ++ 3 files changed, 27 insertions(+), 1 deletion(-) diff --git a/core/base/inc/TBuffer.h b/core/base/inc/TBuffer.h index 71c2a214dbd1e..fb0e369fca388 100644 --- a/core/base/inc/TBuffer.h +++ b/core/base/inc/TBuffer.h @@ -69,6 +69,8 @@ class TBuffer : public TObject { Int_t Write(const char *name, Int_t opt, Long64_t bufsize) const override { return TObject::Write(name, opt, bufsize); } + virtual UInt_t ReserveByteCount(const TClass *) = 0; + public: enum EMode { kRead = 0, kWrite = 1 }; enum EStatusBits { @@ -125,6 +127,28 @@ class TBuffer : public TObject { virtual Long64_t CheckByteCount(ULong64_t startpos, ULong64_t bcnt, const TClass *clss) = 0; virtual Long64_t CheckByteCount(ULong64_t startpos, ULong64_t bcnt, const char *classname) = 0; virtual void SetByteCount(ULong64_t cntpos, Bool_t packInVersion = kFALSE)= 0; + class ByteCountWriter { + TBuffer &fBuffer; + Bool_t fPackInVersion; + ULong64_t fCntPos; + public: + ByteCountWriter() = delete; + ByteCountWriter(const ByteCountWriter&) = delete; + ByteCountWriter& operator=(const ByteCountWriter&) = delete; + ByteCountWriter(ByteCountWriter&&) = delete; + ByteCountWriter& operator=(ByteCountWriter&&) = delete; + + ByteCountWriter(TBuffer &buf, const TClass *cl, Bool_t packInVersion = kFALSE) : fBuffer(buf), fPackInVersion(packInVersion) { + // We could split ReserveByteCount in a 32bit version that uses + // the ByteCountStack and another version that always returns the + // long range position. For now keep it 'simpler' by always using + // the stack. + fCntPos = fBuffer.ReserveByteCount(cl); + } + ~ByteCountWriter() { + fBuffer.SetByteCount(fCntPos, fPackInVersion); + } + }; virtual void SkipVersion(const TClass *cl = nullptr) = 0; virtual Version_t ReadVersion(UInt_t *start = nullptr, UInt_t *bcnt = nullptr, const TClass *cl = nullptr) = 0; diff --git a/io/io/inc/TBufferFile.h b/io/io/inc/TBufferFile.h index e6f69c726793c..2b7d221ea2a77 100644 --- a/io/io/inc/TBufferFile.h +++ b/io/io/inc/TBufferFile.h @@ -77,7 +77,7 @@ class TBufferFile : public TBufferIO { Long64_t CheckByteCount(ULong64_t startpos, ULong64_t bcnt, const TClass *clss, const char* classname); void CheckCount(UInt_t offset) override; UInt_t CheckObject(UInt_t offset, const TClass *cl, Bool_t readClass = kFALSE); - UInt_t ReserveByteCount(const TClass *cl); + UInt_t ReserveByteCount(const TClass *cl) override; void WriteObjectClass(const void *actualObjStart, const TClass *actualClass, Bool_t cacheReuse) override; diff --git a/io/io/inc/TBufferText.h b/io/io/inc/TBufferText.h index a5cbffc99deb0..aa177cbca4bde 100644 --- a/io/io/inc/TBufferText.h +++ b/io/io/inc/TBufferText.h @@ -23,6 +23,8 @@ class TBufferText : public TBufferIO { TBufferText(); TBufferText(TBuffer::EMode mode, TObject *parent = nullptr); + UInt_t ReserveByteCount(const TClass *) override { return 0; } + public: ~TBufferText() override; From 7ba9f3dbafb51e7a4d02511215d61fb893026c09 Mon Sep 17 00:00:00 2001 From: Philippe Canal Date: Tue, 17 Feb 2026 17:26:41 -0600 Subject: [PATCH 13/21] io: Upgrade TArray::WriteArray to support long range byte count --- core/cont/src/TArray.cxx | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/core/cont/src/TArray.cxx b/core/cont/src/TArray.cxx index a5283e5523d53..6589c88c510c1 100644 --- a/core/cont/src/TArray.cxx +++ b/core/cont/src/TArray.cxx @@ -90,18 +90,16 @@ void TArray::WriteArray(TBuffer &b, const TArray *a) b << (UInt_t) 0; } else { + TClass *cl = a->IsA(); // Reserve space for leading byte count - UInt_t cntpos = UInt_t(b.Length()); - b.SetBufferOffset(Int_t(cntpos+sizeof(UInt_t))); + TBuffer::ByteCountWriter bcnt(b, cl); - TClass *cl = a->IsA(); b.WriteClass(cl); - ((TArray *)a)->Streamer(b); + const_cast(a)->Streamer(b); - // Write byte count - b.SetByteCount(cntpos); + // The byte count is written automatically by the ByteCountWriter destructor } } From ec0b3d62026113895979ff5d5e0e4e19887bff27 Mon Sep 17 00:00:00 2001 From: Philippe Canal Date: Wed, 18 Feb 2026 10:39:28 -0600 Subject: [PATCH 14/21] roottest: Fix cmake macro used outside of roottest dir. In order to allow using the roottest CMake macro outside of the roottest sub-directory, we need to make the variable used by those macros 'cache variable' so that they can use elsewhere. This is a hack and works only after the second CMake configuration/invocation. --- cmake/modules/RootMacros.cmake | 4 ++++ roottest/CMakeLists.txt | 16 ++++++++-------- 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/cmake/modules/RootMacros.cmake b/cmake/modules/RootMacros.cmake index f7f5183fe45c5..4ed41de4f8e44 100644 --- a/cmake/modules/RootMacros.cmake +++ b/cmake/modules/RootMacros.cmake @@ -45,6 +45,10 @@ else() set(ROOT_LIBRARY_PROPERTIES ${ROOT_LIBRARY_PROPERTIES} ${ROOT_LIBRARY_PROPERTIES_NO_VERSION} ) endif() +if (NOT DEFINED ROOT_root_CMD) + set(ROOT_root_CMD $) +endif() + include(CMakeParseArguments) #--------------------------------------------------------------------------------------------------- diff --git a/roottest/CMakeLists.txt b/roottest/CMakeLists.txt index 7b290c389767d..427bb0e311fc7 100644 --- a/roottest/CMakeLists.txt +++ b/roottest/CMakeLists.txt @@ -29,10 +29,10 @@ if(MSVC) execute_process(COMMAND ${ROOT_CONFIG_EXECUTABLE} "--tutdir" OUTPUT_VARIABLE ROOT_TUTORIALS_DIR OUTPUT_STRIP_TRAILING_WHITESPACE) cmake_path(CONVERT "${ROOT_TUTORIALS_DIR}" TO_CMAKE_PATH_LIST ROOT_TUTORIALS_DIR) set(ROOT_root_CMD ${ROOTSYS}/bin/root.exe) - set(ROOT_hadd_CMD ${ROOTSYS}/bin/hadd.exe) - set(ROOT_genreflex_CMD ${ROOTSYS}/bin/genreflex.exe) - set(ROOT_rootcint_CMD ${ROOTSYS}/bin/rootcint.exe) - set(ROOT_rootcling_CMD ${ROOTSYS}/bin/rootcling.exe) + set(ROOT_hadd_CMD ${ROOTSYS}/bin/hadd.exe CACHE INTERNAL "ROOTTEST variable for hadd executable") + set(ROOT_genreflex_CMD ${ROOTSYS}/bin/genreflex.exe CACHE INTERNAL "ROOTTEST variable for genreflex executable") + set(ROOT_rootcint_CMD ${ROOTSYS}/bin/rootcint.exe CACHE INTERNAL "ROOTTEST variable for rootcint executable") + set(ROOT_rootcling_CMD ${ROOTSYS}/bin/rootcling.exe CACHE INTERNAL "ROOTTEST variable for rootcling executable") if(CMAKE_GENERATOR MATCHES Ninja) set(ROOT_LIBRARIES Core RIO Net Hist Gpad Graf Tree Rint Matrix MathCore) else() @@ -59,10 +59,10 @@ else() execute_process(COMMAND ${ROOT_CONFIG_EXECUTABLE} "--tutdir" OUTPUT_VARIABLE ROOT_TUTORIALS_DIR OUTPUT_STRIP_TRAILING_WHITESPACE) set(ROOT_LIBRARIES Core RIO Net Hist Gpad Tree Rint Matrix MathCore) set(ROOT_root_CMD ${ROOTSYS}/bin/root.exe) - set(ROOT_hadd_CMD ${ROOTSYS}/bin/hadd) - set(ROOT_genreflex_CMD ${ROOTSYS}/bin/genreflex) - set(ROOT_rootcint_CMD ${ROOTSYS}/bin/rootcint) - set(ROOT_rootcling_CMD rootcling) + set(ROOT_hadd_CMD ${ROOTSYS}/bin/hadd CACHE INTERNAL "ROOTTEST variable for hadd executable") + set(ROOT_genreflex_CMD ${ROOTSYS}/bin/genreflex CACHE INTERNAL "ROOTTEST variable for genreflex executable") + set(ROOT_rootcint_CMD ${ROOTSYS}/bin/rootcint CACHE INTERNAL "ROOTTEST variable for rootcint executable") + set(ROOT_rootcling_CMD rootcling CACHE INTERNAL "ROOTTEST variable for rootcling executable") endif() get_filename_component(ROOT_LIBRARY_DIR "${ROOTSYS}/lib" ABSOLUTE) From b468f460b3313ea5bf4a0b59b12308366a039c04 Mon Sep 17 00:00:00 2001 From: Philippe Canal Date: Fri, 20 Feb 2026 16:05:05 -0600 Subject: [PATCH 15/21] io-test: disable large byte count test on 32bits platforms --- io/io/test/CMakeLists.txt | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/io/io/test/CMakeLists.txt b/io/io/test/CMakeLists.txt index 61f381acf0274..cd5103b6e5fc5 100644 --- a/io/io/test/CMakeLists.txt +++ b/io/io/test/CMakeLists.txt @@ -41,10 +41,21 @@ endif() # ROOT_ADD_TEST(io-io-test-TMapFileTest COMMAND TMapFileTest complete) #endif() -ROOTTEST_COMPILE_MACRO(testByteCount.cxx - FIXTURES_SETUP io-io-tobj-fixture) +# Detect bitness. +# FIXME: This is also done in roottest/CMakeLists.txt which is included 'too late' for +# the tests here. We should unify this logic and do it only once. +if(CMAKE_SIZEOF_VOID_P EQUAL 8) + set(_64BIT 1) +elseif(CMAKE_SIZEOF_VOID_P EQUAL 4) + set(_32BIT 1) +endif() + +if(NOT _32BIT) + ROOTTEST_COMPILE_MACRO(testByteCount.cxx + FIXTURES_SETUP io-io-tobj-fixture) -ROOTTEST_ADD_TEST(testLargeByteCounts - MACRO testByteCount.cxx+ - OUTREF testByteCount.ref - FIXTURES_REQUIRED io-io-tobj-fixture) + ROOTTEST_ADD_TEST(testLargeByteCounts + MACRO testByteCount.cxx+ + OUTREF testByteCount.ref + FIXTURES_REQUIRED io-io-tobj-fixture) +endif() \ No newline at end of file From a5e2c2a7d7df7f8f9e2398f81572fcc4408aba16 Mon Sep 17 00:00:00 2001 From: Philippe Canal Date: Fri, 20 Feb 2026 15:40:55 -0600 Subject: [PATCH 16/21] cmake: Include VStudio in the `Ninja` `RESOURCE_LOCK`. When using the Microsoft Visual Studio generator we have the exact same problem, that it might trigger a rebuild of ROOT so we need to apply the same solution. See 06e00a270e6e4f1157bfc344ad0dcdb6c45bb01a. PS. We might want to eventually rename the resource lock. --- cmake/modules/RootCTest.cmake | 7 ++++--- cmake/modules/RootMacros.cmake | 6 +++--- roottest/cling/dict/ROOT-8096/CMakeLists.txt | 2 +- roottest/cling/stl/dicts/CMakeLists.txt | 2 +- roottest/root/io/rootcint/sigbug/CMakeLists.txt | 2 +- roottest/root/io/tmpifile/CMakeLists.txt | 2 +- roottest/root/io/transient/base/CMakeLists.txt | 2 +- roottest/root/meta/genreflex/ROOT-5768/CMakeLists.txt | 2 +- 8 files changed, 13 insertions(+), 12 deletions(-) diff --git a/cmake/modules/RootCTest.cmake b/cmake/modules/RootCTest.cmake index e434770ce0729..aaa92587165a6 100644 --- a/cmake/modules/RootCTest.cmake +++ b/cmake/modules/RootCTest.cmake @@ -51,13 +51,14 @@ foreach(d ${test_list}) endif() endforeach() -# When ninja is in use, tests that compile an executable might try to rebuild the entire build tree. -# If multiple of these are invoked in parallel, ninja will suffer from race conditions. +# When ninja or the Microsoft generator are in use, tests that compile an executable might try +# to rebuild the entire build tree. If multiple of these are invoked in parallel, ninja will +# suffer from race conditions. # To solve this, do the following: # - Add a test that updates the build tree (equivalent to "ninja all"). This one will run in complete isolation. # - Make all tests that require a ninja build depend on the above test. # - Use a RESOURCE_LOCK on all tests that invoke ninja, so no two tests will invoke ninja in parallel -if(CMAKE_GENERATOR MATCHES Ninja) +if(CMAKE_GENERATOR MATCHES Ninja OR CMAKE_GENERATOR MATCHES "Visual Studio") add_test(NAME ninja-build-all COMMAND ${CMAKE_COMMAND} --build ${CMAKE_BINARY_DIR}) set_tests_properties(ninja-build-all PROPERTIES diff --git a/cmake/modules/RootMacros.cmake b/cmake/modules/RootMacros.cmake index 4ed41de4f8e44..47db6ce65cbd8 100644 --- a/cmake/modules/RootMacros.cmake +++ b/cmake/modules/RootMacros.cmake @@ -2661,7 +2661,7 @@ macro(ROOTTEST_GENERATE_DICTIONARY dictname) -- ${always-make}) set_property(TEST ${GENERATE_DICTIONARY_TEST} PROPERTY ENVIRONMENT ${ROOTTEST_ENVIRONMENT}) - if(CMAKE_GENERATOR MATCHES Ninja) + if(CMAKE_GENERATOR MATCHES Ninja OR CMAKE_GENERATOR MATCHES "Visual Studio") set_property(TEST ${GENERATE_DICTIONARY_TEST} APPEND PROPERTY RESOURCE_LOCK NINJA_BUILD) set_property(TEST ${GENERATE_DICTIONARY_TEST} APPEND PROPERTY FIXTURES_REQUIRED NINJA_BUILD_ALL) endif() @@ -2769,7 +2769,7 @@ macro(ROOTTEST_GENERATE_REFLEX_DICTIONARY dictionary) -- ${always-make}) set_property(TEST ${GENERATE_REFLEX_TEST} PROPERTY ENVIRONMENT ${ROOTTEST_ENVIRONMENT}) - if(CMAKE_GENERATOR MATCHES Ninja) + if(CMAKE_GENERATOR MATCHES Ninja OR CMAKE_GENERATOR MATCHES "Visual Studio") set_property(TEST ${GENERATE_REFLEX_TEST} APPEND PROPERTY RESOURCE_LOCK NINJA_BUILD) set_property(TEST ${GENERATE_REFLEX_TEST} APPEND PROPERTY FIXTURES_REQUIRED NINJA_BUILD_ALL) endif() @@ -2881,7 +2881,7 @@ macro(ROOTTEST_GENERATE_EXECUTABLE executable) RESOURCE_LOCK ${ARG_RESOURCE_LOCK}) endif() - if(CMAKE_GENERATOR MATCHES Ninja) + if(CMAKE_GENERATOR MATCHES Ninja OR CMAKE_GENERATOR MATCHES "Visual Studio") set_property(TEST ${GENERATE_EXECUTABLE_TEST} APPEND PROPERTY RESOURCE_LOCK NINJA_BUILD) set_property(TEST ${GENERATE_EXECUTABLE_TEST} APPEND PROPERTY FIXTURES_REQUIRED NINJA_BUILD_ALL) endif() diff --git a/roottest/cling/dict/ROOT-8096/CMakeLists.txt b/roottest/cling/dict/ROOT-8096/CMakeLists.txt index 07c916ce01b19..b88a0b9ff1a66 100644 --- a/roottest/cling/dict/ROOT-8096/CMakeLists.txt +++ b/roottest/cling/dict/ROOT-8096/CMakeLists.txt @@ -20,7 +20,7 @@ add_test(NAME roottest-cling-dict-ROOT-8096-build -- ${always-make}) # --target ${targetname_libgen}${fast}) set_property(TEST roottest-cling-dict-ROOT-8096-build PROPERTY ENVIRONMENT ${ROOTTEST_ENVIRONMENT}) -if(CMAKE_GENERATOR MATCHES Ninja) +if(CMAKE_GENERATOR MATCHES Ninja OR CMAKE_GENERATOR MATCHES "Visual Studio") set_property(TEST roottest-cling-dict-ROOT-8096-build APPEND PROPERTY RESOURCE_LOCK NINJA_BUILD) set_property(TEST roottest-cling-dict-ROOT-8096-build APPEND PROPERTY FIXTURES_REQUIRED NINJA_BUILD_ALL) endif() diff --git a/roottest/cling/stl/dicts/CMakeLists.txt b/roottest/cling/stl/dicts/CMakeLists.txt index c31d0a3ee25b3..162e6111ebcbe 100644 --- a/roottest/cling/stl/dicts/CMakeLists.txt +++ b/roottest/cling/stl/dicts/CMakeLists.txt @@ -11,7 +11,7 @@ ROOTTEST_LINKER_LIBRARY(stldictTest TEST MyClass1.cpp MyClass2.cpp MyClass3.cpp # of targets. Doing so right now would build the dictionaries twice. ROOT_ADD_TEST(roottest-cling-stl-dicts-build COMMAND ${CMAKE_COMMAND} --build ${CMAKE_BINARY_DIR} ${build_config} --target stldictTest${fast} -- ${always-make}) -if(CMAKE_GENERATOR MATCHES Ninja) +if(CMAKE_GENERATOR MATCHES Ninja OR CMAKE_GENERATOR MATCHES "Visual Studio") set_property(TEST roottest-cling-stl-dicts-build APPEND PROPERTY RESOURCE_LOCK NINJA_BUILD) set_property(TEST roottest-cling-stl-dicts-build APPEND PROPERTY FIXTURES_REQUIRED NINJA_BUILD_ALL) endif() diff --git a/roottest/root/io/rootcint/sigbug/CMakeLists.txt b/roottest/root/io/rootcint/sigbug/CMakeLists.txt index a87613d25dbe6..7277dce32c311 100644 --- a/roottest/root/io/rootcint/sigbug/CMakeLists.txt +++ b/roottest/root/io/rootcint/sigbug/CMakeLists.txt @@ -19,7 +19,7 @@ add_test(NAME ${GENERATE_DICTIONARY_TEST} ${build_config} --target ${dictname}${fast} -- ${always-make}) -if(CMAKE_GENERATOR MATCHES Ninja) +if(CMAKE_GENERATOR MATCHES Ninja OR CMAKE_GENERATOR MATCHES "Visual Studio") set_property(TEST ${GENERATE_DICTIONARY_TEST} APPEND PROPERTY RESOURCE_LOCK NINJA_BUILD) set_property(TEST ${GENERATE_DICTIONARY_TEST} APPEND PROPERTY FIXTURES_REQUIRED NINJA_BUILD_ALL) endif() diff --git a/roottest/root/io/tmpifile/CMakeLists.txt b/roottest/root/io/tmpifile/CMakeLists.txt index 840f475bcba53..fc94946fa7f64 100644 --- a/roottest/root/io/tmpifile/CMakeLists.txt +++ b/roottest/root/io/tmpifile/CMakeLists.txt @@ -13,7 +13,7 @@ ROOTTEST_ADD_TEST(split-fail ROOTTEST_ADD_TEST(libjetevent-build COMMAND ${CMAKE_COMMAND} --build ${CMAKE_BINARY_DIR} ${build_config} --target JetEvent${fast} -- ${always-make}) -if(CMAKE_GENERATOR MATCHES Ninja) +if(CMAKE_GENERATOR MATCHES Ninja OR CMAKE_GENERATOR MATCHES "Visual Studio") set_property(TEST roottest-root-io-tmpifile-libjetevent-build APPEND PROPERTY RESOURCE_LOCK NINJA_BUILD) set_property(TEST roottest-root-io-tmpifile-libjetevent-build APPEND PROPERTY FIXTURES_REQUIRED NINJA_BUILD_ALL) endif() diff --git a/roottest/root/io/transient/base/CMakeLists.txt b/roottest/root/io/transient/base/CMakeLists.txt index c59bacde1dfa2..5d4b65b5cecea 100644 --- a/roottest/root/io/transient/base/CMakeLists.txt +++ b/roottest/root/io/transient/base/CMakeLists.txt @@ -10,7 +10,7 @@ add_test(NAME roottest-root-io-transient-base-build --target base${fast} -- ${always-make}) set_property(TEST roottest-root-io-transient-base-build PROPERTY FIXTURES_SETUP root-io-transient-base-build) -if(CMAKE_GENERATOR MATCHES Ninja) +if(CMAKE_GENERATOR MATCHES Ninja OR CMAKE_GENERATOR MATCHES "Visual Studio") set_property(TEST roottest-root-io-transient-base-build APPEND PROPERTY RESOURCE_LOCK NINJA_BUILD) set_property(TEST roottest-root-io-transient-base-build APPEND PROPERTY FIXTURES_REQUIRED NINJA_BUILD_ALL) endif() diff --git a/roottest/root/meta/genreflex/ROOT-5768/CMakeLists.txt b/roottest/root/meta/genreflex/ROOT-5768/CMakeLists.txt index 18ffb14556114..0bf392ff26a8d 100644 --- a/roottest/root/meta/genreflex/ROOT-5768/CMakeLists.txt +++ b/roottest/root/meta/genreflex/ROOT-5768/CMakeLists.txt @@ -27,7 +27,7 @@ if(NOT MSVC OR win_broken_tests) ROOTTEST_ADD_TEST(PyCoolLib-build COMMAND ${CMAKE_COMMAND} --build ${CMAKE_BINARY_DIR} ${build_config} --target PyCoolLib${fast} -- ${always-make} FIXTURES_REQUIRED PyCool-reflex-dict) - if(CMAKE_GENERATOR MATCHES Ninja) + if(CMAKE_GENERATOR MATCHES Ninja OR CMAKE_GENERATOR MATCHES "Visual Studio") set_property(TEST roottest-root-meta-genreflex-ROOT-5768-PyCoolLib-build APPEND PROPERTY RESOURCE_LOCK NINJA_BUILD) set_property(TEST roottest-root-meta-genreflex-ROOT-5768-PyCoolLib-build APPEND PROPERTY FIXTURES_REQUIRED NINJA_BUILD_ALL) endif() From 98fce553ffb5a9d5340b30862119633630f8dd8a Mon Sep 17 00:00:00 2001 From: Philippe Canal Date: Mon, 23 Feb 2026 10:53:04 -0600 Subject: [PATCH 17/21] cmake: Use single variable to select Ninja Build resource lock --- cmake/modules/RootCTest.cmake | 3 ++- cmake/modules/RootMacros.cmake | 6 +++--- roottest/cling/dict/ROOT-8096/CMakeLists.txt | 2 +- roottest/cling/stl/dicts/CMakeLists.txt | 2 +- roottest/root/io/rootcint/sigbug/CMakeLists.txt | 2 +- roottest/root/io/tmpifile/CMakeLists.txt | 2 +- roottest/root/io/transient/base/CMakeLists.txt | 2 +- roottest/root/meta/genreflex/ROOT-5768/CMakeLists.txt | 2 +- 8 files changed, 11 insertions(+), 10 deletions(-) diff --git a/cmake/modules/RootCTest.cmake b/cmake/modules/RootCTest.cmake index aaa92587165a6..7904c708f5e3a 100644 --- a/cmake/modules/RootCTest.cmake +++ b/cmake/modules/RootCTest.cmake @@ -58,11 +58,12 @@ endforeach() # - Add a test that updates the build tree (equivalent to "ninja all"). This one will run in complete isolation. # - Make all tests that require a ninja build depend on the above test. # - Use a RESOURCE_LOCK on all tests that invoke ninja, so no two tests will invoke ninja in parallel -if(CMAKE_GENERATOR MATCHES Ninja OR CMAKE_GENERATOR MATCHES "Visual Studio") +if(GeneratorNeedsBuildSerialization) add_test(NAME ninja-build-all COMMAND ${CMAKE_COMMAND} --build ${CMAKE_BINARY_DIR}) set_tests_properties(ninja-build-all PROPERTIES RESOURCE_LOCK NINJA_BUILD FIXTURES_SETUP NINJA_BUILD_ALL RUN_SERIAL True) + set(GeneratorNeedsBuildSerialization True) endif() diff --git a/cmake/modules/RootMacros.cmake b/cmake/modules/RootMacros.cmake index 47db6ce65cbd8..2576752fd0e7a 100644 --- a/cmake/modules/RootMacros.cmake +++ b/cmake/modules/RootMacros.cmake @@ -2661,7 +2661,7 @@ macro(ROOTTEST_GENERATE_DICTIONARY dictname) -- ${always-make}) set_property(TEST ${GENERATE_DICTIONARY_TEST} PROPERTY ENVIRONMENT ${ROOTTEST_ENVIRONMENT}) - if(CMAKE_GENERATOR MATCHES Ninja OR CMAKE_GENERATOR MATCHES "Visual Studio") + if(GeneratorNeedsBuildSerialization) set_property(TEST ${GENERATE_DICTIONARY_TEST} APPEND PROPERTY RESOURCE_LOCK NINJA_BUILD) set_property(TEST ${GENERATE_DICTIONARY_TEST} APPEND PROPERTY FIXTURES_REQUIRED NINJA_BUILD_ALL) endif() @@ -2769,7 +2769,7 @@ macro(ROOTTEST_GENERATE_REFLEX_DICTIONARY dictionary) -- ${always-make}) set_property(TEST ${GENERATE_REFLEX_TEST} PROPERTY ENVIRONMENT ${ROOTTEST_ENVIRONMENT}) - if(CMAKE_GENERATOR MATCHES Ninja OR CMAKE_GENERATOR MATCHES "Visual Studio") + if(GeneratorNeedsBuildSerialization) set_property(TEST ${GENERATE_REFLEX_TEST} APPEND PROPERTY RESOURCE_LOCK NINJA_BUILD) set_property(TEST ${GENERATE_REFLEX_TEST} APPEND PROPERTY FIXTURES_REQUIRED NINJA_BUILD_ALL) endif() @@ -2881,7 +2881,7 @@ macro(ROOTTEST_GENERATE_EXECUTABLE executable) RESOURCE_LOCK ${ARG_RESOURCE_LOCK}) endif() - if(CMAKE_GENERATOR MATCHES Ninja OR CMAKE_GENERATOR MATCHES "Visual Studio") + if(GeneratorNeedsBuildSerialization) set_property(TEST ${GENERATE_EXECUTABLE_TEST} APPEND PROPERTY RESOURCE_LOCK NINJA_BUILD) set_property(TEST ${GENERATE_EXECUTABLE_TEST} APPEND PROPERTY FIXTURES_REQUIRED NINJA_BUILD_ALL) endif() diff --git a/roottest/cling/dict/ROOT-8096/CMakeLists.txt b/roottest/cling/dict/ROOT-8096/CMakeLists.txt index b88a0b9ff1a66..4614dba98fa9b 100644 --- a/roottest/cling/dict/ROOT-8096/CMakeLists.txt +++ b/roottest/cling/dict/ROOT-8096/CMakeLists.txt @@ -20,7 +20,7 @@ add_test(NAME roottest-cling-dict-ROOT-8096-build -- ${always-make}) # --target ${targetname_libgen}${fast}) set_property(TEST roottest-cling-dict-ROOT-8096-build PROPERTY ENVIRONMENT ${ROOTTEST_ENVIRONMENT}) -if(CMAKE_GENERATOR MATCHES Ninja OR CMAKE_GENERATOR MATCHES "Visual Studio") +if(GeneratorNeedsBuildSerialization) set_property(TEST roottest-cling-dict-ROOT-8096-build APPEND PROPERTY RESOURCE_LOCK NINJA_BUILD) set_property(TEST roottest-cling-dict-ROOT-8096-build APPEND PROPERTY FIXTURES_REQUIRED NINJA_BUILD_ALL) endif() diff --git a/roottest/cling/stl/dicts/CMakeLists.txt b/roottest/cling/stl/dicts/CMakeLists.txt index 162e6111ebcbe..874281c54f2cb 100644 --- a/roottest/cling/stl/dicts/CMakeLists.txt +++ b/roottest/cling/stl/dicts/CMakeLists.txt @@ -11,7 +11,7 @@ ROOTTEST_LINKER_LIBRARY(stldictTest TEST MyClass1.cpp MyClass2.cpp MyClass3.cpp # of targets. Doing so right now would build the dictionaries twice. ROOT_ADD_TEST(roottest-cling-stl-dicts-build COMMAND ${CMAKE_COMMAND} --build ${CMAKE_BINARY_DIR} ${build_config} --target stldictTest${fast} -- ${always-make}) -if(CMAKE_GENERATOR MATCHES Ninja OR CMAKE_GENERATOR MATCHES "Visual Studio") +if(GeneratorNeedsBuildSerialization) set_property(TEST roottest-cling-stl-dicts-build APPEND PROPERTY RESOURCE_LOCK NINJA_BUILD) set_property(TEST roottest-cling-stl-dicts-build APPEND PROPERTY FIXTURES_REQUIRED NINJA_BUILD_ALL) endif() diff --git a/roottest/root/io/rootcint/sigbug/CMakeLists.txt b/roottest/root/io/rootcint/sigbug/CMakeLists.txt index 7277dce32c311..90bdad91bd6e6 100644 --- a/roottest/root/io/rootcint/sigbug/CMakeLists.txt +++ b/roottest/root/io/rootcint/sigbug/CMakeLists.txt @@ -19,7 +19,7 @@ add_test(NAME ${GENERATE_DICTIONARY_TEST} ${build_config} --target ${dictname}${fast} -- ${always-make}) -if(CMAKE_GENERATOR MATCHES Ninja OR CMAKE_GENERATOR MATCHES "Visual Studio") +if(GeneratorNeedsBuildSerialization) set_property(TEST ${GENERATE_DICTIONARY_TEST} APPEND PROPERTY RESOURCE_LOCK NINJA_BUILD) set_property(TEST ${GENERATE_DICTIONARY_TEST} APPEND PROPERTY FIXTURES_REQUIRED NINJA_BUILD_ALL) endif() diff --git a/roottest/root/io/tmpifile/CMakeLists.txt b/roottest/root/io/tmpifile/CMakeLists.txt index fc94946fa7f64..01598393fc184 100644 --- a/roottest/root/io/tmpifile/CMakeLists.txt +++ b/roottest/root/io/tmpifile/CMakeLists.txt @@ -13,7 +13,7 @@ ROOTTEST_ADD_TEST(split-fail ROOTTEST_ADD_TEST(libjetevent-build COMMAND ${CMAKE_COMMAND} --build ${CMAKE_BINARY_DIR} ${build_config} --target JetEvent${fast} -- ${always-make}) -if(CMAKE_GENERATOR MATCHES Ninja OR CMAKE_GENERATOR MATCHES "Visual Studio") +if(GeneratorNeedsBuildSerialization) set_property(TEST roottest-root-io-tmpifile-libjetevent-build APPEND PROPERTY RESOURCE_LOCK NINJA_BUILD) set_property(TEST roottest-root-io-tmpifile-libjetevent-build APPEND PROPERTY FIXTURES_REQUIRED NINJA_BUILD_ALL) endif() diff --git a/roottest/root/io/transient/base/CMakeLists.txt b/roottest/root/io/transient/base/CMakeLists.txt index 5d4b65b5cecea..9b80c6833a991 100644 --- a/roottest/root/io/transient/base/CMakeLists.txt +++ b/roottest/root/io/transient/base/CMakeLists.txt @@ -10,7 +10,7 @@ add_test(NAME roottest-root-io-transient-base-build --target base${fast} -- ${always-make}) set_property(TEST roottest-root-io-transient-base-build PROPERTY FIXTURES_SETUP root-io-transient-base-build) -if(CMAKE_GENERATOR MATCHES Ninja OR CMAKE_GENERATOR MATCHES "Visual Studio") +if(GeneratorNeedsBuildSerialization) set_property(TEST roottest-root-io-transient-base-build APPEND PROPERTY RESOURCE_LOCK NINJA_BUILD) set_property(TEST roottest-root-io-transient-base-build APPEND PROPERTY FIXTURES_REQUIRED NINJA_BUILD_ALL) endif() diff --git a/roottest/root/meta/genreflex/ROOT-5768/CMakeLists.txt b/roottest/root/meta/genreflex/ROOT-5768/CMakeLists.txt index 0bf392ff26a8d..d28459ed56103 100644 --- a/roottest/root/meta/genreflex/ROOT-5768/CMakeLists.txt +++ b/roottest/root/meta/genreflex/ROOT-5768/CMakeLists.txt @@ -27,7 +27,7 @@ if(NOT MSVC OR win_broken_tests) ROOTTEST_ADD_TEST(PyCoolLib-build COMMAND ${CMAKE_COMMAND} --build ${CMAKE_BINARY_DIR} ${build_config} --target PyCoolLib${fast} -- ${always-make} FIXTURES_REQUIRED PyCool-reflex-dict) - if(CMAKE_GENERATOR MATCHES Ninja OR CMAKE_GENERATOR MATCHES "Visual Studio") + if(GeneratorNeedsBuildSerialization) set_property(TEST roottest-root-meta-genreflex-ROOT-5768-PyCoolLib-build APPEND PROPERTY RESOURCE_LOCK NINJA_BUILD) set_property(TEST roottest-root-meta-genreflex-ROOT-5768-PyCoolLib-build APPEND PROPERTY FIXTURES_REQUIRED NINJA_BUILD_ALL) endif() From e09553788ded7a3b10c5f210145a055e75131d2b Mon Sep 17 00:00:00 2001 From: Philippe Canal Date: Mon, 23 Feb 2026 10:56:59 -0600 Subject: [PATCH 18/21] cmake: Rename Ninja Build resource lock to CMake Build. It is need by at least Ninja and the Visual Studio generator --- cmake/modules/RootCTest.cmake | 8 ++++---- cmake/modules/RootMacros.cmake | 12 ++++++------ roottest/cling/dict/ROOT-8096/CMakeLists.txt | 4 ++-- roottest/cling/stl/dicts/CMakeLists.txt | 4 ++-- roottest/root/io/rootcint/sigbug/CMakeLists.txt | 4 ++-- roottest/root/io/tmpifile/CMakeLists.txt | 4 ++-- roottest/root/io/transient/base/CMakeLists.txt | 4 ++-- .../root/meta/genreflex/ROOT-5768/CMakeLists.txt | 4 ++-- 8 files changed, 22 insertions(+), 22 deletions(-) diff --git a/cmake/modules/RootCTest.cmake b/cmake/modules/RootCTest.cmake index 7904c708f5e3a..a46d7b6d08d9f 100644 --- a/cmake/modules/RootCTest.cmake +++ b/cmake/modules/RootCTest.cmake @@ -59,11 +59,11 @@ endforeach() # - Make all tests that require a ninja build depend on the above test. # - Use a RESOURCE_LOCK on all tests that invoke ninja, so no two tests will invoke ninja in parallel if(GeneratorNeedsBuildSerialization) - add_test(NAME ninja-build-all + add_test(NAME cmake-build-all COMMAND ${CMAKE_COMMAND} --build ${CMAKE_BINARY_DIR}) - set_tests_properties(ninja-build-all PROPERTIES - RESOURCE_LOCK NINJA_BUILD - FIXTURES_SETUP NINJA_BUILD_ALL + set_tests_properties(cmake-build-all PROPERTIES + RESOURCE_LOCK CMAKE_BUILD + FIXTURES_SETUP CMAKE_BUILD_ALL RUN_SERIAL True) set(GeneratorNeedsBuildSerialization True) endif() diff --git a/cmake/modules/RootMacros.cmake b/cmake/modules/RootMacros.cmake index 2576752fd0e7a..f702d88413e79 100644 --- a/cmake/modules/RootMacros.cmake +++ b/cmake/modules/RootMacros.cmake @@ -2662,8 +2662,8 @@ macro(ROOTTEST_GENERATE_DICTIONARY dictname) set_property(TEST ${GENERATE_DICTIONARY_TEST} PROPERTY ENVIRONMENT ${ROOTTEST_ENVIRONMENT}) if(GeneratorNeedsBuildSerialization) - set_property(TEST ${GENERATE_DICTIONARY_TEST} APPEND PROPERTY RESOURCE_LOCK NINJA_BUILD) - set_property(TEST ${GENERATE_DICTIONARY_TEST} APPEND PROPERTY FIXTURES_REQUIRED NINJA_BUILD_ALL) + set_property(TEST ${GENERATE_DICTIONARY_TEST} APPEND PROPERTY RESOURCE_LOCK CMAKE_BUILD) + set_property(TEST ${GENERATE_DICTIONARY_TEST} APPEND PROPERTY FIXTURES_REQUIRED CMAKE_BUILD_ALL) endif() if (ARG_FIXTURES_SETUP) @@ -2770,8 +2770,8 @@ macro(ROOTTEST_GENERATE_REFLEX_DICTIONARY dictionary) set_property(TEST ${GENERATE_REFLEX_TEST} PROPERTY ENVIRONMENT ${ROOTTEST_ENVIRONMENT}) if(GeneratorNeedsBuildSerialization) - set_property(TEST ${GENERATE_REFLEX_TEST} APPEND PROPERTY RESOURCE_LOCK NINJA_BUILD) - set_property(TEST ${GENERATE_REFLEX_TEST} APPEND PROPERTY FIXTURES_REQUIRED NINJA_BUILD_ALL) + set_property(TEST ${GENERATE_REFLEX_TEST} APPEND PROPERTY RESOURCE_LOCK CMAKE_BUILD) + set_property(TEST ${GENERATE_REFLEX_TEST} APPEND PROPERTY FIXTURES_REQUIRED CMAKE_BUILD_ALL) endif() if (ARG_FIXTURES_SETUP) @@ -2882,8 +2882,8 @@ macro(ROOTTEST_GENERATE_EXECUTABLE executable) endif() if(GeneratorNeedsBuildSerialization) - set_property(TEST ${GENERATE_EXECUTABLE_TEST} APPEND PROPERTY RESOURCE_LOCK NINJA_BUILD) - set_property(TEST ${GENERATE_EXECUTABLE_TEST} APPEND PROPERTY FIXTURES_REQUIRED NINJA_BUILD_ALL) + set_property(TEST ${GENERATE_EXECUTABLE_TEST} APPEND PROPERTY RESOURCE_LOCK CMAKE_BUILD) + set_property(TEST ${GENERATE_EXECUTABLE_TEST} APPEND PROPERTY FIXTURES_REQUIRED CMAKE_BUILD_ALL) endif() if(MSVC AND NOT CMAKE_GENERATOR MATCHES Ninja) diff --git a/roottest/cling/dict/ROOT-8096/CMakeLists.txt b/roottest/cling/dict/ROOT-8096/CMakeLists.txt index 4614dba98fa9b..e86dfb94915d3 100644 --- a/roottest/cling/dict/ROOT-8096/CMakeLists.txt +++ b/roottest/cling/dict/ROOT-8096/CMakeLists.txt @@ -21,8 +21,8 @@ add_test(NAME roottest-cling-dict-ROOT-8096-build # --target ${targetname_libgen}${fast}) set_property(TEST roottest-cling-dict-ROOT-8096-build PROPERTY ENVIRONMENT ${ROOTTEST_ENVIRONMENT}) if(GeneratorNeedsBuildSerialization) - set_property(TEST roottest-cling-dict-ROOT-8096-build APPEND PROPERTY RESOURCE_LOCK NINJA_BUILD) - set_property(TEST roottest-cling-dict-ROOT-8096-build APPEND PROPERTY FIXTURES_REQUIRED NINJA_BUILD_ALL) + set_property(TEST roottest-cling-dict-ROOT-8096-build APPEND PROPERTY RESOURCE_LOCK CMAKE_BUILD) + set_property(TEST roottest-cling-dict-ROOT-8096-build APPEND PROPERTY FIXTURES_REQUIRED CMAKE_BUILD_ALL) endif() if(MSVC AND NOT CMAKE_GENERATOR MATCHES Ninja) diff --git a/roottest/cling/stl/dicts/CMakeLists.txt b/roottest/cling/stl/dicts/CMakeLists.txt index 874281c54f2cb..58eb53d5d9839 100644 --- a/roottest/cling/stl/dicts/CMakeLists.txt +++ b/roottest/cling/stl/dicts/CMakeLists.txt @@ -12,6 +12,6 @@ ROOTTEST_LINKER_LIBRARY(stldictTest TEST MyClass1.cpp MyClass2.cpp MyClass3.cpp ROOT_ADD_TEST(roottest-cling-stl-dicts-build COMMAND ${CMAKE_COMMAND} --build ${CMAKE_BINARY_DIR} ${build_config} --target stldictTest${fast} -- ${always-make}) if(GeneratorNeedsBuildSerialization) - set_property(TEST roottest-cling-stl-dicts-build APPEND PROPERTY RESOURCE_LOCK NINJA_BUILD) - set_property(TEST roottest-cling-stl-dicts-build APPEND PROPERTY FIXTURES_REQUIRED NINJA_BUILD_ALL) + set_property(TEST roottest-cling-stl-dicts-build APPEND PROPERTY RESOURCE_LOCK CMAKE_BUILD) + set_property(TEST roottest-cling-stl-dicts-build APPEND PROPERTY FIXTURES_REQUIRED CMAKE_BUILD_ALL) endif() diff --git a/roottest/root/io/rootcint/sigbug/CMakeLists.txt b/roottest/root/io/rootcint/sigbug/CMakeLists.txt index 90bdad91bd6e6..29643d57e2638 100644 --- a/roottest/root/io/rootcint/sigbug/CMakeLists.txt +++ b/roottest/root/io/rootcint/sigbug/CMakeLists.txt @@ -20,6 +20,6 @@ add_test(NAME ${GENERATE_DICTIONARY_TEST} --target ${dictname}${fast} -- ${always-make}) if(GeneratorNeedsBuildSerialization) - set_property(TEST ${GENERATE_DICTIONARY_TEST} APPEND PROPERTY RESOURCE_LOCK NINJA_BUILD) - set_property(TEST ${GENERATE_DICTIONARY_TEST} APPEND PROPERTY FIXTURES_REQUIRED NINJA_BUILD_ALL) + set_property(TEST ${GENERATE_DICTIONARY_TEST} APPEND PROPERTY RESOURCE_LOCK CMAKE_BUILD) + set_property(TEST ${GENERATE_DICTIONARY_TEST} APPEND PROPERTY FIXTURES_REQUIRED CMAKE_BUILD_ALL) endif() diff --git a/roottest/root/io/tmpifile/CMakeLists.txt b/roottest/root/io/tmpifile/CMakeLists.txt index 01598393fc184..d8ce592226b0a 100644 --- a/roottest/root/io/tmpifile/CMakeLists.txt +++ b/roottest/root/io/tmpifile/CMakeLists.txt @@ -14,8 +14,8 @@ ROOTTEST_ADD_TEST(split-fail ROOTTEST_ADD_TEST(libjetevent-build COMMAND ${CMAKE_COMMAND} --build ${CMAKE_BINARY_DIR} ${build_config} --target JetEvent${fast} -- ${always-make}) if(GeneratorNeedsBuildSerialization) - set_property(TEST roottest-root-io-tmpifile-libjetevent-build APPEND PROPERTY RESOURCE_LOCK NINJA_BUILD) - set_property(TEST roottest-root-io-tmpifile-libjetevent-build APPEND PROPERTY FIXTURES_REQUIRED NINJA_BUILD_ALL) + set_property(TEST roottest-root-io-tmpifile-libjetevent-build APPEND PROPERTY RESOURCE_LOCK CMAKE_BUILD) + set_property(TEST roottest-root-io-tmpifile-libjetevent-build APPEND PROPERTY FIXTURES_REQUIRED CMAKE_BUILD_ALL) endif() ROOTTEST_ADD_TEST(sync-rate diff --git a/roottest/root/io/transient/base/CMakeLists.txt b/roottest/root/io/transient/base/CMakeLists.txt index 9b80c6833a991..820907cd30af4 100644 --- a/roottest/root/io/transient/base/CMakeLists.txt +++ b/roottest/root/io/transient/base/CMakeLists.txt @@ -11,8 +11,8 @@ add_test(NAME roottest-root-io-transient-base-build -- ${always-make}) set_property(TEST roottest-root-io-transient-base-build PROPERTY FIXTURES_SETUP root-io-transient-base-build) if(GeneratorNeedsBuildSerialization) - set_property(TEST roottest-root-io-transient-base-build APPEND PROPERTY RESOURCE_LOCK NINJA_BUILD) - set_property(TEST roottest-root-io-transient-base-build APPEND PROPERTY FIXTURES_REQUIRED NINJA_BUILD_ALL) + set_property(TEST roottest-root-io-transient-base-build APPEND PROPERTY RESOURCE_LOCK CMAKE_BUILD) + set_property(TEST roottest-root-io-transient-base-build APPEND PROPERTY FIXTURES_REQUIRED CMAKE_BUILD_ALL) endif() ROOTTEST_ADD_TEST(WriteFile diff --git a/roottest/root/meta/genreflex/ROOT-5768/CMakeLists.txt b/roottest/root/meta/genreflex/ROOT-5768/CMakeLists.txt index d28459ed56103..26027743a4683 100644 --- a/roottest/root/meta/genreflex/ROOT-5768/CMakeLists.txt +++ b/roottest/root/meta/genreflex/ROOT-5768/CMakeLists.txt @@ -28,7 +28,7 @@ if(NOT MSVC OR win_broken_tests) COMMAND ${CMAKE_COMMAND} --build ${CMAKE_BINARY_DIR} ${build_config} --target PyCoolLib${fast} -- ${always-make} FIXTURES_REQUIRED PyCool-reflex-dict) if(GeneratorNeedsBuildSerialization) - set_property(TEST roottest-root-meta-genreflex-ROOT-5768-PyCoolLib-build APPEND PROPERTY RESOURCE_LOCK NINJA_BUILD) - set_property(TEST roottest-root-meta-genreflex-ROOT-5768-PyCoolLib-build APPEND PROPERTY FIXTURES_REQUIRED NINJA_BUILD_ALL) + set_property(TEST roottest-root-meta-genreflex-ROOT-5768-PyCoolLib-build APPEND PROPERTY RESOURCE_LOCK CMAKE_BUILD) + set_property(TEST roottest-root-meta-genreflex-ROOT-5768-PyCoolLib-build APPEND PROPERTY FIXTURES_REQUIRED CMAKE_BUILD_ALL) endif() endif() From 41c4b84d81f4dcd099fabf6cc8007bf828d866e6 Mon Sep 17 00:00:00 2001 From: Philippe Canal Date: Mon, 23 Feb 2026 14:29:28 -0600 Subject: [PATCH 19/21] cmake: Add missing config to roottest (MSVC) (re)build --- cmake/modules/RootCTest.cmake | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmake/modules/RootCTest.cmake b/cmake/modules/RootCTest.cmake index a46d7b6d08d9f..3cbc55098c073 100644 --- a/cmake/modules/RootCTest.cmake +++ b/cmake/modules/RootCTest.cmake @@ -60,7 +60,7 @@ endforeach() # - Use a RESOURCE_LOCK on all tests that invoke ninja, so no two tests will invoke ninja in parallel if(GeneratorNeedsBuildSerialization) add_test(NAME cmake-build-all - COMMAND ${CMAKE_COMMAND} --build ${CMAKE_BINARY_DIR}) + COMMAND ${CMAKE_COMMAND} --build ${CMAKE_BINARY_DIR} --config ${build_configuration}) set_tests_properties(cmake-build-all PROPERTIES RESOURCE_LOCK CMAKE_BUILD FIXTURES_SETUP CMAKE_BUILD_ALL From 67ff236a6dc5bd2cf2af279548aae55987c44215 Mon Sep 17 00:00:00 2001 From: Philippe Canal Date: Tue, 24 Feb 2026 14:16:17 -0600 Subject: [PATCH 20/21] io: Extend code documentation related to ByteCount --- core/base/inc/TBuffer.h | 37 +++++++++++++++++++++++++++++++++++++ io/io/inc/TBufferFile.h | 9 +++++++-- 2 files changed, 44 insertions(+), 2 deletions(-) diff --git a/core/base/inc/TBuffer.h b/core/base/inc/TBuffer.h index fb0e369fca388..af6b0bb000f8d 100644 --- a/core/base/inc/TBuffer.h +++ b/core/base/inc/TBuffer.h @@ -69,6 +69,13 @@ class TBuffer : public TObject { Int_t Write(const char *name, Int_t opt, Long64_t bufsize) const override { return TObject::Write(name, opt, bufsize); } + //////////////////////////////////////////////////////////////////////////////// + /// Reserve space for a leading byte count and return the position where to + /// store the byte count value. + /// + /// \param[in] cl The class for which we are reserving the byte count, used for error reporting. + /// \return The position (cntpos) where the byte count should be stored later, + /// or kOverflowPosition if the position exceeds kMaxCountPosition virtual UInt_t ReserveByteCount(const TClass *) = 0; public: @@ -127,6 +134,36 @@ class TBuffer : public TObject { virtual Long64_t CheckByteCount(ULong64_t startpos, ULong64_t bcnt, const TClass *clss) = 0; virtual Long64_t CheckByteCount(ULong64_t startpos, ULong64_t bcnt, const char *classname) = 0; virtual void SetByteCount(ULong64_t cntpos, Bool_t packInVersion = kFALSE)= 0; + + /// \class TBuffer::ByteCountWriter + /// \ingroup Base + /// \brief RAII helper to automatically write the byte count for an object + /// to be used in the rare case where writing the class version number + /// and the byte count are decoupled. + /// + /// `ByteCountWriter` encapsulates the pattern: + /// 1. Reserve space for a leading byte count with ReserveByteCount(). + /// 2. Stream the object content. + /// 3. Finalize the byte count with SetByteCount(). + /// + /// \note Create the instance as a local variable and keep it alive until all + /// the bytes that should be counted have been written to the buffer. + /// + /// ### Example + /// \code{.cpp} + /// void MyClass::Streamer(TBuffer &b) + /// { + /// if (b.IsWriting()) { + /// // Reserve space for the byte count and auto-finalize on scope exit. + /// TBuffer::ByteCountWriter bcnt(b, MyClass::Class()); + /// + /// b.WriteClass(MyClass::Class()); + /// // ... stream members ... + /// } else { + /// // ... read members ... + /// } + /// } + /// \endcode class ByteCountWriter { TBuffer &fBuffer; Bool_t fPackInVersion; diff --git a/io/io/inc/TBufferFile.h b/io/io/inc/TBufferFile.h index 2b7d221ea2a77..0c532786d7e51 100644 --- a/io/io/inc/TBufferFile.h +++ b/io/io/inc/TBufferFile.h @@ -54,8 +54,13 @@ class TBufferFile : public TBufferIO { using ByteCountLocator_t = std::size_t; // This might become a pair if we implement chunked keys struct ByteCountLocationInfo { - ByteCountLocator_t locator; ///< Position where the byte count value is stored - const TClass* cl; ///< Class for which the byte count is reserved + ///< Position where the byte count value is stored + ByteCountLocator_t locator; + ///< Class for which the byte count is reserved. Usually this is the + ///< the base class or type of the pointer. + const TClass* cl; + ///< Alternative class that might used. Usually this is the derived + ///< class or the actual type of the object. const TClass* alt; }; using ByteCountStack_t = std::vector; From ef03d21e161f3f7cdff5efef2d9368c408d24187 Mon Sep 17 00:00:00 2001 From: Philippe Canal Date: Tue, 24 Feb 2026 14:18:03 -0600 Subject: [PATCH 21/21] NFC: white space --- io/io/src/TBufferFile.cxx | 3 ++- io/io/test/CMakeLists.txt | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/io/io/src/TBufferFile.cxx b/io/io/src/TBufferFile.cxx index 994a01c73a543..5742ff68db112 100644 --- a/io/io/src/TBufferFile.cxx +++ b/io/io/src/TBufferFile.cxx @@ -2885,9 +2885,10 @@ TClass *TBufferFile::ReadClass(const TClass *clReq, UInt_t *objTag) fVersion = 1; // When objTag is not used, the caller is not interested in the byte // count and will not (can not) call CheckByteCount. - if (objTag) + if (objTag) { // Note: the actual class is set later on. fByteCountStack.push_back({fBufCur - fBuffer - sizeof(UInt_t), clReq, nullptr}); + } startpos = UInt_t(fBufCur-fBuffer); *this >> tag; } diff --git a/io/io/test/CMakeLists.txt b/io/io/test/CMakeLists.txt index cd5103b6e5fc5..c39c0eacf39a0 100644 --- a/io/io/test/CMakeLists.txt +++ b/io/io/test/CMakeLists.txt @@ -58,4 +58,4 @@ if(NOT _32BIT) MACRO testByteCount.cxx+ OUTREF testByteCount.ref FIXTURES_REQUIRED io-io-tobj-fixture) -endif() \ No newline at end of file +endif()