From c4de23d022289fde55cef5b14ff37c6a03f476fe Mon Sep 17 00:00:00 2001 From: cmargalejo Date: Mon, 4 May 2026 10:10:21 +0200 Subject: [PATCH 01/10] Warning if unsupported legacy detector signal branches are used --- source/framework/core/src/TRestRun.cxx | 109 +++++++++++++++++++++---- 1 file changed, 93 insertions(+), 16 deletions(-) diff --git a/source/framework/core/src/TRestRun.cxx b/source/framework/core/src/TRestRun.cxx index b6596ac3d..b74ed77a0 100644 --- a/source/framework/core/src/TRestRun.cxx +++ b/source/framework/core/src/TRestRun.cxx @@ -43,10 +43,65 @@ #include "TRestManager.h" #include "TRestVersion.h" +#include + using namespace std; std::mutex mutex_read; +namespace { +void DisableBranchRecursively(TBranch* branch) { + if (branch == nullptr) return; + + branch->SetStatus(false); + auto subs = branch->GetListOfBranches(); + for (int i = 0; i <= subs->GetLast(); i++) { + DisableBranchRecursively((TBranch*)subs->At(i)); + } +} + +bool BranchHasLegacyDetectorSignalStreamer(TBranch* branch) { + if (branch == nullptr) return false; + + auto branchElement = dynamic_cast(branch); + if (branchElement != nullptr && ((std::string)branch->GetName() == "fSignal.fSignalTime" || + (std::string)branch->GetName() == "fSignal.fSignalCharge")) { + const auto version = branchElement->GetClassVersion(); + if (version > 0 && version < 4) return true; + } + + auto subs = branch->GetListOfBranches(); + for (int i = 0; i <= subs->GetLast(); i++) { + if (BranchHasLegacyDetectorSignalStreamer((TBranch*)subs->At(i))) return true; + } + + return false; +} + +bool IsUnsupportedLegacyDetectorSignalBranch(TBranch* branch) { + if (branch == nullptr) return false; + if ((std::string)branch->GetName() != "TRestDetectorSignalEventBranch") return false; + + const auto signalClass = TClass::GetClass("TRestDetectorSignal"); + if (signalClass == nullptr || signalClass->GetClassVersion() < 4) return false; + + return BranchHasLegacyDetectorSignalStreamer(branch); +} + +void WarnUnsupportedLegacyDetectorSignalBranch() { + RESTWarning << "REST Warning : (TRestRun) cannot read TRestDetectorSignalEvent from this file with " + "the loaded detector library." + << RESTendl; + RESTWarning << "The file contains legacy vector signal time/charge split branches, while the " + "current TRestDetectorSignal class expects vector." + << RESTendl; + RESTWarning << "This is the schema change introduced in detectorlib PR109. Other event branches can " + "still be read, but this detector signal event branch is disabled to avoid excessive " + "memory usage or a crash." + << RESTendl; +} +} // namespace + ClassImp(TRestRun); TRestRun::TRestRun() { Initialize(); } @@ -616,6 +671,13 @@ void TRestRun::ReadInputFileTrees() { } fInputEvent->InitializeWithMetadata(this); + if (IsUnsupportedLegacyDetectorSignalBranch(br)) { + WarnUnsupportedLegacyDetectorSignalBranch(); + DisableBranchRecursively(br); + delete fInputEvent; + fInputEvent = nullptr; + return; + } fEventTree->SetBranchAddress(br->GetName(), &fInputEvent); fEventBranchLoc = branches->GetLast(); RESTDebug << "found event branch of event type: " << fInputEvent->ClassName() @@ -1246,30 +1308,45 @@ void TRestRun::SetExtProcess(TRestEventProcess* p) { void TRestRun::SetInputEvent(TRestEvent* event) { if (event != nullptr) { if (fEventTree != nullptr) { - if (fInputEvent != nullptr) { - fEventTree->SetBranchAddress((TString)fInputEvent->ClassName() + "Branch", nullptr); - fEventTree->SetBranchStatus((TString)fInputEvent->ClassName() + "Branch", false); - } TObjArray* branches = fEventTree->GetListOfBranches(); string branchName = (string)event->ClassName() + "Branch"; + TBranch* selectedBranch = nullptr; + Int_t selectedBranchIndex = -1; for (int i = 0; i <= branches->GetLast(); i++) { auto branch = (TBranch*)branches->At(i); if ((string)branch->GetName() == branchName) { - RESTDebug << "Setting input event.. Type: " << event->ClassName() << " Address: " << event - << RESTendl; - fInputEvent = event; - fEventTree->SetBranchAddress(branchName.c_str(), &fInputEvent); - fEventTree->SetBranchStatus(branchName.c_str(), false); - fEventBranchLoc = i; + selectedBranch = branch; + selectedBranchIndex = i; break; - } else if (i == branches->GetLast()) { - RESTWarning << "REST Warning : (TRestRun) cannot find corresponding " - "branch in event tree!" - << RESTendl; - RESTWarning << "Event Type : " << event->ClassName() << RESTendl; - RESTWarning << "Input event not set!" << RESTendl; } } + + if (selectedBranch == nullptr) { + RESTWarning << "REST Warning : (TRestRun) cannot find corresponding " + "branch in event tree!" + << RESTendl; + RESTWarning << "Event Type : " << event->ClassName() << RESTendl; + RESTWarning << "Input event not set!" << RESTendl; + return; + } + + if (IsUnsupportedLegacyDetectorSignalBranch(selectedBranch)) { + WarnUnsupportedLegacyDetectorSignalBranch(); + DisableBranchRecursively(selectedBranch); + return; + } + + if (fInputEvent != nullptr) { + fEventTree->SetBranchAddress((TString)fInputEvent->ClassName() + "Branch", nullptr); + fEventTree->SetBranchStatus((TString)fInputEvent->ClassName() + "Branch", false); + } + + RESTDebug << "Setting input event.. Type: " << event->ClassName() << " Address: " << event + << RESTendl; + fInputEvent = event; + fEventTree->SetBranchAddress(branchName.c_str(), &fInputEvent); + fEventTree->SetBranchStatus(branchName.c_str(), false); + fEventBranchLoc = selectedBranchIndex; } else { fInputEvent = event; } From 0088baf5b4cf79fff23a0a1153bc03222f103a86 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Mon, 4 May 2026 08:17:55 +0000 Subject: [PATCH 02/10] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- source/framework/core/src/TRestRun.cxx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/source/framework/core/src/TRestRun.cxx b/source/framework/core/src/TRestRun.cxx index b74ed77a0..08b187e88 100644 --- a/source/framework/core/src/TRestRun.cxx +++ b/source/framework/core/src/TRestRun.cxx @@ -36,6 +36,8 @@ #include #endif // !WIN32 +#include + #include #include "TRestDataBase.h" @@ -43,8 +45,6 @@ #include "TRestManager.h" #include "TRestVersion.h" -#include - using namespace std; std::mutex mutex_read; From fe50ea43d21733cc91c64125c5b4c8829745a736 Mon Sep 17 00:00:00 2001 From: cmargalejo Date: Fri, 12 Jun 2026 10:03:26 +0200 Subject: [PATCH 03/10] Add two-stage recovery tool for legacy detector signal data REST files written with TRestDetectorSignal ClassDef < 4 store fSignalTime/fSignalCharge as vector. Since restManager output files do not contain the event-class StreamerInfos, ROOT cannot apply schema evolution and misreads the float payload as doubles, making the detector signal branch unreadable (detectorlib#125). The data is intact on disk and fully recoverable: - macros/legacy/recoverLegacySignalData.C (stage 1, run with plain root): reads the signal branch using replica classes that match the legacy on-disk layout exactly and extracts the data to an intermediate file. Deliberately not named REST_*.C so restRoot's macro loading never interprets the replica class definitions. - macros/legacy/REST_RebuildLegacySignalFile.C (stage 2, run with restRoot): writes a new file with the signal branch rebuilt using the current classes, all other event branches and the AnalysisTree copied, metadata keys preserved, and the event-class StreamerInfos stored this time. Optional in-place overwrite keeps a .bak copy. The on-disk layout is detected via TBranchElement::GetClassVersion() on the fSignal sub-branches; files already at version 4 are skipped. Verified on R00236 (V2.4.0, 261 entries): 19750 signals and 447902 points recovered bit-exact; the rebuilt file reads back correctly via TRestRun with the current vector classes. Co-Authored-By: Claude Opus 4.8 --- macros/legacy/REST_RebuildLegacySignalFile.C | 287 +++++++++++++++++++ macros/legacy/recoverLegacySignalData.C | 232 +++++++++++++++ 2 files changed, 519 insertions(+) create mode 100644 macros/legacy/REST_RebuildLegacySignalFile.C create mode 100644 macros/legacy/recoverLegacySignalData.C diff --git a/macros/legacy/REST_RebuildLegacySignalFile.C b/macros/legacy/REST_RebuildLegacySignalFile.C new file mode 100644 index 000000000..26af4734c --- /dev/null +++ b/macros/legacy/REST_RebuildLegacySignalFile.C @@ -0,0 +1,287 @@ +// Stage 2 of 2 — rebuild a legacy REST file with current TRestDetectorSignalEvent. +// +// Takes the original legacy file plus the intermediate file produced by stage 1 +// (macros/legacy/recoverLegacySignalData.C, run with plain root) and writes a new +// file in which: +// - the TRestDetectorSignalEventBranch is rebuilt with the current +// vector-based classes, +// - all other EventTree branches and the AnalysisTree are copied unchanged, +// - all readable metadata keys (TRestRun, readout, processes, ...) are copied, +// - the event-class StreamerInfos ARE stored (they are missing from legacy +// restManager output, which is what made these files unreadable in the +// first place — see rest-for-physics/detectorlib#125). +// +// Usage (restRoot): +// REST_RebuildLegacySignalFile("R00236_...V2.4.0.root") +// writes R00236_...V2.4.0_Fixed.root +// REST_RebuildLegacySignalFile("input.root", "", "", true) +// overwrites input.root in place (the original is kept as input.root.bak) +// +// Arguments: +// originalFile - the legacy REST file +// signalDataFile - intermediate from stage 1; default: _LegacySignalData.root +// outputFile - default: _Fixed.root (ignored when overwrite=true) +// overwrite - replace originalFile in place, keeping a .bak copy + +#include +#include +#include +#include +#include +#include +#include +#include + +#include +#include +#include +#include + +namespace REST_Rebuild_Internal { + +Int_t GetOnDiskSignalVersion(TBranch* branch) { + if (branch == nullptr) return -1; + + auto branchElement = dynamic_cast(branch); + if (branchElement != nullptr) { + std::string name = branch->GetName(); + if (name == "fSignal.fSignalTime" || name == "fSignal.fSignalCharge") + return branchElement->GetClassVersion(); + } + + auto subs = branch->GetListOfBranches(); + for (int i = 0; i <= subs->GetLast(); i++) { + const Int_t version = GetOnDiskSignalVersion((TBranch*)subs->At(i)); + if (version > 0) return version; + } + return -1; +} + +void SetBranchStatusRecursive(TBranch* branch, Bool_t status) { + if (branch == nullptr) return; + branch->SetStatus(status); + auto subs = branch->GetListOfBranches(); + for (int i = 0; i <= subs->GetLast(); i++) + SetBranchStatusRecursive((TBranch*)subs->At(i), status); +} + +} // namespace REST_Rebuild_Internal + +void REST_RebuildLegacySignalFile(const char* originalFile, const char* signalDataFile = "", + const char* outputFile = "", bool overwrite = false) { + using namespace REST_Rebuild_Internal; + + std::string base = originalFile; + const size_t pos = base.rfind(".root"); + if (pos != std::string::npos) base = base.substr(0, pos); + + std::string dataName = signalDataFile; + if (dataName.empty()) dataName = base + "_LegacySignalData.root"; + + std::string outName = outputFile; + if (outName.empty()) outName = base + "_Fixed.root"; + if (overwrite) outName = base + "_FixedTmp.root"; + + // --- open inputs --- + TFile* original = TFile::Open(originalFile); + if (original == nullptr || original->IsZombie()) { + std::cout << "ERROR: cannot open original file: " << originalFile << std::endl; + return; + } + TFile* data = TFile::Open(dataName.c_str()); + if (data == nullptr || data->IsZombie()) { + std::cout << "ERROR: cannot open signal data file: " << dataName << std::endl; + std::cout << "Run stage 1 first (with plain root, NOT restRoot):" << std::endl; + std::cout << " root -l -b -q 'recoverLegacySignalData.C+(\"" << originalFile << "\")'" + << std::endl; + return; + } + + auto oldTree = dynamic_cast(original->Get("EventTree")); + auto dataTree = dynamic_cast(data->Get("LegacySignalData")); + if (oldTree == nullptr || dataTree == nullptr) { + std::cout << "ERROR: EventTree or LegacySignalData tree not found." << std::endl; + return; + } + + TBranch* signalBranch = oldTree->GetBranch("TRestDetectorSignalEventBranch"); + if (signalBranch == nullptr) { + std::cout << "ERROR: no TRestDetectorSignalEventBranch in " << originalFile << std::endl; + return; + } + const Int_t onDiskVersion = GetOnDiskSignalVersion(signalBranch); + if (onDiskVersion >= 4) { + std::cout << "This file already uses the current layout (TRestDetectorSignal v" << onDiskVersion + << "). Nothing to rebuild." << std::endl; + return; + } + + const Long64_t nEntries = oldTree->GetEntries(); + if (dataTree->GetEntries() != nEntries) { + std::cout << "ERROR: entry mismatch — EventTree has " << nEntries << " entries, signal data has " + << dataTree->GetEntries() << ". Wrong intermediate file?" << std::endl; + return; + } + + // --- bind the intermediate tree --- + Int_t runOrigin, subRunOrigin, eventID, subEventID, timeSec, timeNanoSec; + Bool_t ok; + TString* subEventTag = nullptr; + std::vector*signalID = nullptr, *nPoints = nullptr; + std::vector*times = nullptr, *charges = nullptr; + + dataTree->SetBranchAddress("runOrigin", &runOrigin); + dataTree->SetBranchAddress("subRunOrigin", &subRunOrigin); + dataTree->SetBranchAddress("eventID", &eventID); + dataTree->SetBranchAddress("subEventID", &subEventID); + dataTree->SetBranchAddress("timeSec", &timeSec); + dataTree->SetBranchAddress("timeNanoSec", &timeNanoSec); + dataTree->SetBranchAddress("ok", &ok); + dataTree->SetBranchAddress("subEventTag", &subEventTag); + dataTree->SetBranchAddress("signalID", &signalID); + dataTree->SetBranchAddress("nPoints", &nPoints); + dataTree->SetBranchAddress("times", ×); + dataTree->SetBranchAddress("charges", &charges); + + // --- output file (single write session: StreamerInfos are preserved) --- + TFile* out = TFile::Open(outName.c_str(), "RECREATE"); + if (out == nullptr || out->IsZombie()) { + std::cout << "ERROR: cannot create output file: " << outName << std::endl; + return; + } + + // --- copy metadata keys (highest cycle only, skip trees) --- + std::set seen; + std::vector skipped; + TIter nextKey(original->GetListOfKeys()); + TKey* key; + while ((key = (TKey*)nextKey())) { + const std::string keyName = key->GetName(); + if (seen.count(keyName)) continue; // keys are ordered newest cycle first + seen.insert(keyName); + + TClass* cl = TClass::GetClass(key->GetClassName()); + if (cl != nullptr && cl->InheritsFrom("TTree")) continue; // trees handled below + + TObject* obj = (cl != nullptr) ? key->ReadObj() : nullptr; + if (obj == nullptr) { + skipped.push_back(keyName + " (" + key->GetClassName() + ")"); + continue; + } + out->cd(); + obj->Write(keyName.c_str()); + } + + // --- rebuild the EventTree --- + // A brand-new tree is created with branches built from the CURRENT classes. + // Fast-cloning the old tree is not possible: without StreamerInfos in the + // file, ROOT cannot map old split-branch structures onto the current + // classes (e.g. the track branch aborts in InitializeOffsets). Reading the + // other event branches with the current classes works, so they are copied + // by deserialize-and-fill. + SetBranchStatusRecursive(signalBranch, 0); + + std::vector otherBranchNames; + TIter nextBranch(oldTree->GetListOfBranches()); + TBranch* br; + while ((br = (TBranch*)nextBranch())) { + const std::string name = br->GetName(); + if (name == "TRestDetectorSignalEventBranch") continue; + const size_t suffix = name.rfind("Branch"); + const std::string className = (suffix != std::string::npos) ? name.substr(0, suffix) : name; + if (TClass::GetClass(className.c_str()) == nullptr) { + std::cout << "WARNING: no dictionary for event class '" << className << "'; branch '" << name + << "' will NOT be copied!" << std::endl; + SetBranchStatusRecursive(br, 0); + continue; + } + otherBranchNames.push_back(name); + } + + out->cd(); + auto newTree = new TTree("EventTree", "EventTree"); + + // stable storage for the object pointers shared between both trees + std::vector otherEvents(otherBranchNames.size(), nullptr); + for (size_t b = 0; b < otherBranchNames.size(); b++) { + const std::string& name = otherBranchNames[b]; + const std::string className = name.substr(0, name.rfind("Branch")); + TClass* cl = TClass::GetClass(className.c_str()); + otherEvents[b] = (TRestEvent*)cl->New(); + oldTree->SetBranchAddress(name.c_str(), &otherEvents[b]); + newTree->Branch(name.c_str(), className.c_str(), &otherEvents[b]); + } + + auto event = new TRestDetectorSignalEvent(); + newTree->Branch("TRestDetectorSignalEventBranch", &event); + + Long64_t totalSignals = 0, totalPoints = 0; + for (Long64_t i = 0; i < nEntries; i++) { + oldTree->GetEntry(i); // reads the other event branches (signal branch disabled) + dataTree->GetEntry(i); + + event->Initialize(); + event->SetRunOrigin(runOrigin); + event->SetSubRunOrigin(subRunOrigin); + event->SetID(eventID); + event->SetSubID(subEventID); + event->SetSubEventTag(*subEventTag); + event->SetTime((Double_t)timeSec, (Double_t)timeNanoSec); + event->SetOK(ok); + + size_t offset = 0; + for (size_t s = 0; s < signalID->size(); s++) { + TRestDetectorSignal signal; + signal.SetSignalID(signalID->at(s)); + const size_t n = nPoints->at(s); + for (size_t p = 0; p < n; p++) + signal.NewPoint(times->at(offset + p), charges->at(offset + p)); + offset += n; + event->AddSignal(signal); + } + totalSignals += signalID->size(); + totalPoints += offset; + + newTree->Fill(); + } + newTree->Write("", TObject::kOverwrite); + + // --- copy the AnalysisTree unchanged --- + auto anaTree = dynamic_cast(original->Get("AnalysisTree")); + if (anaTree != nullptr) { + out->cd(); + TTree* anaClone = anaTree->CloneTree(-1, "fast"); + anaClone->Write("", TObject::kOverwrite); + } else { + std::cout << "WARNING: no AnalysisTree found; skipping." << std::endl; + } + + out->Close(); + original->Close(); + data->Close(); + + // --- overwrite handling --- + std::string finalName = outName; + if (overwrite) { + const std::string backup = std::string(originalFile) + ".bak"; + if (gSystem->Rename(originalFile, backup.c_str()) != 0) { + std::cout << "ERROR: could not move original to " << backup << "; fixed file left at " + << outName << std::endl; + return; + } + gSystem->Rename(outName.c_str(), originalFile); + finalName = originalFile; + std::cout << "Original file kept as: " << backup << std::endl; + } + + std::cout << std::endl; + std::cout << "Rebuilt " << nEntries << " entries: " << totalSignals << " signals, " << totalPoints + << " points." << std::endl; + if (!skipped.empty()) { + std::cout << "WARNING: " << skipped.size() + << " metadata key(s) could not be read with the current libraries and were NOT copied:" + << std::endl; + for (const auto& s : skipped) std::cout << " - " << s << std::endl; + } + std::cout << "Fixed file written to: " << finalName << std::endl; +} diff --git a/macros/legacy/recoverLegacySignalData.C b/macros/legacy/recoverLegacySignalData.C new file mode 100644 index 000000000..4bd730932 --- /dev/null +++ b/macros/legacy/recoverLegacySignalData.C @@ -0,0 +1,232 @@ +// Stage 1 of 2 — recover TRestDetectorSignalEvent data from legacy REST files. +// +// REST files produced with detectorlib < v4 of TRestDetectorSignal (REST <= v2.4.2) +// store fSignalTime/fSignalCharge as vector. The current class uses +// vector, and since those files do not contain the StreamerInfo of the +// event classes, ROOT cannot convert on read: it misinterprets the float payload +// as doubles, leading to huge bogus allocations (rest-for-physics/detectorlib#125). +// +// This macro reads the signal data with replica classes that match the legacy +// on-disk layout exactly (so no conversion is needed) and extracts it to an +// intermediate file. Stage 2 (REST_RebuildLegacySignalFile.C, run with restRoot) +// rebuilds a fixed file with the current classes. +// +// IMPORTANT: run this macro with PLAIN root, NOT restRoot — the replica classes +// would clash with the real REST classes: +// +// root -l -b -q 'recoverLegacySignalData.C+("R00236_...V2.4.0.root")' +// +// (note the '+': the macro must be compiled with ACLiC so that the replica +// classes get a dictionary) +// +// The intermediate file is written next to the input as +// _LegacySignalData.root unless an explicit output path is given. +// +// This file is deliberately NOT named REST_*.C so that restRoot's --m macro +// loading does not interpret it (the replica class definitions below would +// conflict with the compiled REST classes). + +#include +#include +#include +#include +#include +#include +#include +#include + +#include +#include +#include + +//------------------------------------------------------------------------------ +// Replica classes matching the legacy on-disk layout (REST <= v2.4.2). +// TRestDetectorSignal uses the v3 layout (fName/fType added Aug 2023); files +// written with v1/v2 simply lack those sub-branches and reading them is a no-op. +//------------------------------------------------------------------------------ +class TRestEvent : public TObject { + public: + Int_t fRunOrigin = 0; + Int_t fSubRunOrigin = 0; + Int_t fEventID = 0; + Int_t fSubEventID = 0; + TString fSubEventTag; + TTimeStamp fEventTime; + Bool_t fOk = true; + ClassDef(TRestEvent, 1) +}; + +class TRestDetectorSignal { + public: + Int_t fSignalID = -1; + std::vector fSignalTime; + std::vector fSignalCharge; + std::string fName; + std::string fType; + ClassDef(TRestDetectorSignal, 3) +}; + +class TRestDetectorSignalEvent : public TRestEvent { + public: + std::vector fSignal; + ClassDef(TRestDetectorSignalEvent, 1) +}; + +//------------------------------------------------------------------------------ +// Return the ClassDef version of TRestDetectorSignal recorded in the file's +// fSignal sub-branches, or -1 if it cannot be determined. +//------------------------------------------------------------------------------ +static Int_t GetOnDiskSignalVersion(TBranch* branch) { + if (branch == nullptr) return -1; + + auto branchElement = dynamic_cast(branch); + if (branchElement != nullptr) { + std::string name = branch->GetName(); + if (name == "fSignal.fSignalTime" || name == "fSignal.fSignalCharge") + return branchElement->GetClassVersion(); + } + + auto subs = branch->GetListOfBranches(); + for (int i = 0; i <= subs->GetLast(); i++) { + const Int_t version = GetOnDiskSignalVersion((TBranch*)subs->At(i)); + if (version > 0) return version; + } + return -1; +} + +void recoverLegacySignalData(const char* inputFile, const char* outputFile = "") { + // Refuse to run if the real REST libraries are loaded (restRoot session): + // the replica classes above would clash with the compiled ones. + TString loadedLibraries = gSystem->GetLibraries(); + if (loadedLibraries.Contains("libRestFramework") || loadedLibraries.Contains("libRestDetector")) { + std::cout << "ERROR: REST libraries are loaded in this session." << std::endl; + std::cout << "Run this macro with plain root, not restRoot:" << std::endl; + std::cout << " root -l -b -q 'recoverLegacySignalData.C+(\"" << inputFile << "\")'" << std::endl; + return; + } + + TFile* f = TFile::Open(inputFile); + if (f == nullptr || f->IsZombie()) { + std::cout << "ERROR: cannot open input file: " << inputFile << std::endl; + return; + } + + auto eventTree = dynamic_cast(f->Get("EventTree")); + if (eventTree == nullptr) { + std::cout << "ERROR: no EventTree found in " << inputFile << std::endl; + return; + } + + TBranch* signalBranch = eventTree->GetBranch("TRestDetectorSignalEventBranch"); + if (signalBranch == nullptr) { + std::cout << "ERROR: no TRestDetectorSignalEventBranch in EventTree. Nothing to recover." + << std::endl; + return; + } + + const Int_t onDiskVersion = GetOnDiskSignalVersion(signalBranch); + if (onDiskVersion < 0) { + std::cout << "ERROR: could not determine the on-disk TRestDetectorSignal version." << std::endl; + return; + } + if (onDiskVersion >= 4) { + std::cout << "This file already uses the vector layout (TRestDetectorSignal v" + << onDiskVersion << "). Nothing to recover." << std::endl; + return; + } + std::cout << "On-disk TRestDetectorSignal version: " << onDiskVersion << " (legacy float layout)" + << std::endl; + + auto event = new TRestDetectorSignalEvent(); + eventTree->SetBranchAddress("TRestDetectorSignalEventBranch", &event); + + // Output file + std::string outName = outputFile; + if (outName.empty()) { + outName = inputFile; + const size_t pos = outName.rfind(".root"); + if (pos != std::string::npos) outName = outName.substr(0, pos); + outName += "_LegacySignalData.root"; + } + + TFile out(outName.c_str(), "RECREATE"); + TTree dataTree("LegacySignalData", "TRestDetectorSignalEvent data recovered from legacy file"); + + // Event header + Int_t runOrigin, subRunOrigin, eventID, subEventID, timeSec, timeNanoSec; + Bool_t ok; + TString* subEventTag = new TString(); + // Signal data, flattened: per signal an entry in signalID/nPoints; the + // time/charge values of all signals concatenated in order. + std::vector signalID, nPoints; + std::vector times, charges; + + dataTree.Branch("runOrigin", &runOrigin); + dataTree.Branch("subRunOrigin", &subRunOrigin); + dataTree.Branch("eventID", &eventID); + dataTree.Branch("subEventID", &subEventID); + dataTree.Branch("timeSec", &timeSec); + dataTree.Branch("timeNanoSec", &timeNanoSec); + dataTree.Branch("ok", &ok); + dataTree.Branch("subEventTag", &subEventTag); + dataTree.Branch("signalID", &signalID); + dataTree.Branch("nPoints", &nPoints); + dataTree.Branch("times", ×); + dataTree.Branch("charges", &charges); + + const Long64_t nEntries = eventTree->GetEntries(); + Long64_t totalSignals = 0, totalPoints = 0, suspectValues = 0; + + for (Long64_t i = 0; i < nEntries; i++) { + // Read only the signal branch: the other event branches have no + // dictionary in a plain root session. + signalBranch->GetEntry(i); + + runOrigin = event->fRunOrigin; + subRunOrigin = event->fSubRunOrigin; + eventID = event->fEventID; + subEventID = event->fSubEventID; + timeSec = event->fEventTime.GetSec(); + timeNanoSec = event->fEventTime.GetNanoSec(); + ok = event->fOk; + *subEventTag = event->fSubEventTag; + + signalID.clear(); + nPoints.clear(); + times.clear(); + charges.clear(); + + for (const auto& signal : event->fSignal) { + signalID.push_back(signal.fSignalID); + nPoints.push_back((Int_t)signal.fSignalTime.size()); + times.insert(times.end(), signal.fSignalTime.begin(), signal.fSignalTime.end()); + charges.insert(charges.end(), signal.fSignalCharge.begin(), signal.fSignalCharge.end()); + for (const auto v : signal.fSignalTime) + if (std::abs(v) > 1e12) suspectValues++; + } + totalSignals += signalID.size(); + totalPoints += times.size(); + + dataTree.Fill(); + } + + dataTree.Write(); + + // Provenance + TNamed("sourceFile", inputFile).Write(); + TNamed("onDiskSignalVersion", std::to_string(onDiskVersion).c_str()).Write(); + out.Close(); + + std::cout << std::endl; + std::cout << "Recovered " << nEntries << " entries: " << totalSignals << " signals, " << totalPoints + << " points." << std::endl; + if (suspectValues > 0) + std::cout << "WARNING: " << suspectValues + << " suspicious values (|v| > 1e12) found — the recovered data may be corrupted!" + << std::endl; + std::cout << "Signal data written to: " << outName << std::endl; + std::cout << std::endl; + std::cout << "Next step — rebuild the fixed file with restRoot:" << std::endl; + std::cout << " restRoot -b -q 'REST_RebuildLegacySignalFile.C(\"" << inputFile << "\")'" + << std::endl; +} From f50bd1b956c75c964863c1d05a235709456f9f81 Mon Sep 17 00:00:00 2001 From: cmargalejo Date: Fri, 12 Jun 2026 11:25:13 +0200 Subject: [PATCH 04/10] Point legacy detector signal warning to the recovery macros Co-Authored-By: Claude Opus 4.8 --- source/framework/core/src/TRestRun.cxx | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/source/framework/core/src/TRestRun.cxx b/source/framework/core/src/TRestRun.cxx index 08b187e88..791678efc 100644 --- a/source/framework/core/src/TRestRun.cxx +++ b/source/framework/core/src/TRestRun.cxx @@ -99,6 +99,13 @@ void WarnUnsupportedLegacyDetectorSignalBranch() { "still be read, but this detector signal event branch is disabled to avoid excessive " "memory usage or a crash." << RESTendl; + RESTWarning << "The data is recoverable: convert the file once with the macros in " + "$REST_PATH/macros/legacy :" + << RESTendl; + RESTWarning << " 1) root -l -b -q 'recoverLegacySignalData.C+(\"yourFile.root\")' (plain root, NOT " + "restRoot)" + << RESTendl; + RESTWarning << " 2) restRoot -b -q 'REST_RebuildLegacySignalFile.C(\"yourFile.root\")'" << RESTendl; } } // namespace From f2bb02d039642df2b7babf1eed8c00202227b233 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Fri, 12 Jun 2026 09:42:05 +0000 Subject: [PATCH 05/10] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- macros/legacy/REST_RebuildLegacySignalFile.C | 10 ++++------ macros/legacy/recoverLegacySignalData.C | 3 +-- 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/macros/legacy/REST_RebuildLegacySignalFile.C b/macros/legacy/REST_RebuildLegacySignalFile.C index 26af4734c..16c1196c3 100644 --- a/macros/legacy/REST_RebuildLegacySignalFile.C +++ b/macros/legacy/REST_RebuildLegacySignalFile.C @@ -61,8 +61,7 @@ void SetBranchStatusRecursive(TBranch* branch, Bool_t status) { if (branch == nullptr) return; branch->SetStatus(status); auto subs = branch->GetListOfBranches(); - for (int i = 0; i <= subs->GetLast(); i++) - SetBranchStatusRecursive((TBranch*)subs->At(i), status); + for (int i = 0; i <= subs->GetLast(); i++) SetBranchStatusRecursive((TBranch*)subs->At(i), status); } } // namespace REST_Rebuild_Internal @@ -234,8 +233,7 @@ void REST_RebuildLegacySignalFile(const char* originalFile, const char* signalDa TRestDetectorSignal signal; signal.SetSignalID(signalID->at(s)); const size_t n = nPoints->at(s); - for (size_t p = 0; p < n; p++) - signal.NewPoint(times->at(offset + p), charges->at(offset + p)); + for (size_t p = 0; p < n; p++) signal.NewPoint(times->at(offset + p), charges->at(offset + p)); offset += n; event->AddSignal(signal); } @@ -265,8 +263,8 @@ void REST_RebuildLegacySignalFile(const char* originalFile, const char* signalDa if (overwrite) { const std::string backup = std::string(originalFile) + ".bak"; if (gSystem->Rename(originalFile, backup.c_str()) != 0) { - std::cout << "ERROR: could not move original to " << backup << "; fixed file left at " - << outName << std::endl; + std::cout << "ERROR: could not move original to " << backup << "; fixed file left at " << outName + << std::endl; return; } gSystem->Rename(outName.c_str(), originalFile); diff --git a/macros/legacy/recoverLegacySignalData.C b/macros/legacy/recoverLegacySignalData.C index 4bd730932..cbb41d73a 100644 --- a/macros/legacy/recoverLegacySignalData.C +++ b/macros/legacy/recoverLegacySignalData.C @@ -227,6 +227,5 @@ void recoverLegacySignalData(const char* inputFile, const char* outputFile = "") std::cout << "Signal data written to: " << outName << std::endl; std::cout << std::endl; std::cout << "Next step — rebuild the fixed file with restRoot:" << std::endl; - std::cout << " restRoot -b -q 'REST_RebuildLegacySignalFile.C(\"" << inputFile << "\")'" - << std::endl; + std::cout << " restRoot -b -q 'REST_RebuildLegacySignalFile.C(\"" << inputFile << "\")'" << std::endl; } From 98a0f9937cf01fb15c490f8ae8e9152f82c12e19 Mon Sep 17 00:00:00 2001 From: Vindaar Date: Fri, 24 Jul 2026 16:14:12 +0200 Subject: [PATCH 06/10] Decouple legacy signal protection from branch pruning --- source/framework/core/src/TRestRun.cxx | 87 ++++++++++++++++++++------ 1 file changed, 67 insertions(+), 20 deletions(-) diff --git a/source/framework/core/src/TRestRun.cxx b/source/framework/core/src/TRestRun.cxx index 791678efc..e6e07a3a7 100644 --- a/source/framework/core/src/TRestRun.cxx +++ b/source/framework/core/src/TRestRun.cxx @@ -37,6 +37,7 @@ #endif // !WIN32 #include +#include #include @@ -50,42 +51,73 @@ using namespace std; std::mutex mutex_read; namespace { -void DisableBranchRecursively(TBranch* branch) { +void SetBranchStatusRecursively(TBranch* branch, bool status) { if (branch == nullptr) return; - branch->SetStatus(false); + branch->SetStatus(status); auto subs = branch->GetListOfBranches(); for (int i = 0; i <= subs->GetLast(); i++) { - DisableBranchRecursively((TBranch*)subs->At(i)); + SetBranchStatusRecursively((TBranch*)subs->At(i), status); } } -bool BranchHasLegacyDetectorSignalStreamer(TBranch* branch) { - if (branch == nullptr) return false; +Int_t GetOnDiskDetectorSignalVersion(TBranch* branch) { + if (branch == nullptr) return -1; auto branchElement = dynamic_cast(branch); if (branchElement != nullptr && ((std::string)branch->GetName() == "fSignal.fSignalTime" || (std::string)branch->GetName() == "fSignal.fSignalCharge")) { const auto version = branchElement->GetClassVersion(); - if (version > 0 && version < 4) return true; + if (version > 0) return version; } auto subs = branch->GetListOfBranches(); for (int i = 0; i <= subs->GetLast(); i++) { - if (BranchHasLegacyDetectorSignalStreamer((TBranch*)subs->At(i))) return true; + const auto version = GetOnDiskDetectorSignalVersion((TBranch*)subs->At(i)); + if (version > 0) return version; } + return -1; +} + +bool FileHasDetectorSignalStreamerInfo(TFile* inputFile, Int_t version) { + if (inputFile == nullptr || version < 1) return false; + + auto streamerInfos = inputFile->GetStreamerInfoList(); + for (int i = 0; i <= streamerInfos->GetLast(); i++) { + auto streamerInfo = dynamic_cast(streamerInfos->At(i)); + if (streamerInfo != nullptr && (std::string)streamerInfo->GetName() == "TRestDetectorSignal" && + streamerInfo->GetClassVersion() == version) { + return true; + } + } return false; } -bool IsUnsupportedLegacyDetectorSignalBranch(TBranch* branch) { - if (branch == nullptr) return false; +bool IsUnsupportedLegacyDetectorSignalBranch(TFile* inputFile, TBranch* branch) { + if (inputFile == nullptr || branch == nullptr) return false; if ((std::string)branch->GetName() != "TRestDetectorSignalEventBranch") return false; const auto signalClass = TClass::GetClass("TRestDetectorSignal"); if (signalClass == nullptr || signalClass->GetClassVersion() < 4) return false; - return BranchHasLegacyDetectorSignalStreamer(branch); + const auto onDiskVersion = GetOnDiskDetectorSignalVersion(branch); + if (onDiskVersion < 1 || onDiskVersion >= 4) return false; + + // With the old class StreamerInfo ROOT can evolve the legacy float vectors safely. The dangerous + // restManager files are precisely those where the event-class StreamerInfos were omitted. + return !FileHasDetectorSignalStreamerInfo(inputFile, onDiskVersion); +} + +void DisableUnsupportedLegacyDetectorSignalBranch(TTree* eventTree, TBranch* branch) { + if (eventTree == nullptr || branch == nullptr) return; + + // TChain remembers SetBranchStatus rules and reapplies them when it switches files. The two leaf + // rules are needed because disabling a split parent does not reliably disable its sub-branches. + eventTree->SetBranchStatus(branch->GetName(), false); + eventTree->SetBranchStatus("fSignal.fSignalTime", false); + eventTree->SetBranchStatus("fSignal.fSignalCharge", false); + SetBranchStatusRecursively(branch, false); } void WarnUnsupportedLegacyDetectorSignalBranch() { @@ -646,9 +678,21 @@ void TRestRun::ReadInputFileTrees() { fEventTree = _eventTree; } + TObjArray* branches = fEventTree->GetListOfBranches(); + bool warnedAboutLegacyDetectorSignal = false; + for (int i = 0; i <= branches->GetLast(); i++) { + auto branch = (TBranch*)branches->At(i); + if (!IsUnsupportedLegacyDetectorSignalBranch(fInputFile, branch)) continue; + + if (!warnedAboutLegacyDetectorSignal) { + WarnUnsupportedLegacyDetectorSignalBranch(); + warnedAboutLegacyDetectorSignal = true; + } + DisableUnsupportedLegacyDetectorSignalBranch(fEventTree, branch); + } + RESTDebug << "Finding event branch.." << RESTendl; if (fInputEvent == nullptr) { - TObjArray* branches = fEventTree->GetListOfBranches(); // get the last event branch as input event branch if (branches->GetLast() > -1) { TBranch* br = (TBranch*)branches->At(branches->GetLast()); @@ -657,6 +701,8 @@ void TRestRun::ReadInputFileTrees() { RESTInfo << "No event branch inside file : " << filename << RESTendl; RESTInfo << "This file may be a pure analysis file" << RESTendl; } else { + if (IsUnsupportedLegacyDetectorSignalBranch(fInputFile, br)) return; + string type = Replace(br->GetName(), "Branch", "", 0); TClass* cl = TClass::GetClass(type.c_str()); if (cl->HasDictionary()) { @@ -678,13 +724,6 @@ void TRestRun::ReadInputFileTrees() { } fInputEvent->InitializeWithMetadata(this); - if (IsUnsupportedLegacyDetectorSignalBranch(br)) { - WarnUnsupportedLegacyDetectorSignalBranch(); - DisableBranchRecursively(br); - delete fInputEvent; - fInputEvent = nullptr; - return; - } fEventTree->SetBranchAddress(br->GetName(), &fInputEvent); fEventBranchLoc = branches->GetLast(); RESTDebug << "found event branch of event type: " << fInputEvent->ClassName() @@ -699,7 +738,15 @@ void TRestRun::ReadInputFileTrees() { << filename << RESTendl; RESTWarning << "Branch required: " << brname << RESTendl; } else { + auto selectedBranch = fEventTree->GetBranch(brname.c_str()); + if (IsUnsupportedLegacyDetectorSignalBranch(fInputFile, selectedBranch)) { + delete fInputEvent; + fInputEvent = nullptr; + return; + } + fEventTree->SetBranchAddress(brname.c_str(), &fInputEvent); + fEventBranchLoc = branches->IndexOf(selectedBranch); RESTDebug << brname << " is found and set!" << RESTendl; } } @@ -1337,9 +1384,9 @@ void TRestRun::SetInputEvent(TRestEvent* event) { return; } - if (IsUnsupportedLegacyDetectorSignalBranch(selectedBranch)) { + if (IsUnsupportedLegacyDetectorSignalBranch(fInputFile, selectedBranch)) { WarnUnsupportedLegacyDetectorSignalBranch(); - DisableBranchRecursively(selectedBranch); + DisableUnsupportedLegacyDetectorSignalBranch(fEventTree, selectedBranch); return; } From b3aa2ddf6edbf8202dc486f5b09d68633cef8f52 Mon Sep 17 00:00:00 2001 From: Vindaar Date: Thu, 30 Jul 2026 17:04:58 +0200 Subject: [PATCH 07/10] Harden legacy signal recovery and cover unsafe reads Legacy DetectorSignal files store split vector payloads while current detectorlib expects vector. Letting ROOT bind that incompatible schema can turn payload bytes into bogus allocation sizes, so recovery must be explicit and conservative. Ordinary TRestRun reads remain nonmutating: this change does not add automatic repair or rewrite behavior. Resolve input and output paths without throwing, reject normalized, symlink, and hard-link aliases, and use ROOT's CREATE-only mode so recovery outputs cannot replace an existing file. In-place recovery refuses pre-existing temporary and .bak paths. Every replacement move is checked; if installing the fixed file fails after backing up the original, a checked rollback restores it when possible and otherwise reports the exact surviving locations. Track unreadable metadata and event branches without dictionaries. A sibling recovery output may still be produced with explicit warnings, but in-place replacement is refused whenever reconstruction omitted known content, leaving the candidate file for inspection and the original untouched. Add focused filesystem tests for path equivalence, existing-output and backup preservation, successful replacement, and fault-injected backup, replacement, and rollback failures. Add an optional canonical-file integration target covering unsafe legacy detection, all-entry reads with the branch disabled, and preservation of the current event after legacy or invalid target selection. --- macros/legacy/LegacyRecoveryFileUtils.h | 178 +++++++++++++++ macros/legacy/REST_RebuildLegacySignalFile.C | 58 +++-- macros/legacy/recoverLegacySignalData.C | 28 ++- source/CMakeLists.txt | 39 ++++ .../test/integration/LegacyDetectorSignal.cxx | 119 ++++++++++ .../test/src/LegacyRecoveryFileUtils.cxx | 216 ++++++++++++++++++ 6 files changed, 615 insertions(+), 23 deletions(-) create mode 100644 macros/legacy/LegacyRecoveryFileUtils.h create mode 100644 source/framework/test/integration/LegacyDetectorSignal.cxx create mode 100644 source/framework/test/src/LegacyRecoveryFileUtils.cxx diff --git a/macros/legacy/LegacyRecoveryFileUtils.h b/macros/legacy/LegacyRecoveryFileUtils.h new file mode 100644 index 000000000..9980f1743 --- /dev/null +++ b/macros/legacy/LegacyRecoveryFileUtils.h @@ -0,0 +1,178 @@ +#ifndef REST_LEGACY_RECOVERY_FILE_UTILS_H +#define REST_LEGACY_RECOVERY_FILE_UTILS_H + +#include +#include +#include +#include +#include + +namespace REST_LegacyRecovery { + +namespace fs = std::filesystem; + +struct PathComparison { + bool ok = false; + bool equivalent = false; + std::string error; +}; + +inline PathComparison ComparePaths(const fs::path& first, const fs::path& second) { + std::error_code firstError; + const auto firstAbsolute = fs::absolute(first, firstError); + if (firstError) { + return {false, false, "cannot resolve '" + first.string() + "': " + firstError.message()}; + } + const auto firstPath = fs::weakly_canonical(firstAbsolute, firstError); + if (firstError) { + return {false, false, "cannot resolve '" + first.string() + "': " + firstError.message()}; + } + + std::error_code secondError; + const auto secondAbsolute = fs::absolute(second, secondError); + if (secondError) { + return {false, false, "cannot resolve '" + second.string() + "': " + secondError.message()}; + } + const auto secondPath = fs::weakly_canonical(secondAbsolute, secondError); + if (secondError) { + return {false, false, "cannot resolve '" + second.string() + "': " + secondError.message()}; + } + + if (firstPath == secondPath) return {true, true, ""}; + + std::error_code firstExistsError; + const bool firstExists = fs::exists(firstPath, firstExistsError); + if (firstExistsError) { + return {false, false, "cannot inspect '" + first.string() + "': " + firstExistsError.message()}; + } + + std::error_code secondExistsError; + const bool secondExists = fs::exists(secondPath, secondExistsError); + if (secondExistsError) { + return {false, false, "cannot inspect '" + second.string() + "': " + secondExistsError.message()}; + } + + if (firstExists && secondExists) { + std::error_code equivalentError; + const bool equivalent = fs::equivalent(firstPath, secondPath, equivalentError); + if (equivalentError) { + return {false, false, + "cannot compare '" + first.string() + "' and '" + second.string() + + "': " + equivalentError.message()}; + } + if (equivalent) return {true, true, ""}; + } + + return {true, false, ""}; +} + +inline bool PathEntryExists(const fs::path& path, bool& exists, std::string& error) { + std::error_code statusError; + const auto status = fs::symlink_status(path, statusError); + if (status.type() == fs::file_type::not_found) { + exists = false; + return true; + } + if (statusError) { + error = "cannot inspect '" + path.string() + "': " + statusError.message(); + return false; + } + + exists = true; + return true; +} + +inline bool ValidateUnusedPath(const fs::path& path, const std::string& description, std::ostream& errors) { + bool pathExists = false; + std::string existenceError; + if (!PathEntryExists(path, pathExists, existenceError)) { + errors << "ERROR: " << existenceError << '\n'; + return false; + } + if (pathExists) { + errors << "ERROR: " << description << " already exists: " << path.string() + << "\nChoose a different path or move the existing file first.\n"; + return false; + } + return true; +} + +inline bool ValidateNewOutputPath(const fs::path& inputPath, const fs::path& outputPath, + const std::string& description, std::ostream& errors) { + const auto comparison = ComparePaths(inputPath, outputPath); + if (!comparison.ok) { + errors << "ERROR: " << comparison.error << '\n'; + return false; + } + if (comparison.equivalent) { + errors << "ERROR: " << description << " resolves to the input file. Refusing to overwrite '" + << inputPath.string() << "'.\n"; + return false; + } + + return ValidateUnusedPath(outputPath, description, errors); +} + +using RenameOperation = std::function; + +inline void RenamePath(const fs::path& source, const fs::path& destination, std::error_code& error) { + fs::rename(source, destination, error); +} + +inline bool ReplaceFileWithBackup(const fs::path& replacementPath, const fs::path& originalPath, + const fs::path& backupPath, std::ostream& errors, + const RenameOperation& renameOperation = RenamePath) { + const auto replacementComparison = ComparePaths(replacementPath, originalPath); + if (!replacementComparison.ok) { + errors << "ERROR: " << replacementComparison.error << '\n'; + return false; + } + if (replacementComparison.equivalent) { + errors << "ERROR: replacement and original resolve to the same path: " << originalPath.string() + << '\n'; + return false; + } + + bool backupExists = false; + std::string existenceError; + if (!PathEntryExists(backupPath, backupExists, existenceError)) { + errors << "ERROR: " << existenceError << '\n'; + return false; + } + if (backupExists) { + errors << "ERROR: backup path already exists: " << backupPath.string() + << "\nRefusing to overwrite it. Move or rename that backup and retry.\n"; + return false; + } + + std::error_code backupError; + renameOperation(originalPath, backupPath, backupError); + if (backupError) { + errors << "ERROR: could not move original file '" << originalPath.string() << "' to backup '" + << backupPath.string() << "': " << backupError.message() << '\n'; + return false; + } + + std::error_code replacementError; + renameOperation(replacementPath, originalPath, replacementError); + if (!replacementError) return true; + + errors << "ERROR: original was backed up, but moving fixed file '" << replacementPath.string() + << "' into place failed: " << replacementError.message() << '\n'; + + std::error_code rollbackError; + renameOperation(backupPath, originalPath, rollbackError); + if (!rollbackError) { + errors << "Rollback succeeded: the original file was restored. The fixed file remains at '" + << replacementPath.string() << "'.\n"; + } else { + errors << "CRITICAL: rollback also failed: " << rollbackError.message() + << "\nThe original remains at '" << backupPath.string() << "' and the fixed file remains at '" + << replacementPath.string() << "'.\n"; + } + return false; +} + +} // namespace REST_LegacyRecovery + +#endif diff --git a/macros/legacy/REST_RebuildLegacySignalFile.C b/macros/legacy/REST_RebuildLegacySignalFile.C index 16c1196c3..545cf49d3 100644 --- a/macros/legacy/REST_RebuildLegacySignalFile.C +++ b/macros/legacy/REST_RebuildLegacySignalFile.C @@ -17,6 +17,13 @@ // REST_RebuildLegacySignalFile("input.root", "", "", true) // overwrites input.root in place (the original is kept as input.root.bak) // +// The macro never overwrites an existing output or backup. In-place recovery +// refuses to start while input.root.bak or its temporary fixed file exists. +// Event branches without a loaded dictionary and metadata objects unreadable +// with the current libraries cannot be copied. They are reported explicitly; +// if any are encountered, in-place recovery is refused and the fixed file is +// left at _FixedTmp.root for inspection. +// // Arguments: // originalFile - the legacy REST file // signalDataFile - intermediate from stage 1; default: _LegacySignalData.root @@ -29,7 +36,6 @@ #include #include #include -#include #include #include @@ -37,6 +43,8 @@ #include #include +#include "LegacyRecoveryFileUtils.h" + namespace REST_Rebuild_Internal { Int_t GetOnDiskSignalVersion(TBranch* branch) { @@ -81,6 +89,18 @@ void REST_RebuildLegacySignalFile(const char* originalFile, const char* signalDa if (outName.empty()) outName = base + "_Fixed.root"; if (overwrite) outName = base + "_FixedTmp.root"; + if (!REST_LegacyRecovery::ValidateNewOutputPath(originalFile, outName, "fixed output", std::cout)) { + return; + } + if (!REST_LegacyRecovery::ValidateNewOutputPath(dataName, outName, "fixed output", std::cout)) { + return; + } + + const std::string backupName = std::string(originalFile) + ".bak"; + if (overwrite && !REST_LegacyRecovery::ValidateUnusedPath(backupName, "backup path", std::cout)) { + return; + } + // --- open inputs --- TFile* original = TFile::Open(originalFile); if (original == nullptr || original->IsZombie()) { @@ -143,7 +163,7 @@ void REST_RebuildLegacySignalFile(const char* originalFile, const char* signalDa dataTree->SetBranchAddress("charges", &charges); // --- output file (single write session: StreamerInfos are preserved) --- - TFile* out = TFile::Open(outName.c_str(), "RECREATE"); + TFile* out = TFile::Open(outName.c_str(), "CREATE"); if (out == nullptr || out->IsZombie()) { std::cout << "ERROR: cannot create output file: " << outName << std::endl; return; @@ -181,6 +201,7 @@ void REST_RebuildLegacySignalFile(const char* originalFile, const char* signalDa SetBranchStatusRecursive(signalBranch, 0); std::vector otherBranchNames; + std::vector skippedEventBranches; TIter nextBranch(oldTree->GetListOfBranches()); TBranch* br; while ((br = (TBranch*)nextBranch())) { @@ -191,6 +212,7 @@ void REST_RebuildLegacySignalFile(const char* originalFile, const char* signalDa if (TClass::GetClass(className.c_str()) == nullptr) { std::cout << "WARNING: no dictionary for event class '" << className << "'; branch '" << name << "' will NOT be copied!" << std::endl; + skippedEventBranches.push_back(name + " (" + className + ")"); SetBranchStatusRecursive(br, 0); continue; } @@ -258,28 +280,36 @@ void REST_RebuildLegacySignalFile(const char* originalFile, const char* signalDa original->Close(); data->Close(); + if (!skipped.empty()) { + std::cout << "WARNING: " << skipped.size() + << " metadata key(s) could not be read with the current libraries and were NOT copied:" + << std::endl; + for (const auto& s : skipped) std::cout << " - " << s << std::endl; + } + if (!skippedEventBranches.empty()) { + std::cout << "WARNING: " << skippedEventBranches.size() + << " event branch(es) had no loaded dictionary and were NOT copied:" << std::endl; + for (const auto& s : skippedEventBranches) std::cout << " - " << s << std::endl; + } + // --- overwrite handling --- std::string finalName = outName; if (overwrite) { - const std::string backup = std::string(originalFile) + ".bak"; - if (gSystem->Rename(originalFile, backup.c_str()) != 0) { - std::cout << "ERROR: could not move original to " << backup << "; fixed file left at " << outName - << std::endl; + if (!skipped.empty() || !skippedEventBranches.empty()) { + std::cout << "ERROR: refusing in-place replacement because the rebuilt file has omitted " + "content.\n" + << "The original is unchanged. Inspect the candidate file at: " << outName << std::endl; + return; + } + if (!REST_LegacyRecovery::ReplaceFileWithBackup(outName, originalFile, backupName, std::cout)) { return; } - gSystem->Rename(outName.c_str(), originalFile); finalName = originalFile; - std::cout << "Original file kept as: " << backup << std::endl; + std::cout << "Original file kept as: " << backupName << std::endl; } std::cout << std::endl; std::cout << "Rebuilt " << nEntries << " entries: " << totalSignals << " signals, " << totalPoints << " points." << std::endl; - if (!skipped.empty()) { - std::cout << "WARNING: " << skipped.size() - << " metadata key(s) could not be read with the current libraries and were NOT copied:" - << std::endl; - for (const auto& s : skipped) std::cout << " - " << s << std::endl; - } std::cout << "Fixed file written to: " << finalName << std::endl; } diff --git a/macros/legacy/recoverLegacySignalData.C b/macros/legacy/recoverLegacySignalData.C index cbb41d73a..bc0b5ed59 100644 --- a/macros/legacy/recoverLegacySignalData.C +++ b/macros/legacy/recoverLegacySignalData.C @@ -21,6 +21,7 @@ // // The intermediate file is written next to the input as // _LegacySignalData.root unless an explicit output path is given. +// Existing output files are never overwritten. // // This file is deliberately NOT named REST_*.C so that restRoot's --m macro // loading does not interpret it (the replica class definitions below would @@ -39,6 +40,8 @@ #include #include +#include "LegacyRecoveryFileUtils.h" + //------------------------------------------------------------------------------ // Replica classes matching the legacy on-disk layout (REST <= v2.4.2). // TRestDetectorSignal uses the v3 layout (fName/fType added Aug 2023); files @@ -105,6 +108,18 @@ void recoverLegacySignalData(const char* inputFile, const char* outputFile = "") return; } + std::string outName = outputFile; + if (outName.empty()) { + outName = inputFile; + const size_t pos = outName.rfind(".root"); + if (pos != std::string::npos) outName = outName.substr(0, pos); + outName += "_LegacySignalData.root"; + } + if (!REST_LegacyRecovery::ValidateNewOutputPath(inputFile, outName, "legacy signal data output", + std::cout)) { + return; + } + TFile* f = TFile::Open(inputFile); if (f == nullptr || f->IsZombie()) { std::cout << "ERROR: cannot open input file: " << inputFile << std::endl; @@ -140,16 +155,11 @@ void recoverLegacySignalData(const char* inputFile, const char* outputFile = "") auto event = new TRestDetectorSignalEvent(); eventTree->SetBranchAddress("TRestDetectorSignalEventBranch", &event); - // Output file - std::string outName = outputFile; - if (outName.empty()) { - outName = inputFile; - const size_t pos = outName.rfind(".root"); - if (pos != std::string::npos) outName = outName.substr(0, pos); - outName += "_LegacySignalData.root"; + TFile out(outName.c_str(), "CREATE"); + if (out.IsZombie()) { + std::cout << "ERROR: cannot create output file: " << outName << std::endl; + return; } - - TFile out(outName.c_str(), "RECREATE"); TTree dataTree("LegacySignalData", "TRestDetectorSignalEvent data recovered from legacy file"); // Event header diff --git a/source/CMakeLists.txt b/source/CMakeLists.txt index a238382ec..1795a4d65 100644 --- a/source/CMakeLists.txt +++ b/source/CMakeLists.txt @@ -102,6 +102,45 @@ foreach (dir ${dirs}) endforeach () endforeach () +set(BUILD_LEGACY_SIGNAL_TEST ${TEST}) +if (NOT TARGET RestDetector) + set(BUILD_LEGACY_SIGNAL_TEST OFF) +endif () +if (NOT TARGET RestRaw) + set(BUILD_LEGACY_SIGNAL_TEST OFF) +endif () +if (NOT TARGET RestTrack) + set(BUILD_LEGACY_SIGNAL_TEST OFF) +endif () + +if (BUILD_LEGACY_SIGNAL_TEST) + set( + REST_LEGACY_SIGNAL_TEST_FILE + "" + CACHE FILEPATH + "Optional canonical legacy DetectorSignal ROOT file for integration tests" + ) + set(LEGACY_SIGNAL_TEST_SOURCE + framework/test/integration/LegacyDetectorSignal.cxx) + add_executable(testLegacyDetectorSignal ${LEGACY_SIGNAL_TEST_SOURCE}) + target_include_directories( + testLegacyDetectorSignal + PRIVATE framework/external/tinyxml framework/tools/inc + framework/core/inc libraries/detector/inc libraries/raw/inc) + target_compile_definitions( + testLegacyDetectorSignal + PRIVATE + REST_LEGACY_SIGNAL_TEST_FILE="${REST_LEGACY_SIGNAL_TEST_FILE}") + target_link_libraries( + testLegacyDetectorSignal + PRIVATE RestDetector RestRaw RestTrack RestFramework gtest_main + stdc++fs) + include(GoogleTest) + gtest_add_tests( + TARGET testLegacyDetectorSignal SOURCES ${LEGACY_SIGNAL_TEST_SOURCE}) +endif () +unset(BUILD_LEGACY_SIGNAL_TEST) + # remove duplicates if (DEFINED rest_include_dirs) list(REMOVE_DUPLICATES rest_include_dirs) diff --git a/source/framework/test/integration/LegacyDetectorSignal.cxx b/source/framework/test/integration/LegacyDetectorSignal.cxx new file mode 100644 index 000000000..3b6497150 --- /dev/null +++ b/source/framework/test/integration/LegacyDetectorSignal.cxx @@ -0,0 +1,119 @@ +#include +#include +#include +#include +#include +#include + +#include +#include +#include +#include + +namespace { + +namespace fs = std::filesystem; + +class UnknownEvent : public TRestRawSignalEvent { + public: + const char* ClassName() const override { return "UnknownEvent"; } +}; + +TBranch* FindBranch(TBranch* branch, const std::string& name) { + if (branch == nullptr) return nullptr; + if (branch->GetName() == name) return branch; + + auto children = branch->GetListOfBranches(); + for (int i = 0; i <= children->GetLast(); i++) { + auto found = FindBranch((TBranch*)children->At(i), name); + if (found != nullptr) return found; + } + return nullptr; +} + +bool IsEnabled(TBranch* branch) { return branch != nullptr && !branch->TestBit(::kDoNotProcess); } + +fs::path GetLegacySignalTestFile() { + const char* environmentFile = std::getenv("REST_LEGACY_SIGNAL_TEST_FILE"); + if (environmentFile != nullptr && environmentFile[0] != '\0') return environmentFile; + return REST_LEGACY_SIGNAL_TEST_FILE; +} + +class LegacyDetectorSignalTest : public ::testing::Test { + protected: + void SetUp() override { + filename = GetLegacySignalTestFile(); + if (filename.empty() || !fs::is_regular_file(filename)) { + GTEST_SKIP() << "Set REST_LEGACY_SIGNAL_TEST_FILE to the canonical legacy Signal ROOT file"; + } + + ASSERT_GE(gSystem->Load("libRestDetector.so"), 0); + ASSERT_GE(gSystem->Load("libRestRaw.so"), 0); + ASSERT_GE(gSystem->Load("libRestTrack.so"), 0); + } + + void AssertLegacySignalDisabled(TRestRun& run) { + auto signalBranch = run.GetEventTree()->GetBranch("TRestDetectorSignalEventBranch"); + ASSERT_NE(signalBranch, nullptr); + auto timeBranch = FindBranch(signalBranch, "fSignal.fSignalTime"); + auto chargeBranch = FindBranch(signalBranch, "fSignal.fSignalCharge"); + ASSERT_NE(timeBranch, nullptr); + ASSERT_NE(chargeBranch, nullptr); + EXPECT_FALSE(IsEnabled(signalBranch)); + EXPECT_FALSE(IsEnabled(timeBranch)); + EXPECT_FALSE(IsEnabled(chargeBranch)); + } + + fs::path filename; +}; + +TEST_F(LegacyDetectorSignalTest, DetectsAndDisablesUnsafeBranch) { + TRestRun run; + run.OpenInputFile(filename.string().c_str()); + + ASSERT_NE(run.GetInputEvent(), nullptr); + EXPECT_STREQ(run.GetInputEvent()->ClassName(), "TRestTrackEvent"); + AssertLegacySignalDisabled(run); + + for (Long64_t entry = 0; entry < run.GetEntries(); entry++) run.GetEntry(entry); +} + +TEST_F(LegacyDetectorSignalTest, RejectedLegacySelectionPreservesCurrentEvent) { + TRestRun run; + run.OpenInputFile(filename.string().c_str()); + run.GetEntry(0); + AssertLegacySignalDisabled(run); + + auto currentEvent = run.GetInputEvent(); + ASSERT_NE(currentEvent, nullptr); + const std::string currentType = currentEvent->ClassName(); + + auto requestedEvent = std::make_unique(); + auto requestedEventPointer = requestedEvent.get(); + run.SetInputEvent(requestedEventPointer); + if (run.GetInputEvent() == requestedEventPointer) requestedEvent.release(); + + EXPECT_EQ(run.GetInputEvent(), currentEvent); + EXPECT_EQ(std::string(run.GetInputEvent()->ClassName()), currentType); + AssertLegacySignalDisabled(run); +} + +TEST_F(LegacyDetectorSignalTest, InvalidSelectionPreservesCurrentEvent) { + TRestRun run; + run.OpenInputFile(filename.string().c_str()); + run.GetEntry(0); + + auto currentEvent = run.GetInputEvent(); + ASSERT_NE(currentEvent, nullptr); + const std::string currentType = currentEvent->ClassName(); + + auto requestedEvent = std::make_unique(); + auto requestedEventPointer = requestedEvent.get(); + run.SetInputEvent(requestedEventPointer); + if (run.GetInputEvent() == requestedEventPointer) requestedEvent.release(); + + EXPECT_EQ(run.GetInputEvent(), currentEvent); + EXPECT_EQ(std::string(run.GetInputEvent()->ClassName()), currentType); +} + +} // namespace diff --git a/source/framework/test/src/LegacyRecoveryFileUtils.cxx b/source/framework/test/src/LegacyRecoveryFileUtils.cxx new file mode 100644 index 000000000..f0a5feeb3 --- /dev/null +++ b/source/framework/test/src/LegacyRecoveryFileUtils.cxx @@ -0,0 +1,216 @@ +#include "../../../../macros/legacy/LegacyRecoveryFileUtils.h" + +#include + +#include +#include +#include +#include +#include +#include +#include + +namespace { + +namespace fs = std::filesystem; +using REST_LegacyRecovery::ComparePaths; +using REST_LegacyRecovery::ReplaceFileWithBackup; +using REST_LegacyRecovery::ValidateNewOutputPath; + +class TemporaryDirectory { + public: + TemporaryDirectory() { + static std::atomic sequence{0}; + const auto suffix = + std::to_string(std::chrono::high_resolution_clock::now().time_since_epoch().count()) + "_" + + std::to_string(sequence++); + path = fs::temp_directory_path() / ("rest_legacy_recovery_test_" + suffix); + fs::create_directories(path); + } + + ~TemporaryDirectory() { + std::error_code error; + fs::remove_all(path, error); + } + + fs::path path; +}; + +void WriteText(const fs::path& path, const std::string& text) { + std::ofstream output(path); + ASSERT_TRUE(output.is_open()); + output << text; +} + +std::string ReadText(const fs::path& path) { + std::ifstream input(path); + return {std::istreambuf_iterator(input), std::istreambuf_iterator()}; +} + +TEST(LegacyRecoveryFileUtils, RejectsEquivalentAndExistingOutputPaths) { + TemporaryDirectory temporary; + const auto input = temporary.path / "input.root"; + const auto existingOutput = temporary.path / "existing.root"; + const auto newOutput = temporary.path / "new.root"; + const auto nested = temporary.path / "nested"; + fs::create_directory(nested); + WriteText(input, "input"); + WriteText(existingOutput, "existing"); + + std::ostringstream errors; + EXPECT_FALSE(ValidateNewOutputPath(input, input, "output", errors)); + + errors.str(""); + EXPECT_FALSE(ValidateNewOutputPath(input, nested / ".." / "input.root", "output", errors)); + + std::error_code symlinkError; + const auto symlink = temporary.path / "input-link.root"; + fs::create_symlink(input, symlink, symlinkError); + if (!symlinkError) { + errors.str(""); + EXPECT_FALSE(ValidateNewOutputPath(input, symlink, "output", errors)); + } + + std::error_code hardLinkError; + const auto hardLink = temporary.path / "input-hard-link.root"; + fs::create_hard_link(input, hardLink, hardLinkError); + if (!hardLinkError) { + errors.str(""); + EXPECT_FALSE(ValidateNewOutputPath(input, hardLink, "output", errors)); + } + + errors.str(""); + EXPECT_FALSE(ValidateNewOutputPath(input, existingOutput, "output", errors)); + EXPECT_EQ(ReadText(existingOutput), "existing"); + + errors.str(""); + EXPECT_TRUE(ValidateNewOutputPath(input, newOutput, "output", errors)); + EXPECT_FALSE(fs::exists(newOutput)); +} + +TEST(LegacyRecoveryFileUtils, ReplacesOriginalAndKeepsBackup) { + TemporaryDirectory temporary; + const auto original = temporary.path / "input.root"; + const auto replacement = temporary.path / "fixed.root"; + const auto backup = temporary.path / "input.root.bak"; + WriteText(original, "original"); + WriteText(replacement, "fixed"); + + std::ostringstream errors; + EXPECT_TRUE(ReplaceFileWithBackup(replacement, original, backup, errors)); + EXPECT_EQ(ReadText(original), "fixed"); + EXPECT_EQ(ReadText(backup), "original"); + EXPECT_FALSE(fs::exists(replacement)); + EXPECT_TRUE(errors.str().empty()); +} + +TEST(LegacyRecoveryFileUtils, RefusesExistingBackupWithoutChangingFiles) { + TemporaryDirectory temporary; + const auto original = temporary.path / "input.root"; + const auto replacement = temporary.path / "fixed.root"; + const auto backup = temporary.path / "input.root.bak"; + WriteText(original, "original"); + WriteText(replacement, "fixed"); + WriteText(backup, "previous backup"); + + std::ostringstream errors; + EXPECT_FALSE(ReplaceFileWithBackup(replacement, original, backup, errors)); + EXPECT_EQ(ReadText(original), "original"); + EXPECT_EQ(ReadText(replacement), "fixed"); + EXPECT_EQ(ReadText(backup), "previous backup"); + EXPECT_NE(errors.str().find("Refusing to overwrite"), std::string::npos); +} + +TEST(LegacyRecoveryFileUtils, PreservesFilesWhenBackupMoveFails) { + TemporaryDirectory temporary; + const auto original = temporary.path / "input.root"; + const auto replacement = temporary.path / "fixed.root"; + const auto backup = temporary.path / "input.root.bak"; + WriteText(original, "original"); + WriteText(replacement, "fixed"); + + int renameCalls = 0; + const auto failBackup = [&renameCalls](const fs::path&, const fs::path&, std::error_code& error) { + renameCalls++; + error = std::make_error_code(std::errc::permission_denied); + }; + + std::ostringstream errors; + EXPECT_FALSE(ReplaceFileWithBackup(replacement, original, backup, errors, failBackup)); + EXPECT_EQ(renameCalls, 1); + EXPECT_EQ(ReadText(original), "original"); + EXPECT_EQ(ReadText(replacement), "fixed"); + EXPECT_FALSE(fs::exists(backup)); + EXPECT_NE(errors.str().find("could not move original"), std::string::npos); +} + +TEST(LegacyRecoveryFileUtils, RestoresOriginalWhenReplacementMoveFails) { + TemporaryDirectory temporary; + const auto original = temporary.path / "input.root"; + const auto replacement = temporary.path / "fixed.root"; + const auto backup = temporary.path / "input.root.bak"; + WriteText(original, "original"); + WriteText(replacement, "fixed"); + + int renameCalls = 0; + const auto failReplacement = [&renameCalls](const fs::path& source, const fs::path& destination, + std::error_code& error) { + renameCalls++; + if (renameCalls == 2) { + error = std::make_error_code(std::errc::permission_denied); + return; + } + fs::rename(source, destination, error); + }; + + std::ostringstream errors; + EXPECT_FALSE(ReplaceFileWithBackup(replacement, original, backup, errors, failReplacement)); + EXPECT_EQ(renameCalls, 3); + EXPECT_EQ(ReadText(original), "original"); + EXPECT_EQ(ReadText(replacement), "fixed"); + EXPECT_FALSE(fs::exists(backup)); + EXPECT_NE(errors.str().find("Rollback succeeded"), std::string::npos); +} + +TEST(LegacyRecoveryFileUtils, ReportsLocationsWhenReplacementAndRollbackFail) { + TemporaryDirectory temporary; + const auto original = temporary.path / "input.root"; + const auto replacement = temporary.path / "fixed.root"; + const auto backup = temporary.path / "input.root.bak"; + WriteText(original, "original"); + WriteText(replacement, "fixed"); + + int renameCalls = 0; + const auto failReplacementAndRollback = + [&renameCalls](const fs::path& source, const fs::path& destination, std::error_code& error) { + renameCalls++; + if (renameCalls > 1) { + error = std::make_error_code(std::errc::permission_denied); + return; + } + fs::rename(source, destination, error); + }; + + std::ostringstream errors; + EXPECT_FALSE(ReplaceFileWithBackup(replacement, original, backup, errors, failReplacementAndRollback)); + EXPECT_EQ(renameCalls, 3); + EXPECT_FALSE(fs::exists(original)); + EXPECT_EQ(ReadText(backup), "original"); + EXPECT_EQ(ReadText(replacement), "fixed"); + EXPECT_NE(errors.str().find("CRITICAL"), std::string::npos); + EXPECT_NE(errors.str().find(backup.string()), std::string::npos); + EXPECT_NE(errors.str().find(replacement.string()), std::string::npos); +} + +TEST(LegacyRecoveryFileUtils, ComparesNonexistentDestinationsWithoutThrowing) { + TemporaryDirectory temporary; + const auto first = temporary.path / "does-not-exist.root"; + const auto second = temporary.path / "also-does-not-exist.root"; + + const auto comparison = ComparePaths(first, second); + EXPECT_TRUE(comparison.ok); + EXPECT_FALSE(comparison.equivalent); + EXPECT_TRUE(comparison.error.empty()); +} + +} // namespace From 44c9d7f73efe2a05d25ffe1edce58856cbd2ae0a Mon Sep 17 00:00:00 2001 From: Vindaar Date: Thu, 30 Jul 2026 18:22:51 +0200 Subject: [PATCH 08/10] Authenticate and validate legacy signal recovery Bind stage-one output to the exact normalized source identity and persist that provenance through the rebuilt result. Reject unknown or inconsistent legacy schemas and malformed flattened signal arrays before creating output. The identity metadata is deliberately an integrity guard for this workflow rather than a cryptographic authentication mechanism. Check every binding, write, flush, and close operation, then reopen rebuilt candidates and validate provenance, tree and branch structure, the current signal schema, and recovered counts before any in-place rename. This keeps the original and any existing backup untouched when validation fails. Add negative coverage for schema, provenance, array corruption, and candidate replacement failures, plus an actual ROOT candidate-readback test. --- .../LegacyRecoveryCandidateValidation.h | 194 ++++++++++ macros/legacy/LegacyRecoveryDataUtils.h | 203 ++++++++++ macros/legacy/LegacyRecoveryFileUtils.h | 57 ++- macros/legacy/LegacyRecoveryProvenance.h | 199 ++++++++++ macros/legacy/REST_RebuildLegacySignalFile.C | 346 ++++++++++++++---- macros/legacy/recoverLegacySignalData.C | 244 +++++++++--- .../test/integration/LegacyDetectorSignal.cxx | 85 +++++ .../test/src/LegacyRecoveryFileUtils.cxx | 188 ++++++++++ 8 files changed, 1378 insertions(+), 138 deletions(-) create mode 100644 macros/legacy/LegacyRecoveryCandidateValidation.h create mode 100644 macros/legacy/LegacyRecoveryDataUtils.h create mode 100644 macros/legacy/LegacyRecoveryProvenance.h diff --git a/macros/legacy/LegacyRecoveryCandidateValidation.h b/macros/legacy/LegacyRecoveryCandidateValidation.h new file mode 100644 index 000000000..35bb15f7b --- /dev/null +++ b/macros/legacy/LegacyRecoveryCandidateValidation.h @@ -0,0 +1,194 @@ +#ifndef REST_LEGACY_RECOVERY_CANDIDATE_VALIDATION_H +#define REST_LEGACY_RECOVERY_CANDIDATE_VALIDATION_H + +#include +#include +#include +#include + +#include +#include +#include +#include +#include +#include + +#include "LegacyRecoveryDataUtils.h" +#include "LegacyRecoveryProvenance.h" + +namespace REST_LegacyRecovery { + +namespace CandidateValidationDetail { + +inline void CollectSignalSchemaVersions(TBranch* branch, SignalSchemaVersions& versions, bool& timeSeen, + bool& chargeSeen) { + if (branch == nullptr) return; + + auto branchElement = dynamic_cast(branch); + if (branchElement != nullptr) { + const std::string name = branch->GetName(); + const int version = branchElement->GetClassVersion(); + if (name == "fSignal.fSignalTime") { + versions.time = timeSeen && versions.time != version ? -2 : version; + timeSeen = true; + } else if (name == "fSignal.fSignalCharge") { + versions.charge = chargeSeen && versions.charge != version ? -2 : version; + chargeSeen = true; + } + } + + auto children = branch->GetListOfBranches(); + for (int index = 0; index <= children->GetLast(); ++index) { + CollectSignalSchemaVersions(static_cast(children->At(index)), versions, timeSeen, + chargeSeen); + } +} + +} // namespace CandidateValidationDetail + +inline SignalSchemaVersions DetectSignalSchemaVersions(TBranch* branch) { + SignalSchemaVersions versions; + bool timeSeen = false; + bool chargeSeen = false; + CandidateValidationDetail::CollectSignalSchemaVersions(branch, versions, timeSeen, chargeSeen); + if (!timeSeen) versions.time = -1; + if (!chargeSeen) versions.charge = -1; + return versions; +} + +struct CandidateExpectations { + RecoveryProvenance provenance; + Long64_t analysisEntries = -1; + std::vector metadataKeys; + std::vector otherEventBranches; + std::vector analysisBranches; +}; + +inline bool ValidateRecoveryCandidate(const std::filesystem::path& candidatePath, + const CandidateExpectations& expected, std::ostream& errors) { + std::unique_ptr candidate(TFile::Open(candidatePath.string().c_str(), "READ")); + if (candidate == nullptr || candidate->IsZombie()) { + errors << "ERROR: cannot reopen rebuilt candidate: " << candidatePath.string() << ".\n"; + return false; + } + if (candidate->TestBit(TFile::kRecovered)) { + errors << "ERROR: rebuilt candidate required ROOT file recovery and is not safe to install.\n"; + return false; + } + + RecoveryProvenance actualProvenance; + if (!ReadRecoveryProvenance(*candidate, actualProvenance, errors) || + !ValidateResultProvenance(actualProvenance, expected.provenance, candidate->GetUUID().AsString(), + errors)) { + return false; + } + + for (const auto& key : expected.metadataKeys) { + if (candidate->GetKey(key.c_str()) == nullptr) { + errors << "ERROR: rebuilt candidate is missing copied metadata key '" << key << "'.\n"; + return false; + } + } + + auto eventTree = dynamic_cast(candidate->Get("EventTree")); + if (eventTree == nullptr) { + errors << "ERROR: rebuilt candidate has no readable EventTree.\n"; + return false; + } + if (eventTree->GetEntries() != static_cast(expected.provenance.recovered.entries)) { + errors << "ERROR: rebuilt candidate EventTree has " << eventTree->GetEntries() + << " entries, expected " << expected.provenance.recovered.entries << ".\n"; + return false; + } + + for (const auto& branch : expected.otherEventBranches) { + if (eventTree->GetBranch(branch.c_str()) == nullptr) { + errors << "ERROR: rebuilt candidate is missing copied event branch '" << branch << "'.\n"; + return false; + } + } + + auto signalBranch = eventTree->GetBranch("TRestDetectorSignalEventBranch"); + if (signalBranch == nullptr) { + errors << "ERROR: rebuilt candidate has no TRestDetectorSignalEventBranch.\n"; + return false; + } + const auto schema = DetectSignalSchemaVersions(signalBranch); + if (!ValidateCurrentSignalSchema(schema, errors, "rebuilt candidate")) return false; + + auto eventOwner = std::make_unique(); + TRestDetectorSignalEvent* event = eventOwner.get(); + const int bindStatus = eventTree->SetBranchAddress("TRestDetectorSignalEventBranch", &event); + if (bindStatus < TTree::kMatch) { + errors << "ERROR: cannot bind rebuilt candidate signal branch (status " << bindStatus << ").\n"; + return false; + } + + RecoveryCounts actualCounts; + actualCounts.entries = static_cast(eventTree->GetEntries()); + for (Long64_t entry = 0; entry < eventTree->GetEntries(); ++entry) { + if (signalBranch->GetEntry(entry) <= 0 || event == nullptr) { + errors << "ERROR: cannot read rebuilt signal event at entry " << entry << ".\n"; + eventTree->ResetBranchAddresses(); + return false; + } + + const int signalCount = event->GetNumberOfSignals(); + if (signalCount < 0 || !CheckedAdd(actualCounts.signals, static_cast(signalCount), + errors, "rebuilt signals")) { + eventTree->ResetBranchAddresses(); + return false; + } + for (int signalIndex = 0; signalIndex < signalCount; ++signalIndex) { + auto signal = event->GetSignal(signalIndex); + if (signal == nullptr) { + errors << "ERROR: rebuilt signal event " << entry << " has a null signal at index " + << signalIndex << ".\n"; + eventTree->ResetBranchAddresses(); + return false; + } + const int pointCount = signal->GetNumberOfPoints(); + if (pointCount < 0 || !CheckedAdd(actualCounts.points, static_cast(pointCount), + errors, "rebuilt points")) { + eventTree->ResetBranchAddresses(); + return false; + } + } + } + eventTree->ResetBranchAddresses(); + + if (actualCounts.entries != expected.provenance.recovered.entries || + actualCounts.signals != expected.provenance.recovered.signals || + actualCounts.points != expected.provenance.recovered.points) { + errors << "ERROR: rebuilt candidate readback counts differ from the validated intermediate:" + << " entries " << actualCounts.entries << "/" << expected.provenance.recovered.entries + << ", signals " << actualCounts.signals << "/" << expected.provenance.recovered.signals + << ", points " << actualCounts.points << "/" << expected.provenance.recovered.points << ".\n"; + return false; + } + + auto analysisTree = dynamic_cast(candidate->Get("AnalysisTree")); + if (expected.analysisEntries < 0) { + if (analysisTree != nullptr) { + errors << "ERROR: rebuilt candidate unexpectedly contains an AnalysisTree.\n"; + return false; + } + } else if (analysisTree == nullptr || analysisTree->GetEntries() != expected.analysisEntries) { + errors << "ERROR: rebuilt candidate AnalysisTree is missing or has the wrong entry count.\n"; + return false; + } else { + for (const auto& branch : expected.analysisBranches) { + if (analysisTree->GetBranch(branch.c_str()) == nullptr) { + errors << "ERROR: rebuilt candidate AnalysisTree is missing branch '" << branch << "'.\n"; + return false; + } + } + } + + candidate->Close(); + return true; +} + +} // namespace REST_LegacyRecovery + +#endif diff --git a/macros/legacy/LegacyRecoveryDataUtils.h b/macros/legacy/LegacyRecoveryDataUtils.h new file mode 100644 index 000000000..96d64b568 --- /dev/null +++ b/macros/legacy/LegacyRecoveryDataUtils.h @@ -0,0 +1,203 @@ +#ifndef REST_LEGACY_RECOVERY_DATA_UTILS_H +#define REST_LEGACY_RECOVERY_DATA_UTILS_H + +#include +#include +#include +#include +#include +#include +#include + +namespace REST_LegacyRecovery { + +inline constexpr int kRecoveryFormatVersion = 2; +inline constexpr const char* kIntermediateProvenanceKind = "legacy-signal-data"; +inline constexpr const char* kResultProvenanceKind = "rebuilt-rest-file"; + +struct SignalSchemaVersions { + int time = -1; + int charge = -1; +}; + +struct RecoveryCounts { + std::uint64_t entries = 0; + std::uint64_t signals = 0; + std::uint64_t points = 0; +}; + +struct SourceIdentity { + std::string uuid; + std::string normalizedPath; + std::uint64_t fileSize = 0; + std::uint64_t entries = 0; + int signalVersion = -1; +}; + +struct RecoveryProvenance { + int formatVersion = -1; + std::string kind; + SourceIdentity source; + RecoveryCounts recovered; + std::string intermediateUuid; + std::string intermediatePath; + std::string resultUuid; +}; + +inline bool ValidateSupportedLegacySchema(const SignalSchemaVersions& versions, std::ostream& errors, + const std::string& context) { + if (versions.time != versions.charge) { + errors << "ERROR: " << context << " has inconsistent signal schema versions: time=" << versions.time + << ", charge=" << versions.charge << ".\n"; + return false; + } + if (versions.time < 1 || versions.time > 3) { + errors << "ERROR: " << context << " uses unsupported or unknown TRestDetectorSignal schema v" + << versions.time << "; only legacy versions 1..3 are recoverable.\n"; + return false; + } + return true; +} + +inline bool ValidateCurrentSignalSchema(const SignalSchemaVersions& versions, std::ostream& errors, + const std::string& context) { + if (versions.time != versions.charge) { + errors << "ERROR: " << context << " has inconsistent signal schema versions: time=" << versions.time + << ", charge=" << versions.charge << ".\n"; + return false; + } + if (versions.time < 4) { + errors << "ERROR: " << context << " does not contain the current vector signal schema" + << " (detected version " << versions.time << ").\n"; + return false; + } + return true; +} + +inline bool CheckedAdd(std::uint64_t& total, std::uint64_t increment, std::ostream& errors, + const std::string& description) { + if (increment > std::numeric_limits::max() - total) { + errors << "ERROR: overflow while counting " << description << ".\n"; + return false; + } + total += increment; + return true; +} + +template +bool ValidateFlattenedSignalData(const IdContainer* signalIds, const CountContainer* pointCounts, + const TimeContainer* times, const ChargeContainer* charges, + std::uint64_t& points, std::ostream& errors, const std::string& context) { + points = 0; + if (signalIds == nullptr || pointCounts == nullptr || times == nullptr || charges == nullptr) { + errors << "ERROR: " << context << " has a null required array pointer.\n"; + return false; + } + if (signalIds->size() != pointCounts->size()) { + errors << "ERROR: " << context << " has " << signalIds->size() << " signal IDs but " + << pointCounts->size() << " point counts.\n"; + return false; + } + if (times->size() != charges->size()) { + errors << "ERROR: " << context << " has " << times->size() << " times but " << charges->size() + << " charges.\n"; + return false; + } + for (std::size_t point = 0; point < times->size(); ++point) { + if (!std::isfinite(static_cast(times->at(point))) || + !std::isfinite(static_cast(charges->at(point)))) { + errors << "ERROR: " << context << " has a non-finite time or charge at point " << point << ".\n"; + return false; + } + } + + for (std::size_t signal = 0; signal < pointCounts->size(); ++signal) { + const auto value = pointCounts->at(signal); + using Count = std::decay_t; + if constexpr (std::is_signed_v) { + if (value < 0) { + errors << "ERROR: " << context << " has a negative point count for signal " << signal + << ".\n"; + return false; + } + } + + const auto count = static_cast(value); + if (!CheckedAdd(points, count, errors, context + " points")) return false; + if (points > times->size()) { + errors << "ERROR: " << context << " point counts exceed the flattened arrays at signal " << signal + << ".\n"; + return false; + } + } + + if (points != times->size()) { + errors << "ERROR: " << context << " point counts sum to " << points + << " but the flattened arrays contain " << times->size() << " values.\n"; + return false; + } + return true; +} + +inline bool ValidateIntermediateSource(const RecoveryProvenance& provenance, + const SourceIdentity& expectedSource, + const std::string& actualIntermediateUuid, std::ostream& errors) { + bool valid = true; + if (provenance.formatVersion != kRecoveryFormatVersion) { + errors << "ERROR: unsupported recovery provenance format " << provenance.formatVersion + << " (expected " << kRecoveryFormatVersion << ").\n"; + valid = false; + } + if (provenance.kind != kIntermediateProvenanceKind) { + errors << "ERROR: recovery provenance kind is '" << provenance.kind << "', expected '" + << kIntermediateProvenanceKind << "'.\n"; + valid = false; + } + if (provenance.source.uuid != expectedSource.uuid) { + errors << "ERROR: intermediate source UUID does not match the requested source file.\n"; + valid = false; + } + if (provenance.source.normalizedPath != expectedSource.normalizedPath) { + errors << "ERROR: intermediate source path '" << provenance.source.normalizedPath + << "' does not match requested source path '" << expectedSource.normalizedPath << "'.\n"; + valid = false; + } + if (provenance.source.fileSize != expectedSource.fileSize) { + errors << "ERROR: intermediate source size " << provenance.source.fileSize + << " does not match requested source size " << expectedSource.fileSize << ".\n"; + valid = false; + } + if (provenance.source.entries != expectedSource.entries) { + errors << "ERROR: intermediate source entry count " << provenance.source.entries + << " does not match requested source entry count " << expectedSource.entries << ".\n"; + valid = false; + } + if (provenance.source.signalVersion != expectedSource.signalVersion) { + errors << "ERROR: intermediate legacy signal version " << provenance.source.signalVersion + << " does not match requested source version " << expectedSource.signalVersion << ".\n"; + valid = false; + } + if (provenance.intermediateUuid != actualIntermediateUuid) { + errors << "ERROR: embedded intermediate UUID does not match the opened intermediate file.\n"; + valid = false; + } + return valid; +} + +inline bool ValidateRecoveredCounts(const RecoveryProvenance& provenance, const RecoveryCounts& actual, + std::ostream& errors, const std::string& context) { + if (provenance.recovered.entries == actual.entries && provenance.recovered.signals == actual.signals && + provenance.recovered.points == actual.points) { + return true; + } + + errors << "ERROR: " << context << " recovery counts do not match provenance:" + << " entries " << actual.entries << "/" << provenance.recovered.entries << ", signals " + << actual.signals << "/" << provenance.recovered.signals << ", points " << actual.points << "/" + << provenance.recovered.points << ".\n"; + return false; +} + +} // namespace REST_LegacyRecovery + +#endif diff --git a/macros/legacy/LegacyRecoveryFileUtils.h b/macros/legacy/LegacyRecoveryFileUtils.h index 9980f1743..09f6a01ec 100644 --- a/macros/legacy/LegacyRecoveryFileUtils.h +++ b/macros/legacy/LegacyRecoveryFileUtils.h @@ -17,27 +17,37 @@ struct PathComparison { std::string error; }; -inline PathComparison ComparePaths(const fs::path& first, const fs::path& second) { - std::error_code firstError; - const auto firstAbsolute = fs::absolute(first, firstError); - if (firstError) { - return {false, false, "cannot resolve '" + first.string() + "': " + firstError.message()}; - } - const auto firstPath = fs::weakly_canonical(firstAbsolute, firstError); - if (firstError) { - return {false, false, "cannot resolve '" + first.string() + "': " + firstError.message()}; +inline bool ResolvePathIdentity(const fs::path& path, std::string& identity, std::string& error) { + std::error_code absoluteError; + const auto absolute = fs::absolute(path, absoluteError); + if (absoluteError) { + error = "cannot resolve '" + path.string() + "': " + absoluteError.message(); + return false; } - std::error_code secondError; - const auto secondAbsolute = fs::absolute(second, secondError); - if (secondError) { - return {false, false, "cannot resolve '" + second.string() + "': " + secondError.message()}; - } - const auto secondPath = fs::weakly_canonical(secondAbsolute, secondError); - if (secondError) { - return {false, false, "cannot resolve '" + second.string() + "': " + secondError.message()}; + std::error_code canonicalError; + const auto canonical = fs::weakly_canonical(absolute, canonicalError); + if (canonicalError) { + error = "cannot resolve '" + path.string() + "': " + canonicalError.message(); + return false; } + identity = canonical.string(); + return true; +} + +inline PathComparison ComparePaths(const fs::path& first, const fs::path& second) { + std::string firstIdentity; + std::string firstError; + if (!ResolvePathIdentity(first, firstIdentity, firstError)) return {false, false, firstError}; + + std::string secondIdentity; + std::string secondError; + if (!ResolvePathIdentity(second, secondIdentity, secondError)) return {false, false, secondError}; + + const fs::path firstPath(firstIdentity); + const fs::path secondPath(secondIdentity); + if (firstPath == secondPath) return {true, true, ""}; std::error_code firstExistsError; @@ -114,6 +124,7 @@ inline bool ValidateNewOutputPath(const fs::path& inputPath, const fs::path& out } using RenameOperation = std::function; +using CandidateValidation = std::function; inline void RenamePath(const fs::path& source, const fs::path& destination, std::error_code& error) { fs::rename(source, destination, error); @@ -173,6 +184,18 @@ inline bool ReplaceFileWithBackup(const fs::path& replacementPath, const fs::pat return false; } +inline bool ValidateAndReplaceFileWithBackup(const fs::path& replacementPath, const fs::path& originalPath, + const fs::path& backupPath, std::ostream& errors, + const CandidateValidation& validateCandidate, + const RenameOperation& renameOperation = RenamePath) { + if (!validateCandidate(replacementPath, errors)) { + errors << "ERROR: candidate validation failed. The original file and any existing backup " + "were not touched.\n"; + return false; + } + return ReplaceFileWithBackup(replacementPath, originalPath, backupPath, errors, renameOperation); +} + } // namespace REST_LegacyRecovery #endif diff --git a/macros/legacy/LegacyRecoveryProvenance.h b/macros/legacy/LegacyRecoveryProvenance.h new file mode 100644 index 000000000..9cf62dd1e --- /dev/null +++ b/macros/legacy/LegacyRecoveryProvenance.h @@ -0,0 +1,199 @@ +#ifndef REST_LEGACY_RECOVERY_PROVENANCE_H +#define REST_LEGACY_RECOVERY_PROVENANCE_H + +#include +#include +#include + +#include +#include +#include +#include + +#include "LegacyRecoveryDataUtils.h" + +namespace REST_LegacyRecovery { + +inline constexpr const char* kRecoveryFormatKey = "REST_LegacyRecovery_FormatVersion"; +inline constexpr const char* kRecoveryKindKey = "REST_LegacyRecovery_Kind"; +inline constexpr const char* kRecoverySourceUuidKey = "REST_LegacyRecovery_SourceUUID"; +inline constexpr const char* kRecoverySourcePathKey = "REST_LegacyRecovery_SourcePath"; +inline constexpr const char* kRecoverySourceSizeKey = "REST_LegacyRecovery_SourceSize"; +inline constexpr const char* kRecoverySourceEntriesKey = "REST_LegacyRecovery_SourceEntries"; +inline constexpr const char* kRecoverySourceSignalVersionKey = "REST_LegacyRecovery_SourceSignalVersion"; +inline constexpr const char* kRecoveryEntriesKey = "REST_LegacyRecovery_RecoveredEntries"; +inline constexpr const char* kRecoverySignalsKey = "REST_LegacyRecovery_RecoveredSignals"; +inline constexpr const char* kRecoveryPointsKey = "REST_LegacyRecovery_RecoveredPoints"; +inline constexpr const char* kRecoveryIntermediateUuidKey = "REST_LegacyRecovery_IntermediateUUID"; +inline constexpr const char* kRecoveryIntermediatePathKey = "REST_LegacyRecovery_IntermediatePath"; +inline constexpr const char* kRecoveryResultUuidKey = "REST_LegacyRecovery_ResultUUID"; + +inline bool WriteNamedValue(TDirectory& directory, const char* name, const std::string& value, + std::ostream& errors) { + directory.cd(); + TNamed object(name, value.c_str()); + if (object.Write(name, TObject::kOverwrite) > 0) return true; + + errors << "ERROR: failed to write recovery provenance key '" << name << "'.\n"; + return false; +} + +inline bool WriteRecoveryProvenance(TDirectory& directory, const RecoveryProvenance& provenance, + std::ostream& errors) { + if (!WriteNamedValue(directory, kRecoveryFormatKey, std::to_string(provenance.formatVersion), errors) || + !WriteNamedValue(directory, kRecoveryKindKey, provenance.kind, errors) || + !WriteNamedValue(directory, kRecoverySourceUuidKey, provenance.source.uuid, errors) || + !WriteNamedValue(directory, kRecoverySourcePathKey, provenance.source.normalizedPath, errors) || + !WriteNamedValue(directory, kRecoverySourceSizeKey, std::to_string(provenance.source.fileSize), + errors) || + !WriteNamedValue(directory, kRecoverySourceEntriesKey, std::to_string(provenance.source.entries), + errors) || + !WriteNamedValue(directory, kRecoverySourceSignalVersionKey, + std::to_string(provenance.source.signalVersion), errors) || + !WriteNamedValue(directory, kRecoveryEntriesKey, std::to_string(provenance.recovered.entries), + errors) || + !WriteNamedValue(directory, kRecoverySignalsKey, std::to_string(provenance.recovered.signals), + errors) || + !WriteNamedValue(directory, kRecoveryPointsKey, std::to_string(provenance.recovered.points), + errors) || + !WriteNamedValue(directory, kRecoveryIntermediateUuidKey, provenance.intermediateUuid, errors)) { + return false; + } + + if (!provenance.intermediatePath.empty() && + !WriteNamedValue(directory, kRecoveryIntermediatePathKey, provenance.intermediatePath, errors)) { + return false; + } + if (!provenance.resultUuid.empty() && + !WriteNamedValue(directory, kRecoveryResultUuidKey, provenance.resultUuid, errors)) { + return false; + } + return true; +} + +inline bool ReadNamedValue(TDirectory& directory, const char* name, std::string& value, + std::ostream& errors) { + auto object = dynamic_cast(directory.Get(name)); + if (object == nullptr) { + errors << "ERROR: required recovery provenance key '" << name << "' is missing or invalid.\n"; + return false; + } + value = object->GetTitle(); + return true; +} + +template +bool ParseInteger(const std::string& text, Integer& value) { + if (text.empty()) return false; + if constexpr (std::is_unsigned_v) { + if (text.front() == '-') return false; + } + + // std::from_chars would be a natural fit, but older ROOT/Cling releases + // cannot parse the GCC 14 header. Stream extraction keeps these + // recovery macros usable with the ROOT versions deployed with REST v2.4. + std::istringstream input(text); + input >> std::noskipws >> value; + return !input.fail() && input.eof(); +} + +template +bool ReadIntegerValue(TDirectory& directory, const char* name, Integer& value, std::ostream& errors) { + std::string text; + if (!ReadNamedValue(directory, name, text, errors)) return false; + if (ParseInteger(text, value)) return true; + + errors << "ERROR: recovery provenance key '" << name << "' is not a valid integer: '" << text << "'.\n"; + return false; +} + +inline bool ReadRecoveryProvenance(TDirectory& directory, RecoveryProvenance& provenance, + std::ostream& errors) { + if (!ReadIntegerValue(directory, kRecoveryFormatKey, provenance.formatVersion, errors) || + !ReadNamedValue(directory, kRecoveryKindKey, provenance.kind, errors) || + !ReadNamedValue(directory, kRecoverySourceUuidKey, provenance.source.uuid, errors) || + !ReadNamedValue(directory, kRecoverySourcePathKey, provenance.source.normalizedPath, errors) || + !ReadIntegerValue(directory, kRecoverySourceSizeKey, provenance.source.fileSize, errors) || + !ReadIntegerValue(directory, kRecoverySourceEntriesKey, provenance.source.entries, errors) || + !ReadIntegerValue(directory, kRecoverySourceSignalVersionKey, provenance.source.signalVersion, + errors) || + !ReadIntegerValue(directory, kRecoveryEntriesKey, provenance.recovered.entries, errors) || + !ReadIntegerValue(directory, kRecoverySignalsKey, provenance.recovered.signals, errors) || + !ReadIntegerValue(directory, kRecoveryPointsKey, provenance.recovered.points, errors) || + !ReadNamedValue(directory, kRecoveryIntermediateUuidKey, provenance.intermediateUuid, errors)) { + return false; + } + + if (provenance.kind == kResultProvenanceKind) { + if (!ReadNamedValue(directory, kRecoveryIntermediatePathKey, provenance.intermediatePath, errors) || + !ReadNamedValue(directory, kRecoveryResultUuidKey, provenance.resultUuid, errors)) { + return false; + } + } + return true; +} + +inline bool CheckAndCloseOutputFile(TFile& file, std::ostream& errors) { + // TFile::GetErrno() forwards the process-global errno, which may contain a + // stale value from an unrelated earlier operation. Reset it immediately + // before each operation whose error state we inspect. + file.ResetErrno(); + file.Flush(); + const int flushErrno = file.GetErrno(); + bool valid = !file.TestBit(TFile::kWriteError) && flushErrno == 0; + if (!valid) { + errors << "ERROR: write failure while flushing '" << file.GetName() << "'" + << " (errno=" << flushErrno << ").\n"; + } + + file.ResetErrno(); + file.Close(); + const int closeErrno = file.GetErrno(); + if (file.TestBit(TFile::kWriteError) || closeErrno != 0) { + errors << "ERROR: write failure while closing '" << file.GetName() << "'" + << " (errno=" << closeErrno << ").\n"; + valid = false; + } + return valid; +} + +inline bool ValidateResultProvenance(const RecoveryProvenance& actual, const RecoveryProvenance& expected, + const std::string& openedResultUuid, std::ostream& errors) { + bool valid = true; + if (actual.formatVersion != kRecoveryFormatVersion || actual.formatVersion != expected.formatVersion) { + errors << "ERROR: rebuilt file has an unexpected recovery provenance format.\n"; + valid = false; + } + if (actual.kind != kResultProvenanceKind || actual.kind != expected.kind) { + errors << "ERROR: rebuilt file has an unexpected recovery provenance kind.\n"; + valid = false; + } + if (actual.source.uuid != expected.source.uuid || + actual.source.normalizedPath != expected.source.normalizedPath || + actual.source.fileSize != expected.source.fileSize || + actual.source.entries != expected.source.entries || + actual.source.signalVersion != expected.source.signalVersion) { + errors << "ERROR: rebuilt file source provenance does not match the validated source.\n"; + valid = false; + } + if (actual.recovered.entries != expected.recovered.entries || + actual.recovered.signals != expected.recovered.signals || + actual.recovered.points != expected.recovered.points) { + errors << "ERROR: rebuilt file recovery counts do not match the validated candidate.\n"; + valid = false; + } + if (actual.intermediateUuid != expected.intermediateUuid || + actual.intermediatePath != expected.intermediatePath) { + errors << "ERROR: rebuilt file intermediate provenance does not match the validated input.\n"; + valid = false; + } + if (actual.resultUuid != openedResultUuid || actual.resultUuid != expected.resultUuid) { + errors << "ERROR: rebuilt file UUID does not match its persisted recovery provenance.\n"; + valid = false; + } + return valid; +} + +} // namespace REST_LegacyRecovery + +#endif diff --git a/macros/legacy/REST_RebuildLegacySignalFile.C b/macros/legacy/REST_RebuildLegacySignalFile.C index 545cf49d3..3f32b9f12 100644 --- a/macros/legacy/REST_RebuildLegacySignalFile.C +++ b/macros/legacy/REST_RebuildLegacySignalFile.C @@ -23,6 +23,12 @@ // with the current libraries cannot be copied. They are reported explicitly; // if any are encountered, in-place recovery is refused and the fixed file is // left at _FixedTmp.root for inspection. +// The intermediate is accepted only when its ROOT UUID and persisted source +// identity match the requested source. This prevents accidental mix-ups but is +// not a cryptographic signature against a maliciously edited intermediate. +// Before an in-place replacement, the completed candidate is closed, reopened, +// and read back in full; any validation failure leaves the original and any +// pre-existing backup untouched. // // Arguments: // originalFile - the legacy REST file @@ -38,33 +44,20 @@ #include #include +#include #include +#include #include #include #include +#include "LegacyRecoveryCandidateValidation.h" +#include "LegacyRecoveryDataUtils.h" #include "LegacyRecoveryFileUtils.h" +#include "LegacyRecoveryProvenance.h" namespace REST_Rebuild_Internal { -Int_t GetOnDiskSignalVersion(TBranch* branch) { - if (branch == nullptr) return -1; - - auto branchElement = dynamic_cast(branch); - if (branchElement != nullptr) { - std::string name = branch->GetName(); - if (name == "fSignal.fSignalTime" || name == "fSignal.fSignalCharge") - return branchElement->GetClassVersion(); - } - - auto subs = branch->GetListOfBranches(); - for (int i = 0; i <= subs->GetLast(); i++) { - const Int_t version = GetOnDiskSignalVersion((TBranch*)subs->At(i)); - if (version > 0) return version; - } - return -1; -} - void SetBranchStatusRecursive(TBranch* branch, Bool_t status) { if (branch == nullptr) return; branch->SetStatus(status); @@ -72,6 +65,79 @@ void SetBranchStatusRecursive(TBranch* branch, Bool_t status) { for (int i = 0; i <= subs->GetLast(); i++) SetBranchStatusRecursive((TBranch*)subs->At(i), status); } +template +bool BindRequiredBranch(TTree* tree, const char* name, Address address, std::ostream& errors) { + if (tree->GetBranch(name) == nullptr) { + errors << "ERROR: required intermediate branch '" << name << "' is missing.\n"; + return false; + } + const int status = tree->SetBranchAddress(name, address); + if (status >= TTree::kMatch) return true; + + errors << "ERROR: cannot bind required intermediate branch '" << name << "' (status " << status << ").\n"; + return false; +} + +struct IntermediateData { + Int_t runOrigin = 0; + Int_t subRunOrigin = 0; + Int_t eventID = 0; + Int_t subEventID = 0; + Int_t timeSec = 0; + Int_t timeNanoSec = 0; + Bool_t ok = false; + TString* subEventTag = nullptr; + std::vector* signalID = nullptr; + std::vector* nPoints = nullptr; + std::vector* times = nullptr; + std::vector* charges = nullptr; + + bool Bind(TTree* tree, std::ostream& errors) { + return BindRequiredBranch(tree, "runOrigin", &runOrigin, errors) && + BindRequiredBranch(tree, "subRunOrigin", &subRunOrigin, errors) && + BindRequiredBranch(tree, "eventID", &eventID, errors) && + BindRequiredBranch(tree, "subEventID", &subEventID, errors) && + BindRequiredBranch(tree, "timeSec", &timeSec, errors) && + BindRequiredBranch(tree, "timeNanoSec", &timeNanoSec, errors) && + BindRequiredBranch(tree, "ok", &ok, errors) && + BindRequiredBranch(tree, "subEventTag", &subEventTag, errors) && + BindRequiredBranch(tree, "signalID", &signalID, errors) && + BindRequiredBranch(tree, "nPoints", &nPoints, errors) && + BindRequiredBranch(tree, "times", ×, errors) && + BindRequiredBranch(tree, "charges", &charges, errors); + } + + bool Validate(Long64_t entry, std::uint64_t& pointCount, std::ostream& errors) const { + if (subEventTag == nullptr) { + errors << "ERROR: intermediate entry " << entry << " has a null subEventTag pointer.\n"; + return false; + } + return REST_LegacyRecovery::ValidateFlattenedSignalData( + signalID, nPoints, times, charges, pointCount, errors, + "intermediate entry " + std::to_string(entry)); + } +}; + +bool ScanIntermediateTree(TTree* tree, IntermediateData& values, REST_LegacyRecovery::RecoveryCounts& counts, + std::ostream& errors) { + counts = {}; + counts.entries = static_cast(tree->GetEntries()); + for (Long64_t entry = 0; entry < tree->GetEntries(); ++entry) { + if (tree->GetEntry(entry) <= 0) { + errors << "ERROR: cannot read intermediate entry " << entry << ".\n"; + return false; + } + std::uint64_t points = 0; + if (!values.Validate(entry, points, errors) || + !REST_LegacyRecovery::CheckedAdd(counts.signals, values.signalID->size(), errors, + "intermediate signals") || + !REST_LegacyRecovery::CheckedAdd(counts.points, points, errors, "intermediate points")) { + return false; + } + } + return true; +} + } // namespace REST_Rebuild_Internal void REST_RebuildLegacySignalFile(const char* originalFile, const char* signalDataFile = "", @@ -102,12 +168,12 @@ void REST_RebuildLegacySignalFile(const char* originalFile, const char* signalDa } // --- open inputs --- - TFile* original = TFile::Open(originalFile); + std::unique_ptr original(TFile::Open(originalFile)); if (original == nullptr || original->IsZombie()) { std::cout << "ERROR: cannot open original file: " << originalFile << std::endl; return; } - TFile* data = TFile::Open(dataName.c_str()); + std::unique_ptr data(TFile::Open(dataName.c_str())); if (data == nullptr || data->IsZombie()) { std::cout << "ERROR: cannot open signal data file: " << dataName << std::endl; std::cout << "Run stage 1 first (with plain root, NOT restRoot):" << std::endl; @@ -128,42 +194,62 @@ void REST_RebuildLegacySignalFile(const char* originalFile, const char* signalDa std::cout << "ERROR: no TRestDetectorSignalEventBranch in " << originalFile << std::endl; return; } - const Int_t onDiskVersion = GetOnDiskSignalVersion(signalBranch); - if (onDiskVersion >= 4) { - std::cout << "This file already uses the current layout (TRestDetectorSignal v" << onDiskVersion - << "). Nothing to rebuild." << std::endl; + const auto onDiskVersions = REST_LegacyRecovery::DetectSignalSchemaVersions(signalBranch); + if (!REST_LegacyRecovery::ValidateSupportedLegacySchema(onDiskVersions, std::cout, "source file")) { return; } + const Int_t onDiskVersion = onDiskVersions.time; const Long64_t nEntries = oldTree->GetEntries(); + if (nEntries < 0 || original->GetSize() < 0) { + std::cout << "ERROR: source file reports an invalid entry count or file size." << std::endl; + return; + } if (dataTree->GetEntries() != nEntries) { std::cout << "ERROR: entry mismatch — EventTree has " << nEntries << " entries, signal data has " << dataTree->GetEntries() << ". Wrong intermediate file?" << std::endl; return; } - // --- bind the intermediate tree --- - Int_t runOrigin, subRunOrigin, eventID, subEventID, timeSec, timeNanoSec; - Bool_t ok; - TString* subEventTag = nullptr; - std::vector*signalID = nullptr, *nPoints = nullptr; - std::vector*times = nullptr, *charges = nullptr; - - dataTree->SetBranchAddress("runOrigin", &runOrigin); - dataTree->SetBranchAddress("subRunOrigin", &subRunOrigin); - dataTree->SetBranchAddress("eventID", &eventID); - dataTree->SetBranchAddress("subEventID", &subEventID); - dataTree->SetBranchAddress("timeSec", &timeSec); - dataTree->SetBranchAddress("timeNanoSec", &timeNanoSec); - dataTree->SetBranchAddress("ok", &ok); - dataTree->SetBranchAddress("subEventTag", &subEventTag); - dataTree->SetBranchAddress("signalID", &signalID); - dataTree->SetBranchAddress("nPoints", &nPoints); - dataTree->SetBranchAddress("times", ×); - dataTree->SetBranchAddress("charges", &charges); + std::string sourcePath; + std::string pathError; + if (!REST_LegacyRecovery::ResolvePathIdentity(originalFile, sourcePath, pathError)) { + std::cout << "ERROR: " << pathError << std::endl; + return; + } + std::string intermediatePath; + if (!REST_LegacyRecovery::ResolvePathIdentity(dataName, intermediatePath, pathError)) { + std::cout << "ERROR: " << pathError << std::endl; + return; + } + + REST_LegacyRecovery::SourceIdentity sourceIdentity; + sourceIdentity.uuid = original->GetUUID().AsString(); + sourceIdentity.normalizedPath = sourcePath; + sourceIdentity.fileSize = static_cast(original->GetSize()); + sourceIdentity.entries = static_cast(nEntries); + sourceIdentity.signalVersion = onDiskVersion; + + REST_LegacyRecovery::RecoveryProvenance intermediateProvenance; + if (!REST_LegacyRecovery::ReadRecoveryProvenance(*data, intermediateProvenance, std::cout) || + !REST_LegacyRecovery::ValidateIntermediateSource(intermediateProvenance, sourceIdentity, + data->GetUUID().AsString(), std::cout)) { + std::cout << "ERROR: refusing to combine an unauthenticated intermediate with the source." + << std::endl; + return; + } + + IntermediateData intermediateValues; + if (!intermediateValues.Bind(dataTree, std::cout)) return; + REST_LegacyRecovery::RecoveryCounts scannedCounts; + if (!ScanIntermediateTree(dataTree, intermediateValues, scannedCounts, std::cout) || + !REST_LegacyRecovery::ValidateRecoveredCounts(intermediateProvenance, scannedCounts, std::cout, + "intermediate")) { + return; + } // --- output file (single write session: StreamerInfos are preserved) --- - TFile* out = TFile::Open(outName.c_str(), "CREATE"); + std::unique_ptr out(TFile::Open(outName.c_str(), "CREATE")); if (out == nullptr || out->IsZombie()) { std::cout << "ERROR: cannot create output file: " << outName << std::endl; return; @@ -172,6 +258,7 @@ void REST_RebuildLegacySignalFile(const char* originalFile, const char* signalDa // --- copy metadata keys (highest cycle only, skip trees) --- std::set seen; std::vector skipped; + std::vector copiedMetadataKeys; TIter nextKey(original->GetListOfKeys()); TKey* key; while ((key = (TKey*)nextKey())) { @@ -188,7 +275,12 @@ void REST_RebuildLegacySignalFile(const char* originalFile, const char* signalDa continue; } out->cd(); - obj->Write(keyName.c_str()); + if (obj->Write(keyName.c_str(), TObject::kOverwrite) <= 0) { + std::cout << "ERROR: failed to write metadata key '" << keyName << "'." << std::endl; + REST_LegacyRecovery::CheckAndCloseOutputFile(*out, std::cout); + return; + } + copiedMetadataKeys.push_back(keyName); } // --- rebuild the EventTree --- @@ -201,6 +293,7 @@ void REST_RebuildLegacySignalFile(const char* originalFile, const char* signalDa SetBranchStatusRecursive(signalBranch, 0); std::vector otherBranchNames; + std::vector otherBranches; std::vector skippedEventBranches; TIter nextBranch(oldTree->GetListOfBranches()); TBranch* br; @@ -217,6 +310,7 @@ void REST_RebuildLegacySignalFile(const char* originalFile, const char* signalDa continue; } otherBranchNames.push_back(name); + otherBranches.push_back(br); } out->cd(); @@ -229,54 +323,147 @@ void REST_RebuildLegacySignalFile(const char* originalFile, const char* signalDa const std::string className = name.substr(0, name.rfind("Branch")); TClass* cl = TClass::GetClass(className.c_str()); otherEvents[b] = (TRestEvent*)cl->New(); - oldTree->SetBranchAddress(name.c_str(), &otherEvents[b]); - newTree->Branch(name.c_str(), className.c_str(), &otherEvents[b]); + if (otherEvents[b] == nullptr) { + std::cout << "ERROR: cannot construct event class '" << className << "'." << std::endl; + REST_LegacyRecovery::CheckAndCloseOutputFile(*out, std::cout); + return; + } + const int bindStatus = oldTree->SetBranchAddress(name.c_str(), &otherEvents[b]); + if (bindStatus < TTree::kMatch) { + std::cout << "ERROR: cannot bind source event branch '" << name << "' (status " << bindStatus + << ")." << std::endl; + REST_LegacyRecovery::CheckAndCloseOutputFile(*out, std::cout); + return; + } + if (newTree->Branch(name.c_str(), className.c_str(), &otherEvents[b]) == nullptr) { + std::cout << "ERROR: cannot create rebuilt event branch '" << name << "'." << std::endl; + REST_LegacyRecovery::CheckAndCloseOutputFile(*out, std::cout); + return; + } } auto event = new TRestDetectorSignalEvent(); - newTree->Branch("TRestDetectorSignalEventBranch", &event); + if (newTree->Branch("TRestDetectorSignalEventBranch", &event) == nullptr) { + std::cout << "ERROR: cannot create rebuilt detector signal branch." << std::endl; + REST_LegacyRecovery::CheckAndCloseOutputFile(*out, std::cout); + return; + } - Long64_t totalSignals = 0, totalPoints = 0; + REST_LegacyRecovery::RecoveryCounts writtenCounts; + writtenCounts.entries = scannedCounts.entries; for (Long64_t i = 0; i < nEntries; i++) { - oldTree->GetEntry(i); // reads the other event branches (signal branch disabled) - dataTree->GetEntry(i); + for (size_t branchIndex = 0; branchIndex < otherBranches.size(); ++branchIndex) { + if (otherBranches[branchIndex]->GetEntry(i) <= 0) { + std::cout << "ERROR: cannot read source event branch '" << otherBranchNames[branchIndex] + << "' at entry " << i << "." << std::endl; + REST_LegacyRecovery::CheckAndCloseOutputFile(*out, std::cout); + return; + } + } + if (dataTree->GetEntry(i) <= 0) { + std::cout << "ERROR: cannot reread intermediate entry " << i << "." << std::endl; + REST_LegacyRecovery::CheckAndCloseOutputFile(*out, std::cout); + return; + } + std::uint64_t entryPoints = 0; + if (!intermediateValues.Validate(i, entryPoints, std::cout)) { + REST_LegacyRecovery::CheckAndCloseOutputFile(*out, std::cout); + return; + } event->Initialize(); - event->SetRunOrigin(runOrigin); - event->SetSubRunOrigin(subRunOrigin); - event->SetID(eventID); - event->SetSubID(subEventID); - event->SetSubEventTag(*subEventTag); - event->SetTime((Double_t)timeSec, (Double_t)timeNanoSec); - event->SetOK(ok); + event->SetRunOrigin(intermediateValues.runOrigin); + event->SetSubRunOrigin(intermediateValues.subRunOrigin); + event->SetID(intermediateValues.eventID); + event->SetSubID(intermediateValues.subEventID); + event->SetSubEventTag(*intermediateValues.subEventTag); + event->SetTime((Double_t)intermediateValues.timeSec, (Double_t)intermediateValues.timeNanoSec); + event->SetOK(intermediateValues.ok); size_t offset = 0; - for (size_t s = 0; s < signalID->size(); s++) { + for (size_t s = 0; s < intermediateValues.signalID->size(); s++) { TRestDetectorSignal signal; - signal.SetSignalID(signalID->at(s)); - const size_t n = nPoints->at(s); - for (size_t p = 0; p < n; p++) signal.NewPoint(times->at(offset + p), charges->at(offset + p)); + signal.SetSignalID(intermediateValues.signalID->at(s)); + const size_t n = static_cast(intermediateValues.nPoints->at(s)); + if (offset > intermediateValues.times->size() || n > intermediateValues.times->size() - offset) { + std::cout << "ERROR: intermediate entry " << i + << " exceeds flattened array bounds while rebuilding signal " << s << "." + << std::endl; + REST_LegacyRecovery::CheckAndCloseOutputFile(*out, std::cout); + return; + } + for (size_t p = 0; p < n; p++) { + signal.NewPoint(intermediateValues.times->at(offset + p), + intermediateValues.charges->at(offset + p)); + } offset += n; event->AddSignal(signal); } - totalSignals += signalID->size(); - totalPoints += offset; + if (offset != entryPoints || + !REST_LegacyRecovery::CheckedAdd(writtenCounts.signals, intermediateValues.signalID->size(), + std::cout, "rebuilt signals") || + !REST_LegacyRecovery::CheckedAdd(writtenCounts.points, offset, std::cout, "rebuilt points")) { + std::cout << "ERROR: rebuilt entry " << i << " failed count validation." << std::endl; + REST_LegacyRecovery::CheckAndCloseOutputFile(*out, std::cout); + return; + } - newTree->Fill(); + if (newTree->Fill() < 0) { + std::cout << "ERROR: failed to write rebuilt EventTree entry " << i << "." << std::endl; + REST_LegacyRecovery::CheckAndCloseOutputFile(*out, std::cout); + return; + } + } + if (writtenCounts.entries != scannedCounts.entries || writtenCounts.signals != scannedCounts.signals || + writtenCounts.points != scannedCounts.points) { + std::cout << "ERROR: rebuilt counts changed while writing the candidate." << std::endl; + REST_LegacyRecovery::CheckAndCloseOutputFile(*out, std::cout); + return; + } + if (newTree->Write("", TObject::kOverwrite) <= 0) { + std::cout << "ERROR: failed to write the rebuilt EventTree." << std::endl; + REST_LegacyRecovery::CheckAndCloseOutputFile(*out, std::cout); + return; } - newTree->Write("", TObject::kOverwrite); // --- copy the AnalysisTree unchanged --- auto anaTree = dynamic_cast(original->Get("AnalysisTree")); + Long64_t analysisEntries = -1; + std::vector analysisBranchNames; if (anaTree != nullptr) { + analysisEntries = anaTree->GetEntries(); + TIter nextAnalysisBranch(anaTree->GetListOfBranches()); + TBranch* analysisBranch; + while ((analysisBranch = static_cast(nextAnalysisBranch()))) { + analysisBranchNames.emplace_back(analysisBranch->GetName()); + } out->cd(); TTree* anaClone = anaTree->CloneTree(-1, "fast"); - anaClone->Write("", TObject::kOverwrite); + if (anaClone == nullptr || anaClone->GetEntries() != analysisEntries || + anaClone->Write("", TObject::kOverwrite) <= 0) { + std::cout << "ERROR: failed to clone and write the AnalysisTree." << std::endl; + REST_LegacyRecovery::CheckAndCloseOutputFile(*out, std::cout); + return; + } } else { std::cout << "WARNING: no AnalysisTree found; skipping." << std::endl; } - out->Close(); + REST_LegacyRecovery::RecoveryProvenance resultProvenance = intermediateProvenance; + resultProvenance.kind = REST_LegacyRecovery::kResultProvenanceKind; + resultProvenance.recovered = scannedCounts; + resultProvenance.intermediatePath = intermediatePath; + resultProvenance.resultUuid = out->GetUUID().AsString(); + if (!REST_LegacyRecovery::WriteRecoveryProvenance(*out, resultProvenance, std::cout)) { + std::cout << "ERROR: failed to persist rebuilt-file recovery provenance." << std::endl; + REST_LegacyRecovery::CheckAndCloseOutputFile(*out, std::cout); + return; + } + if (!REST_LegacyRecovery::CheckAndCloseOutputFile(*out, std::cout)) { + std::cout << "ERROR: rebuilt candidate is incomplete; the original was not touched." << std::endl; + return; + } + original->Close(); data->Close(); @@ -294,6 +481,17 @@ void REST_RebuildLegacySignalFile(const char* originalFile, const char* signalDa // --- overwrite handling --- std::string finalName = outName; + REST_LegacyRecovery::CandidateExpectations candidateExpectations; + candidateExpectations.provenance = resultProvenance; + candidateExpectations.analysisEntries = analysisEntries; + candidateExpectations.metadataKeys = copiedMetadataKeys; + candidateExpectations.otherEventBranches = otherBranchNames; + candidateExpectations.analysisBranches = analysisBranchNames; + const auto validateCandidate = [&candidateExpectations](const std::filesystem::path& path, + std::ostream& errors) { + return REST_LegacyRecovery::ValidateRecoveryCandidate(path, candidateExpectations, errors); + }; + if (overwrite) { if (!skipped.empty() || !skippedEventBranches.empty()) { std::cout << "ERROR: refusing in-place replacement because the rebuilt file has omitted " @@ -301,15 +499,21 @@ void REST_RebuildLegacySignalFile(const char* originalFile, const char* signalDa << "The original is unchanged. Inspect the candidate file at: " << outName << std::endl; return; } - if (!REST_LegacyRecovery::ReplaceFileWithBackup(outName, originalFile, backupName, std::cout)) { + if (!REST_LegacyRecovery::ValidateAndReplaceFileWithBackup(outName, originalFile, backupName, + std::cout, validateCandidate)) { return; } finalName = originalFile; std::cout << "Original file kept as: " << backupName << std::endl; + } else if (!validateCandidate(outName, std::cout)) { + std::cout << "ERROR: rebuilt sibling candidate failed readback validation and must not be " + "used." + << std::endl; + return; } std::cout << std::endl; - std::cout << "Rebuilt " << nEntries << " entries: " << totalSignals << " signals, " << totalPoints - << " points." << std::endl; + std::cout << "Rebuilt " << nEntries << " entries: " << scannedCounts.signals << " signals, " + << scannedCounts.points << " points." << std::endl; std::cout << "Fixed file written to: " << finalName << std::endl; } diff --git a/macros/legacy/recoverLegacySignalData.C b/macros/legacy/recoverLegacySignalData.C index bc0b5ed59..7e9b4a0e1 100644 --- a/macros/legacy/recoverLegacySignalData.C +++ b/macros/legacy/recoverLegacySignalData.C @@ -22,6 +22,10 @@ // The intermediate file is written next to the input as // _LegacySignalData.root unless an explicit output path is given. // Existing output files are never overwritten. +// It records the source ROOT UUID, canonicalized source path, file size, +// legacy schema version, and recovered counts. These facts authenticate the +// intended source against accidental mix-ups; they are not a cryptographic +// signature and do not make a maliciously edited intermediate trustworthy. // // This file is deliberately NOT named REST_*.C so that restRoot's --m macro // loading does not interpret it (the replica class definitions below would @@ -36,11 +40,17 @@ #include #include +#include +#include #include +#include +#include #include #include +#include "LegacyRecoveryDataUtils.h" #include "LegacyRecoveryFileUtils.h" +#include "LegacyRecoveryProvenance.h" //------------------------------------------------------------------------------ // Replica classes matching the legacy on-disk layout (REST <= v2.4.2). @@ -76,25 +86,85 @@ class TRestDetectorSignalEvent : public TRestEvent { }; //------------------------------------------------------------------------------ -// Return the ClassDef version of TRestDetectorSignal recorded in the file's -// fSignal sub-branches, or -1 if it cannot be determined. +// Return the ClassDef versions recorded in the time and charge sub-branches. //------------------------------------------------------------------------------ -static Int_t GetOnDiskSignalVersion(TBranch* branch) { - if (branch == nullptr) return -1; +static void CollectOnDiskSignalVersions(TBranch* branch, REST_LegacyRecovery::SignalSchemaVersions& versions, + bool& timeSeen, bool& chargeSeen) { + if (branch == nullptr) return; auto branchElement = dynamic_cast(branch); if (branchElement != nullptr) { - std::string name = branch->GetName(); - if (name == "fSignal.fSignalTime" || name == "fSignal.fSignalCharge") - return branchElement->GetClassVersion(); + const std::string name = branch->GetName(); + const int version = branchElement->GetClassVersion(); + if (name == "fSignal.fSignalTime") { + versions.time = timeSeen && versions.time != version ? -2 : version; + timeSeen = true; + } else if (name == "fSignal.fSignalCharge") { + versions.charge = chargeSeen && versions.charge != version ? -2 : version; + chargeSeen = true; + } } - auto subs = branch->GetListOfBranches(); - for (int i = 0; i <= subs->GetLast(); i++) { - const Int_t version = GetOnDiskSignalVersion((TBranch*)subs->At(i)); - if (version > 0) return version; + auto children = branch->GetListOfBranches(); + for (int index = 0; index <= children->GetLast(); ++index) { + CollectOnDiskSignalVersions(static_cast(children->At(index)), versions, timeSeen, + chargeSeen); } - return -1; +} + +static REST_LegacyRecovery::SignalSchemaVersions GetOnDiskSignalVersions(TBranch* branch) { + REST_LegacyRecovery::SignalSchemaVersions versions; + bool timeSeen = false; + bool chargeSeen = false; + CollectOnDiskSignalVersions(branch, versions, timeSeen, chargeSeen); + if (!timeSeen) versions.time = -1; + if (!chargeSeen) versions.charge = -1; + return versions; +} + +static bool ValidateLegacyEvent(const TRestDetectorSignalEvent* event, Long64_t entry, + std::uint64_t& signalCount, std::uint64_t& pointCount, + Long64_t* suspectValues, std::ostream& errors) { + signalCount = 0; + pointCount = 0; + if (event == nullptr) { + errors << "ERROR: legacy signal event pointer is null at entry " << entry << ".\n"; + return false; + } + + signalCount = event->fSignal.size(); + for (std::size_t index = 0; index < event->fSignal.size(); ++index) { + const auto& signal = event->fSignal[index]; + if (signal.fSignalTime.size() != signal.fSignalCharge.size()) { + errors << "ERROR: legacy signal event " << entry << ", signal " << index << " has " + << signal.fSignalTime.size() << " time values but " << signal.fSignalCharge.size() + << " charge values.\n"; + return false; + } + if (signal.fSignalTime.size() > static_cast(std::numeric_limits::max())) { + errors << "ERROR: legacy signal event " << entry << ", signal " << index + << " has too many points to encode in the intermediate format.\n"; + return false; + } + if (!REST_LegacyRecovery::CheckedAdd(pointCount, signal.fSignalTime.size(), errors, + "legacy signal points")) { + return false; + } + for (std::size_t point = 0; point < signal.fSignalTime.size(); ++point) { + const auto time = signal.fSignalTime[point]; + const auto charge = signal.fSignalCharge[point]; + if (!std::isfinite(time) || !std::isfinite(charge)) { + errors << "ERROR: legacy signal event " << entry << ", signal " << index + << " has a non-finite time or charge at point " << point << ".\n"; + return false; + } + if (suspectValues != nullptr) { + if (std::abs(time) > 1e12) ++(*suspectValues); + if (std::abs(charge) > 1e12) ++(*suspectValues); + } + } + } + return true; } void recoverLegacySignalData(const char* inputFile, const char* outputFile = "") { @@ -120,7 +190,7 @@ void recoverLegacySignalData(const char* inputFile, const char* outputFile = "") return; } - TFile* f = TFile::Open(inputFile); + std::unique_ptr f(TFile::Open(inputFile)); if (f == nullptr || f->IsZombie()) { std::cout << "ERROR: cannot open input file: " << inputFile << std::endl; return; @@ -139,21 +209,61 @@ void recoverLegacySignalData(const char* inputFile, const char* outputFile = "") return; } - const Int_t onDiskVersion = GetOnDiskSignalVersion(signalBranch); - if (onDiskVersion < 0) { - std::cout << "ERROR: could not determine the on-disk TRestDetectorSignal version." << std::endl; - return; - } - if (onDiskVersion >= 4) { - std::cout << "This file already uses the vector layout (TRestDetectorSignal v" - << onDiskVersion << "). Nothing to recover." << std::endl; + const auto onDiskVersions = GetOnDiskSignalVersions(signalBranch); + if (!REST_LegacyRecovery::ValidateSupportedLegacySchema(onDiskVersions, std::cout, "source file")) { return; } + const Int_t onDiskVersion = onDiskVersions.time; std::cout << "On-disk TRestDetectorSignal version: " << onDiskVersion << " (legacy float layout)" << std::endl; auto event = new TRestDetectorSignalEvent(); - eventTree->SetBranchAddress("TRestDetectorSignalEventBranch", &event); + const int signalBindStatus = eventTree->SetBranchAddress("TRestDetectorSignalEventBranch", &event); + if (signalBindStatus < TTree::kMatch) { + std::cout << "ERROR: cannot bind the legacy detector signal branch (status " << signalBindStatus + << ")." << std::endl; + return; + } + + const Long64_t nEntries = eventTree->GetEntries(); + const Long64_t sourceSize = f->GetSize(); + if (nEntries < 0 || sourceSize < 0) { + std::cout << "ERROR: source file reports an invalid entry count or file size." << std::endl; + return; + } + REST_LegacyRecovery::RecoveryCounts recoveredCounts; + recoveredCounts.entries = static_cast(nEntries); + Long64_t suspectValues = 0; + for (Long64_t entry = 0; entry < nEntries; ++entry) { + if (signalBranch->GetEntry(entry) <= 0) { + std::cout << "ERROR: cannot read the legacy detector signal branch at entry " << entry << "." + << std::endl; + return; + } + std::uint64_t eventSignals = 0; + std::uint64_t eventPoints = 0; + if (!ValidateLegacyEvent(event, entry, eventSignals, eventPoints, &suspectValues, std::cout) || + !REST_LegacyRecovery::CheckedAdd(recoveredCounts.signals, eventSignals, std::cout, + "legacy signals") || + !REST_LegacyRecovery::CheckedAdd(recoveredCounts.points, eventPoints, std::cout, + "legacy points")) { + return; + } + } + + std::string sourcePath; + std::string pathError; + if (!REST_LegacyRecovery::ResolvePathIdentity(inputFile, sourcePath, pathError)) { + std::cout << "ERROR: " << pathError << std::endl; + return; + } + + REST_LegacyRecovery::SourceIdentity sourceIdentity; + sourceIdentity.uuid = f->GetUUID().AsString(); + sourceIdentity.normalizedPath = sourcePath; + sourceIdentity.fileSize = static_cast(sourceSize); + sourceIdentity.entries = recoveredCounts.entries; + sourceIdentity.signalVersion = onDiskVersion; TFile out(outName.c_str(), "CREATE"); if (out.IsZombie()) { @@ -171,26 +281,36 @@ void recoverLegacySignalData(const char* inputFile, const char* outputFile = "") std::vector signalID, nPoints; std::vector times, charges; - dataTree.Branch("runOrigin", &runOrigin); - dataTree.Branch("subRunOrigin", &subRunOrigin); - dataTree.Branch("eventID", &eventID); - dataTree.Branch("subEventID", &subEventID); - dataTree.Branch("timeSec", &timeSec); - dataTree.Branch("timeNanoSec", &timeNanoSec); - dataTree.Branch("ok", &ok); - dataTree.Branch("subEventTag", &subEventTag); - dataTree.Branch("signalID", &signalID); - dataTree.Branch("nPoints", &nPoints); - dataTree.Branch("times", ×); - dataTree.Branch("charges", &charges); - - const Long64_t nEntries = eventTree->GetEntries(); - Long64_t totalSignals = 0, totalPoints = 0, suspectValues = 0; + if (dataTree.Branch("runOrigin", &runOrigin) == nullptr || + dataTree.Branch("subRunOrigin", &subRunOrigin) == nullptr || + dataTree.Branch("eventID", &eventID) == nullptr || + dataTree.Branch("subEventID", &subEventID) == nullptr || + dataTree.Branch("timeSec", &timeSec) == nullptr || + dataTree.Branch("timeNanoSec", &timeNanoSec) == nullptr || dataTree.Branch("ok", &ok) == nullptr || + dataTree.Branch("subEventTag", &subEventTag) == nullptr || + dataTree.Branch("signalID", &signalID) == nullptr || + dataTree.Branch("nPoints", &nPoints) == nullptr || dataTree.Branch("times", ×) == nullptr || + dataTree.Branch("charges", &charges) == nullptr) { + std::cout << "ERROR: failed to create one or more intermediate branches." << std::endl; + REST_LegacyRecovery::CheckAndCloseOutputFile(out, std::cout); + return; + } for (Long64_t i = 0; i < nEntries; i++) { // Read only the signal branch: the other event branches have no // dictionary in a plain root session. - signalBranch->GetEntry(i); + if (signalBranch->GetEntry(i) <= 0) { + std::cout << "ERROR: cannot reread the legacy detector signal branch at entry " << i << "." + << std::endl; + REST_LegacyRecovery::CheckAndCloseOutputFile(out, std::cout); + return; + } + std::uint64_t eventSignals = 0; + std::uint64_t eventPoints = 0; + if (!ValidateLegacyEvent(event, i, eventSignals, eventPoints, nullptr, std::cout)) { + REST_LegacyRecovery::CheckAndCloseOutputFile(out, std::cout); + return; + } runOrigin = event->fRunOrigin; subRunOrigin = event->fSubRunOrigin; @@ -211,25 +331,49 @@ void recoverLegacySignalData(const char* inputFile, const char* outputFile = "") nPoints.push_back((Int_t)signal.fSignalTime.size()); times.insert(times.end(), signal.fSignalTime.begin(), signal.fSignalTime.end()); charges.insert(charges.end(), signal.fSignalCharge.begin(), signal.fSignalCharge.end()); - for (const auto v : signal.fSignalTime) - if (std::abs(v) > 1e12) suspectValues++; } - totalSignals += signalID.size(); - totalPoints += times.size(); + if (eventSignals != signalID.size() || eventPoints != times.size() || + times.size() != charges.size()) { + std::cout << "ERROR: legacy event " << i << " changed while flattening the validated signal data." + << std::endl; + REST_LegacyRecovery::CheckAndCloseOutputFile(out, std::cout); + return; + } - dataTree.Fill(); + if (dataTree.Fill() < 0) { + std::cout << "ERROR: failed to write intermediate entry " << i << "." << std::endl; + REST_LegacyRecovery::CheckAndCloseOutputFile(out, std::cout); + return; + } } - dataTree.Write(); + if (dataTree.Write() <= 0) { + std::cout << "ERROR: failed to write the LegacySignalData tree." << std::endl; + REST_LegacyRecovery::CheckAndCloseOutputFile(out, std::cout); + return; + } - // Provenance - TNamed("sourceFile", inputFile).Write(); - TNamed("onDiskSignalVersion", std::to_string(onDiskVersion).c_str()).Write(); - out.Close(); + REST_LegacyRecovery::RecoveryProvenance provenance; + provenance.formatVersion = REST_LegacyRecovery::kRecoveryFormatVersion; + provenance.kind = REST_LegacyRecovery::kIntermediateProvenanceKind; + provenance.source = sourceIdentity; + provenance.recovered = recoveredCounts; + provenance.intermediateUuid = out.GetUUID().AsString(); + if (!REST_LegacyRecovery::WriteRecoveryProvenance(out, provenance, std::cout) || + TNamed("sourceFile", sourcePath.c_str()).Write() <= 0 || + TNamed("onDiskSignalVersion", std::to_string(onDiskVersion).c_str()).Write() <= 0) { + std::cout << "ERROR: failed to write complete intermediate provenance." << std::endl; + REST_LegacyRecovery::CheckAndCloseOutputFile(out, std::cout); + return; + } + if (!REST_LegacyRecovery::CheckAndCloseOutputFile(out, std::cout)) { + std::cout << "ERROR: intermediate output is incomplete and must not be used." << std::endl; + return; + } std::cout << std::endl; - std::cout << "Recovered " << nEntries << " entries: " << totalSignals << " signals, " << totalPoints - << " points." << std::endl; + std::cout << "Recovered " << nEntries << " entries: " << recoveredCounts.signals << " signals, " + << recoveredCounts.points << " points." << std::endl; if (suspectValues > 0) std::cout << "WARNING: " << suspectValues << " suspicious values (|v| > 1e12) found — the recovered data may be corrupted!" diff --git a/source/framework/test/integration/LegacyDetectorSignal.cxx b/source/framework/test/integration/LegacyDetectorSignal.cxx index 3b6497150..990bce3fe 100644 --- a/source/framework/test/integration/LegacyDetectorSignal.cxx +++ b/source/framework/test/integration/LegacyDetectorSignal.cxx @@ -1,19 +1,59 @@ #include +#include #include #include #include #include #include +#include +#include #include #include +#include +#include #include +#include #include +#include "../../../../macros/legacy/LegacyRecoveryCandidateValidation.h" +#include "../../../../macros/legacy/LegacyRecoveryFileUtils.h" +#include "../../../../macros/legacy/LegacyRecoveryProvenance.h" + namespace { namespace fs = std::filesystem; +class TemporaryDirectory { + public: + TemporaryDirectory() { + static std::atomic sequence{0}; + const auto suffix = + std::to_string(std::chrono::high_resolution_clock::now().time_since_epoch().count()) + "_" + + std::to_string(sequence++); + path = fs::temp_directory_path() / ("rest_legacy_candidate_test_" + suffix); + fs::create_directories(path); + } + + ~TemporaryDirectory() { + std::error_code error; + fs::remove_all(path, error); + } + + fs::path path; +}; + +void WriteText(const fs::path& path, const std::string& text) { + std::ofstream output(path); + ASSERT_TRUE(output.is_open()); + output << text; +} + +std::string ReadText(const fs::path& path) { + std::ifstream input(path); + return {std::istreambuf_iterator(input), std::istreambuf_iterator()}; +} + class UnknownEvent : public TRestRawSignalEvent { public: const char* ClassName() const override { return "UnknownEvent"; } @@ -116,4 +156,49 @@ TEST_F(LegacyDetectorSignalTest, InvalidSelectionPreservesCurrentEvent) { EXPECT_EQ(std::string(run.GetInputEvent()->ClassName()), currentType); } +TEST(LegacyRecoveryCandidateValidation, InvalidCandidateLeavesOriginalUntouched) { + TemporaryDirectory temporary; + const auto original = temporary.path / "input.root"; + const auto replacement = temporary.path / "fixed.root"; + const auto backup = temporary.path / "input.root.bak"; + WriteText(original, "original"); + + REST_LegacyRecovery::RecoveryProvenance provenance; + provenance.formatVersion = REST_LegacyRecovery::kRecoveryFormatVersion; + provenance.kind = REST_LegacyRecovery::kResultProvenanceKind; + provenance.source.uuid = "source-uuid"; + provenance.source.normalizedPath = "/canonical/input.root"; + provenance.source.fileSize = 1234; + provenance.source.entries = 0; + provenance.source.signalVersion = 3; + provenance.intermediateUuid = "intermediate-uuid"; + provenance.intermediatePath = "/canonical/intermediate.root"; + + std::ostringstream writeErrors; + { + TFile candidate(replacement.string().c_str(), "CREATE"); + ASSERT_FALSE(candidate.IsZombie()); + provenance.resultUuid = candidate.GetUUID().AsString(); + ASSERT_TRUE(REST_LegacyRecovery::WriteRecoveryProvenance(candidate, provenance, writeErrors)) + << writeErrors.str(); + ASSERT_TRUE(REST_LegacyRecovery::CheckAndCloseOutputFile(candidate, writeErrors)) + << writeErrors.str(); + } + + REST_LegacyRecovery::CandidateExpectations expectations; + expectations.provenance = provenance; + const auto validateCandidate = [&expectations](const fs::path& path, std::ostream& errors) { + return REST_LegacyRecovery::ValidateRecoveryCandidate(path, expectations, errors); + }; + + std::ostringstream errors; + EXPECT_FALSE(REST_LegacyRecovery::ValidateAndReplaceFileWithBackup(replacement, original, backup, errors, + validateCandidate)); + EXPECT_EQ(ReadText(original), "original"); + EXPECT_TRUE(fs::is_regular_file(replacement)); + EXPECT_FALSE(fs::exists(backup)); + EXPECT_NE(errors.str().find("no readable EventTree"), std::string::npos); + EXPECT_NE(errors.str().find("were not touched"), std::string::npos); +} + } // namespace diff --git a/source/framework/test/src/LegacyRecoveryFileUtils.cxx b/source/framework/test/src/LegacyRecoveryFileUtils.cxx index f0a5feeb3..132fb0115 100644 --- a/source/framework/test/src/LegacyRecoveryFileUtils.cxx +++ b/source/framework/test/src/LegacyRecoveryFileUtils.cxx @@ -7,15 +7,28 @@ #include #include #include +#include #include #include +#include + +#include "../../../../macros/legacy/LegacyRecoveryDataUtils.h" +#include "../../../../macros/legacy/LegacyRecoveryProvenance.h" namespace { namespace fs = std::filesystem; using REST_LegacyRecovery::ComparePaths; +using REST_LegacyRecovery::ParseInteger; +using REST_LegacyRecovery::RecoveryProvenance; using REST_LegacyRecovery::ReplaceFileWithBackup; +using REST_LegacyRecovery::ResolvePathIdentity; +using REST_LegacyRecovery::SourceIdentity; +using REST_LegacyRecovery::ValidateAndReplaceFileWithBackup; +using REST_LegacyRecovery::ValidateFlattenedSignalData; +using REST_LegacyRecovery::ValidateIntermediateSource; using REST_LegacyRecovery::ValidateNewOutputPath; +using REST_LegacyRecovery::ValidateSupportedLegacySchema; class TemporaryDirectory { public: @@ -213,4 +226,179 @@ TEST(LegacyRecoveryFileUtils, ComparesNonexistentDestinationsWithoutThrowing) { EXPECT_TRUE(comparison.error.empty()); } +TEST(LegacyRecoveryFileUtils, ResolvesSymlinkSpellingsToTheSameSourceIdentity) { + TemporaryDirectory temporary; + const auto input = temporary.path / "input.root"; + const auto symlink = temporary.path / "input-link.root"; + WriteText(input, "input"); + + std::error_code symlinkError; + fs::create_symlink(input, symlink, symlinkError); + if (symlinkError) GTEST_SKIP() << "Cannot create symlink: " << symlinkError.message(); + + std::string inputIdentity; + std::string symlinkIdentity; + std::string error; + ASSERT_TRUE(ResolvePathIdentity(input, inputIdentity, error)) << error; + ASSERT_TRUE(ResolvePathIdentity(symlink, symlinkIdentity, error)) << error; + EXPECT_EQ(inputIdentity, symlinkIdentity); +} + +TEST(LegacyRecoveryDataUtils, AcceptsOnlyKnownLegacySchemaVersions) { + std::ostringstream errors; + EXPECT_TRUE(ValidateSupportedLegacySchema({1, 1}, errors, "test")); + EXPECT_TRUE(ValidateSupportedLegacySchema({2, 2}, errors, "test")); + EXPECT_TRUE(ValidateSupportedLegacySchema({3, 3}, errors, "test")); + + for (const auto versions : + {REST_LegacyRecovery::SignalSchemaVersions{-1, -1}, REST_LegacyRecovery::SignalSchemaVersions{0, 0}, + REST_LegacyRecovery::SignalSchemaVersions{4, 4}, REST_LegacyRecovery::SignalSchemaVersions{2, 3}}) { + errors.str(""); + EXPECT_FALSE(ValidateSupportedLegacySchema(versions, errors, "test")); + EXPECT_FALSE(errors.str().empty()); + } +} + +TEST(LegacyRecoveryDataUtils, RejectsMalformedFlattenedSignalArrays) { + const std::vector ids{7, 9}; + const std::vector counts{2, 1}; + const std::vector times{1.F, 2.F, 3.F}; + const std::vector charges{4.F, 5.F, 6.F}; + std::uint64_t points = 0; + std::ostringstream errors; + + EXPECT_TRUE(ValidateFlattenedSignalData(&ids, &counts, ×, &charges, points, errors, "test")); + EXPECT_EQ(points, 3U); + + const std::vector tooFewCounts{3}; + errors.str(""); + EXPECT_FALSE(ValidateFlattenedSignalData(&ids, &tooFewCounts, ×, &charges, points, errors, "test")); + + const std::vector tooFewCharges{4.F, 5.F}; + errors.str(""); + EXPECT_FALSE(ValidateFlattenedSignalData(&ids, &counts, ×, &tooFewCharges, points, errors, "test")); + + const std::vector nonFiniteTimes{1.F, std::numeric_limits::infinity(), 3.F}; + errors.str(""); + EXPECT_FALSE( + ValidateFlattenedSignalData(&ids, &counts, &nonFiniteTimes, &charges, points, errors, "test")); + + const std::vector negativeCounts{2, -1}; + errors.str(""); + EXPECT_FALSE( + ValidateFlattenedSignalData(&ids, &negativeCounts, ×, &charges, points, errors, "test")); + + const std::vector countsTooLarge{2, 2}; + errors.str(""); + EXPECT_FALSE( + ValidateFlattenedSignalData(&ids, &countsTooLarge, ×, &charges, points, errors, "test")); + + const std::vector countsTooSmall{1, 1}; + errors.str(""); + EXPECT_FALSE( + ValidateFlattenedSignalData(&ids, &countsTooSmall, ×, &charges, points, errors, "test")); + + const std::vector* nullIds = nullptr; + errors.str(""); + EXPECT_FALSE(ValidateFlattenedSignalData(nullIds, &counts, ×, &charges, points, errors, "test")); +} + +TEST(LegacyRecoveryDataUtils, RejectsMismatchedIntermediateProvenance) { + SourceIdentity source; + source.uuid = "source-uuid"; + source.normalizedPath = "/canonical/input.root"; + source.fileSize = 1234; + source.entries = 17; + source.signalVersion = 3; + + RecoveryProvenance provenance; + provenance.formatVersion = REST_LegacyRecovery::kRecoveryFormatVersion; + provenance.kind = REST_LegacyRecovery::kIntermediateProvenanceKind; + provenance.source = source; + provenance.intermediateUuid = "intermediate-uuid"; + + std::ostringstream errors; + EXPECT_TRUE(ValidateIntermediateSource(provenance, source, provenance.intermediateUuid, errors)); + + const auto expectRejected = [&](const RecoveryProvenance& changed) { + errors.str(""); + EXPECT_FALSE(ValidateIntermediateSource(changed, source, "intermediate-uuid", errors)); + EXPECT_FALSE(errors.str().empty()); + }; + + auto changed = provenance; + changed.formatVersion++; + expectRejected(changed); + changed = provenance; + changed.kind = REST_LegacyRecovery::kResultProvenanceKind; + expectRejected(changed); + changed = provenance; + changed.source.uuid = "wrong-source"; + expectRejected(changed); + changed = provenance; + changed.source.normalizedPath = "/different/input.root"; + expectRejected(changed); + changed = provenance; + changed.source.fileSize++; + expectRejected(changed); + changed = provenance; + changed.source.entries++; + expectRejected(changed); + changed = provenance; + changed.source.signalVersion--; + expectRejected(changed); + changed = provenance; + changed.intermediateUuid = "wrong-intermediate"; + expectRejected(changed); +} + +TEST(LegacyRecoveryProvenance, ParsesOnlyCompleteAndRepresentableIntegers) { + std::uint64_t unsignedValue = 0; + EXPECT_TRUE(ParseInteger("42", unsignedValue)); + EXPECT_EQ(unsignedValue, 42U); + EXPECT_FALSE(ParseInteger("", unsignedValue)); + EXPECT_FALSE(ParseInteger("-1", unsignedValue)); + EXPECT_FALSE(ParseInteger("42x", unsignedValue)); + EXPECT_FALSE(ParseInteger("18446744073709551616", unsignedValue)); + + int signedValue = 0; + EXPECT_TRUE(ParseInteger("-3", signedValue)); + EXPECT_EQ(signedValue, -3); + EXPECT_FALSE(ParseInteger(" 3", signedValue)); + EXPECT_FALSE(ParseInteger("2147483648", signedValue)); +} + +TEST(LegacyRecoveryFileUtils, FailedCandidateValidationLeavesAllFilesUntouched) { + TemporaryDirectory temporary; + const auto original = temporary.path / "input.root"; + const auto replacement = temporary.path / "fixed.root"; + const auto backup = temporary.path / "input.root.bak"; + WriteText(original, "original"); + WriteText(replacement, "invalid candidate"); + WriteText(backup, "previous backup"); + + int validationCalls = 0; + int renameCalls = 0; + const auto rejectCandidate = [&validationCalls](const fs::path& path, std::ostream& errors) { + validationCalls++; + errors << "candidate rejected: " << path.string() << '\n'; + return false; + }; + const auto countRename = [&renameCalls](const fs::path& source, const fs::path& destination, + std::error_code& error) { + renameCalls++; + fs::rename(source, destination, error); + }; + + std::ostringstream errors; + EXPECT_FALSE(ValidateAndReplaceFileWithBackup(replacement, original, backup, errors, rejectCandidate, + countRename)); + EXPECT_EQ(validationCalls, 1); + EXPECT_EQ(renameCalls, 0); + EXPECT_EQ(ReadText(original), "original"); + EXPECT_EQ(ReadText(replacement), "invalid candidate"); + EXPECT_EQ(ReadText(backup), "previous backup"); + EXPECT_NE(errors.str().find("were not touched"), std::string::npos); +} + } // namespace From e73f23fe2c8c0a0d18f8ffa67eed20f670bb201b Mon Sep 17 00:00:00 2001 From: Vindaar Date: Thu, 30 Jul 2026 20:51:45 +0200 Subject: [PATCH 09/10] Add safe one-command legacy signal recovery Expose restRoot --recover-legacy-signals INPUT [--output OUTPUT | --in-place] as an early one-shot command. Orchestrate a build-matched plain ROOT child and a fresh matching REST child without invoking a shell, transport user paths through child-only environment variables, disable startup files, forward signals to the complete child process group, and propagate macro exit statuses. Use canonical authenticated input paths, private mode-0700 same-filesystem work directories, unique candidates, strict complete rebuild validation, and atomic no-replace installation. Keep default sibling output beside the supplied input spelling, preserve basic file permissions for in-place replacement, retain diagnostics on failure, reject existing outputs and dangling symlinks, and never silently overwrite an output or backup created during a race. Preserve every highest-cycle top-level TTree beyond EventTree and AnalysisTree with a fast clone. Validate its key and object names, class, entry count, and recursive branch inventory so an incomplete candidate cannot replace the original. Document the schema transition, workflow, guarantees, and platform limitations. Cover parsing, exact process construction, special-character and symlink paths, environment isolation, executable search permissions, child-group signals and reaping, output races, extra-tree preservation, cleanup and retention, local-only policy, filename suffixes, and in-place replacement behavior. Point the unsafe-branch warning at the new command. --- doc/tutorials/List of REST Programs.md | 2 + ...Recovering legacy detector signal files.md | 112 ++++ .../LegacyRecoveryCandidateValidation.h | 137 ++++ macros/legacy/LegacyRecoveryFileUtils.h | 23 +- macros/legacy/REST_RebuildLegacySignalFile.C | 94 ++- macros/legacy/recoverLegacySignalData.C | 45 +- source/bin/CMakeLists.txt | 36 +- source/bin/LegacySignalRecoveryCLI.cxx | 586 ++++++++++++++++++ source/bin/LegacySignalRecoveryCLI.h | 65 ++ source/bin/restRoot.cxx | 33 + source/bin/test/LegacySignalRecoveryCLI.cxx | 413 ++++++++++++ .../test/LegacySignalRecoveryProcessProbe.cxx | 60 ++ source/framework/core/src/TRestRun.cxx | 10 +- .../test/integration/LegacyDetectorSignal.cxx | 94 +++ .../test/src/LegacyRecoveryFileUtils.cxx | 24 + 15 files changed, 1702 insertions(+), 32 deletions(-) create mode 100644 doc/tutorials/Recovering legacy detector signal files.md create mode 100644 source/bin/LegacySignalRecoveryCLI.cxx create mode 100644 source/bin/LegacySignalRecoveryCLI.h create mode 100644 source/bin/test/LegacySignalRecoveryCLI.cxx create mode 100644 source/bin/test/LegacySignalRecoveryProcessProbe.cxx diff --git a/doc/tutorials/List of REST Programs.md b/doc/tutorials/List of REST Programs.md index 5ac5693bd..7b6c01cb3 100644 --- a/doc/tutorials/List of REST Programs.md +++ b/doc/tutorials/List of REST Programs.md @@ -8,6 +8,8 @@ This is just an indexing list of existing programs included with REST compilatio **restManager** : The manager program of REST allowing to execute the REST event processes defined in TRestManager. Additional definitions allow to include additional metadata structures inside REST. +**restRoot** : The ROOT interactive shell with REST libraries and helpers loaded. It also provides the safe one-command legacy detector-signal recovery workflow described in [Recovering legacy detector signal files](). + **restPlots** : It uses the plot definitions given by RML configuration in a TRestAnalysisPlot section. It creates the plots from the variables at the TRestAnalysisTree and creates a PDF report and a ROOT file including the histograms created. ## Histogram and integration executables diff --git a/doc/tutorials/Recovering legacy detector signal files.md b/doc/tutorials/Recovering legacy detector signal files.md new file mode 100644 index 000000000..dbe527d91 --- /dev/null +++ b/doc/tutorials/Recovering legacy detector signal files.md @@ -0,0 +1,112 @@ +# Recovering legacy detector signal files + +REST files written with detectorlib versions before the `TRestDetectorSignal` +schema update stored signal time and charge arrays as `vector`. Current +detectorlib uses `vector`. Some old files do not contain enough ROOT +streamer information to convert that change safely; directly selecting the +legacy detector-signal branch can otherwise cause a very large allocation, +crash, or out-of-memory failure. + +The recovery command supports the known legacy signal schema versions 1, 2, +and 3: + +```sh +restRoot --recover-legacy-signals input.root +``` + +The default result is `input_Fixed.root`. The input is not modified. Choose a +different new output path with: + +```sh +restRoot --recover-legacy-signals input.root --output recovered.root +``` + +Existing outputs are never overwritten. Input and output must be local +filesystem paths. If the supplied input is a symbolic link, the default output +is created beside that link while the canonical target is used as the +authenticated source. In-place recovery through a symbolic link is refused. + +## In-place recovery + +In-place replacement is deliberately explicit: + +```sh +restRoot --recover-legacy-signals input.root --in-place +``` + +The rebuilt candidate is fully closed, reopened, and checked before the input +is changed. The original is then retained as `input.root.bak`, and its basic +filesystem permissions are applied to the replacement. Recovery refuses to +start if that backup path already exists. + +Use the default sibling output first when practical. In-place replacement +cannot preserve every platform-specific attribute, such as all ACLs, extended +attributes, or ownership changes that the current user cannot apply. + +## Why the command uses two processes + +Recovery has two intentionally isolated stages: + +1. A plain, build-matched ROOT process reads the old `vector` layout + with replica classes and writes a private intermediate file. ROOT startup + files are disabled so they cannot load REST classes into this process. +2. A fresh REST process authenticates that intermediate against the exact + source identity and rebuilds the branch with the current + `vector` classes. + +The command never sends filenames through a shell or a ROOT expression. Paths +are passed in the child environment, so spaces, quotes, and shell metacharacters +are treated as filename characters. It uses the ROOT binary and REST +installation that match the invoked installed `restRoot`, even if `PATH`, +`ROOTSYS`, or `REST_PATH` point elsewhere. + +Interrupt, termination, and hangup signals are forwarded to the complete +active child process group before the command reports the resulting status. + +The one-command workflow is strict: unreadable metadata, event branches, or +additional top-level trees make it fail instead of silently producing an +incomplete result. Every highest-cycle top-level tree is copied. Every +candidate is read back and its provenance, tree names, classes, entry counts, +branch inventories, signal schema, signal counts, and point counts are +checked. + +Recovery provenance prevents accidental source/intermediate mix-ups. It is an +integrity guard for the workflow, not a cryptographic signature against a +maliciously modified file. + +## Failures and cleanup + +A nonzero exit status means recovery did not complete. Candidate creation and +validation do not touch the original. Sibling-output installation uses an +atomic no-overwrite operation, so a destination created by another process is +not replaced. + +After validation, in-place installation moves the original to `.bak` and then +moves the candidate into place. If the second move fails, recovery attempts to +restore the original automatically. If that rollback also fails, the error +reports the exact original-at-backup and candidate paths; no file is silently +discarded. + +The command reports its private work directory after a stage failure and +retains it for diagnosis. It removes only its own work directory after +success. + +Check that: + +- there is enough free space beside the requested output; +- the output directory is writable; +- no requested output or `.bak` path already exists; +- the file contains a known legacy `TRestDetectorSignalEventBranch`; and +- REST was installed after building the framework and detectorlib versions + being used. + +Cross-process file locking and power-loss transaction guarantees are outside +the current workflow. Do not modify the source or destination concurrently. +The one-command orchestrator is currently available on POSIX systems; the +underlying manual macros remain available on other supported platforms. + +For detailed command help, run: + +```sh +restRoot --recover-legacy-signals --help +``` diff --git a/macros/legacy/LegacyRecoveryCandidateValidation.h b/macros/legacy/LegacyRecoveryCandidateValidation.h index 35bb15f7b..a1b5556b7 100644 --- a/macros/legacy/LegacyRecoveryCandidateValidation.h +++ b/macros/legacy/LegacyRecoveryCandidateValidation.h @@ -2,14 +2,19 @@ #define REST_LEGACY_RECOVERY_CANDIDATE_VALIDATION_H #include +#include #include +#include +#include #include #include +#include #include #include #include #include +#include #include #include @@ -56,12 +61,142 @@ inline SignalSchemaVersions DetectSignalSchemaVersions(TBranch* branch) { return versions; } +struct AdditionalTreeExpectation { + std::string keyName; + std::string objectName; + std::string className; + Long64_t entries = -1; + std::vector branches; +}; + +inline void CollectBranchInventory(const TObjArray* branches, const std::string& parent, + std::vector& inventory) { + if (branches == nullptr) return; + for (int index = 0; index <= branches->GetLast(); ++index) { + auto branch = dynamic_cast(branches->At(index)); + if (branch == nullptr) continue; + const std::string path = parent.empty() ? branch->GetName() : parent + "/" + branch->GetName(); + inventory.push_back(path); + CollectBranchInventory(branch->GetListOfBranches(), path, inventory); + } +} + +inline AdditionalTreeExpectation DescribeAdditionalTree(const std::string& keyName, TTree& tree) { + AdditionalTreeExpectation expectation; + expectation.keyName = keyName; + expectation.objectName = tree.GetName(); + expectation.className = tree.ClassName(); + expectation.entries = tree.GetEntries(); + CollectBranchInventory(tree.GetListOfBranches(), "", expectation.branches); + std::sort(expectation.branches.begin(), expectation.branches.end()); + return expectation; +} + +inline bool CopyAdditionalTopLevelTrees(TFile& source, TFile& destination, + std::vector& expectations, + std::ostream& errors) { + expectations.clear(); + std::set seen; + TIter nextKey(source.GetListOfKeys()); + TKey* key; + while ((key = static_cast(nextKey()))) { + const std::string keyName = key->GetName(); + if (!seen.insert(keyName).second) continue; // highest cycle only + if (keyName == "EventTree" || keyName == "AnalysisTree") continue; + + auto keyClass = TClass::GetClass(key->GetClassName()); + if (keyClass == nullptr || !keyClass->InheritsFrom(TTree::Class())) continue; + + std::unique_ptr object(key->ReadObj()); + auto tree = dynamic_cast(object.get()); + if (tree == nullptr) { + errors << "ERROR: cannot read additional top-level tree key '" << keyName << "' (" + << key->GetClassName() << ").\n"; + return false; + } + + const auto expectation = DescribeAdditionalTree(keyName, *tree); + destination.cd(); + auto clone = tree->CloneTree(-1, "fast"); + if (clone == nullptr || clone->GetEntries() != expectation.entries || + clone->Write(keyName.c_str(), TObject::kOverwrite) <= 0) { + errors << "ERROR: failed to clone and write additional top-level tree '" << keyName << "'.\n"; + return false; + } + expectations.push_back(expectation); + } + std::sort(expectations.begin(), expectations.end(), + [](const AdditionalTreeExpectation& first, const AdditionalTreeExpectation& second) { + return first.keyName < second.keyName; + }); + return true; +} + +inline bool ValidateAdditionalTopLevelTrees(TFile& candidate, + const std::vector& expected, + std::ostream& errors) { + std::set actualTreeKeys; + std::set seen; + TIter nextKey(candidate.GetListOfKeys()); + TKey* key; + while ((key = static_cast(nextKey()))) { + const std::string keyName = key->GetName(); + if (!seen.insert(keyName).second) continue; // highest cycle only + if (keyName == "EventTree" || keyName == "AnalysisTree") continue; + + auto keyClass = TClass::GetClass(key->GetClassName()); + if (keyClass != nullptr && keyClass->InheritsFrom(TTree::Class())) actualTreeKeys.insert(keyName); + } + + std::set expectedTreeKeys; + for (const auto& expectation : expected) { + if (!expectedTreeKeys.insert(expectation.keyName).second) { + errors << "ERROR: duplicate additional-tree expectation for key '" << expectation.keyName + << "'.\n"; + return false; + } + + auto tree = dynamic_cast(candidate.Get(expectation.keyName.c_str())); + if (tree == nullptr) { + errors << "ERROR: rebuilt candidate is missing additional top-level tree '" << expectation.keyName + << "'.\n"; + return false; + } + if (tree->GetName() != expectation.objectName || tree->ClassName() != expectation.className) { + errors << "ERROR: rebuilt candidate additional tree '" << expectation.keyName + << "' has a different object name or class.\n"; + return false; + } + if (tree->GetEntries() != expectation.entries) { + errors << "ERROR: rebuilt candidate additional tree '" << expectation.keyName << "' has " + << tree->GetEntries() << " entries, expected " << expectation.entries << ".\n"; + return false; + } + + std::vector actualBranches; + CollectBranchInventory(tree->GetListOfBranches(), "", actualBranches); + std::sort(actualBranches.begin(), actualBranches.end()); + if (actualBranches != expectation.branches) { + errors << "ERROR: rebuilt candidate additional tree '" << expectation.keyName + << "' has a different branch inventory.\n"; + return false; + } + } + + if (actualTreeKeys != expectedTreeKeys) { + errors << "ERROR: rebuilt candidate additional top-level tree inventory differs from the source.\n"; + return false; + } + return true; +} + struct CandidateExpectations { RecoveryProvenance provenance; Long64_t analysisEntries = -1; std::vector metadataKeys; std::vector otherEventBranches; std::vector analysisBranches; + std::vector additionalTrees; }; inline bool ValidateRecoveryCandidate(const std::filesystem::path& candidatePath, @@ -185,6 +320,8 @@ inline bool ValidateRecoveryCandidate(const std::filesystem::path& candidatePath } } + if (!ValidateAdditionalTopLevelTrees(*candidate, expected.additionalTrees, errors)) return false; + candidate->Close(); return true; } diff --git a/macros/legacy/LegacyRecoveryFileUtils.h b/macros/legacy/LegacyRecoveryFileUtils.h index 09f6a01ec..7a4ffc592 100644 --- a/macros/legacy/LegacyRecoveryFileUtils.h +++ b/macros/legacy/LegacyRecoveryFileUtils.h @@ -17,6 +17,16 @@ struct PathComparison { std::string error; }; +inline fs::path BuildSiblingRootPath(const fs::path& input, const std::string& suffix) { + fs::path output = input; + const auto filename = output.filename(); + if (filename.extension() == ".root") + output.replace_filename(filename.stem().string() + suffix + ".root"); + else + output += suffix + ".root"; + return output; +} + inline bool ResolvePathIdentity(const fs::path& path, std::string& identity, std::string& error) { std::error_code absoluteError; const auto absolute = fs::absolute(path, absoluteError); @@ -127,7 +137,18 @@ using RenameOperation = std::function; inline void RenamePath(const fs::path& source, const fs::path& destination, std::error_code& error) { - fs::rename(source, destination, error); + // Recovery operates on regular files in one directory. A hard link plus + // unlink provides no-replace semantics on POSIX, unlike rename(), which + // would silently overwrite a path created after the preflight check. + fs::create_hard_link(source, destination, error); + if (error) return; + + std::error_code removeError; + if (fs::remove(source, removeError)) return; + + std::error_code rollbackError; + fs::remove(destination, rollbackError); + error = removeError ? removeError : std::make_error_code(std::errc::io_error); } inline bool ReplaceFileWithBackup(const fs::path& replacementPath, const fs::path& originalPath, diff --git a/macros/legacy/REST_RebuildLegacySignalFile.C b/macros/legacy/REST_RebuildLegacySignalFile.C index 3f32b9f12..7c90e250b 100644 --- a/macros/legacy/REST_RebuildLegacySignalFile.C +++ b/macros/legacy/REST_RebuildLegacySignalFile.C @@ -5,7 +5,8 @@ // file in which: // - the TRestDetectorSignalEventBranch is rebuilt with the current // vector-based classes, -// - all other EventTree branches and the AnalysisTree are copied unchanged, +// - all other EventTree branches, the AnalysisTree, and every additional +// top-level TTree (highest key cycle) are copied unchanged, // - all readable metadata keys (TRestRun, readout, processes, ...) are copied, // - the event-class StreamerInfos ARE stored (they are missing from legacy // restManager output, which is what made these files unreadable in the @@ -33,7 +34,8 @@ // Arguments: // originalFile - the legacy REST file // signalDataFile - intermediate from stage 1; default: _LegacySignalData.root -// outputFile - default: _Fixed.root (ignored when overwrite=true) +// outputFile - default: _Fixed.root, or _FixedTmp.root +// when overwrite=true; an explicit path is always honored // overwrite - replace originalFile in place, keeping a .bak copy #include @@ -45,6 +47,7 @@ #include #include +#include #include #include #include @@ -58,6 +61,9 @@ namespace REST_Rebuild_Internal { +int gLegacySignalRebuildStatus = 1; +bool gRequireCompleteRecovery = false; + void SetBranchStatusRecursive(TBranch* branch, Bool_t status) { if (branch == nullptr) return; branch->SetStatus(status); @@ -143,17 +149,16 @@ bool ScanIntermediateTree(TTree* tree, IntermediateData& values, REST_LegacyReco void REST_RebuildLegacySignalFile(const char* originalFile, const char* signalDataFile = "", const char* outputFile = "", bool overwrite = false) { using namespace REST_Rebuild_Internal; - - std::string base = originalFile; - const size_t pos = base.rfind(".root"); - if (pos != std::string::npos) base = base.substr(0, pos); + gLegacySignalRebuildStatus = 1; std::string dataName = signalDataFile; - if (dataName.empty()) dataName = base + "_LegacySignalData.root"; + if (dataName.empty()) + dataName = REST_LegacyRecovery::BuildSiblingRootPath(originalFile, "_LegacySignalData").string(); std::string outName = outputFile; - if (outName.empty()) outName = base + "_Fixed.root"; - if (overwrite) outName = base + "_FixedTmp.root"; + if (outName.empty()) + outName = REST_LegacyRecovery::BuildSiblingRootPath(originalFile, overwrite ? "_FixedTmp" : "_Fixed") + .string(); if (!REST_LegacyRecovery::ValidateNewOutputPath(originalFile, outName, "fixed output", std::cout)) { return; @@ -176,9 +181,9 @@ void REST_RebuildLegacySignalFile(const char* originalFile, const char* signalDa std::unique_ptr data(TFile::Open(dataName.c_str())); if (data == nullptr || data->IsZombie()) { std::cout << "ERROR: cannot open signal data file: " << dataName << std::endl; - std::cout << "Run stage 1 first (with plain root, NOT restRoot):" << std::endl; - std::cout << " root -l -b -q 'recoverLegacySignalData.C+(\"" << originalFile << "\")'" - << std::endl; + std::cout << "Run recoverLegacySignalData.C first with plain root (NOT restRoot)." << std::endl; + std::cout << " original: " << originalFile << std::endl; + std::cout << " intermediate: " << dataName << std::endl; return; } @@ -449,6 +454,13 @@ void REST_RebuildLegacySignalFile(const char* originalFile, const char* signalDa std::cout << "WARNING: no AnalysisTree found; skipping." << std::endl; } + // --- copy every other top-level tree, using the highest key cycle --- + std::vector additionalTrees; + if (!REST_LegacyRecovery::CopyAdditionalTopLevelTrees(*original, *out, additionalTrees, std::cout)) { + REST_LegacyRecovery::CheckAndCloseOutputFile(*out, std::cout); + return; + } + REST_LegacyRecovery::RecoveryProvenance resultProvenance = intermediateProvenance; resultProvenance.kind = REST_LegacyRecovery::kResultProvenanceKind; resultProvenance.recovered = scannedCounts; @@ -478,6 +490,12 @@ void REST_RebuildLegacySignalFile(const char* originalFile, const char* signalDa << " event branch(es) had no loaded dictionary and were NOT copied:" << std::endl; for (const auto& s : skippedEventBranches) std::cout << " - " << s << std::endl; } + if (gRequireCompleteRecovery && (!skipped.empty() || !skippedEventBranches.empty())) { + std::cout + << "ERROR: strict recovery refuses a rebuilt file that omitted metadata or event branches.\n" + << "The original is unchanged. Inspect the candidate file at: " << outName << std::endl; + return; + } // --- overwrite handling --- std::string finalName = outName; @@ -487,6 +505,7 @@ void REST_RebuildLegacySignalFile(const char* originalFile, const char* signalDa candidateExpectations.metadataKeys = copiedMetadataKeys; candidateExpectations.otherEventBranches = otherBranchNames; candidateExpectations.analysisBranches = analysisBranchNames; + candidateExpectations.additionalTrees = additionalTrees; const auto validateCandidate = [&candidateExpectations](const std::filesystem::path& path, std::ostream& errors) { return REST_LegacyRecovery::ValidateRecoveryCandidate(path, candidateExpectations, errors); @@ -499,6 +518,20 @@ void REST_RebuildLegacySignalFile(const char* originalFile, const char* signalDa << "The original is unchanged. Inspect the candidate file at: " << outName << std::endl; return; } + std::error_code permissionError; + const auto originalPermissions = std::filesystem::status(originalFile, permissionError).permissions(); + if (permissionError) { + std::cout << "ERROR: cannot read original file permissions: " << permissionError.message() + << ". The original is unchanged." << std::endl; + return; + } + std::filesystem::permissions(outName, originalPermissions, std::filesystem::perm_options::replace, + permissionError); + if (permissionError) { + std::cout << "ERROR: cannot preserve original file permissions on the rebuilt candidate: " + << permissionError.message() << ". The original is unchanged." << std::endl; + return; + } if (!REST_LegacyRecovery::ValidateAndReplaceFileWithBackup(outName, originalFile, backupName, std::cout, validateCandidate)) { return; @@ -516,4 +549,41 @@ void REST_RebuildLegacySignalFile(const char* originalFile, const char* signalDa std::cout << "Rebuilt " << nEntries << " entries: " << scannedCounts.signals << " signals, " << scannedCounts.points << " points." << std::endl; std::cout << "Fixed file written to: " << finalName << std::endl; + gLegacySignalRebuildStatus = 0; +} + +int REST_RebuildLegacySignalFileWithStatus(const char* originalFile, const char* signalDataFile = "", + const char* outputFile = "", bool overwrite = false, + bool requireComplete = false) { + REST_Rebuild_Internal::gRequireCompleteRecovery = requireComplete; + REST_RebuildLegacySignalFile(originalFile, signalDataFile, outputFile, overwrite); + REST_Rebuild_Internal::gRequireCompleteRecovery = false; + return REST_Rebuild_Internal::gLegacySignalRebuildStatus; +} + +// No-argument entry point used only by the restRoot one-command orchestrator. +// Paths come from the child environment and never enter ROOT's command parser. +void REST_RebuildLegacySignalFile() { + const char* input = gSystem->Getenv("REST_LEGACY_RECOVERY_INPUT"); + const char* intermediate = gSystem->Getenv("REST_LEGACY_RECOVERY_INTERMEDIATE"); + const char* output = gSystem->Getenv("REST_LEGACY_RECOVERY_OUTPUT"); + const char* inPlaceValue = gSystem->Getenv("REST_LEGACY_RECOVERY_IN_PLACE"); + const char* requireCompleteValue = gSystem->Getenv("REST_LEGACY_RECOVERY_REQUIRE_COMPLETE"); + if (input == nullptr || intermediate == nullptr || output == nullptr || inPlaceValue == nullptr || + requireCompleteValue == nullptr) { + std::cerr << "ERROR: incomplete stage-2 recovery environment." << std::endl; + gSystem->Exit(64); + return; + } + const std::string inPlaceText(inPlaceValue); + const std::string requireCompleteText(requireCompleteValue); + if ((inPlaceText != "0" && inPlaceText != "1") || + (requireCompleteText != "0" && requireCompleteText != "1")) { + std::cerr << "ERROR: invalid boolean value in the stage-2 recovery environment." << std::endl; + gSystem->Exit(64); + return; + } + + gSystem->Exit(REST_RebuildLegacySignalFileWithStatus(input, intermediate, output, inPlaceText == "1", + requireCompleteText == "1")); } diff --git a/macros/legacy/recoverLegacySignalData.C b/macros/legacy/recoverLegacySignalData.C index 7e9b4a0e1..78d3594b2 100644 --- a/macros/legacy/recoverLegacySignalData.C +++ b/macros/legacy/recoverLegacySignalData.C @@ -122,6 +122,10 @@ static REST_LegacyRecovery::SignalSchemaVersions GetOnDiskSignalVersions(TBranch return versions; } +namespace { +int gLegacySignalExtractionStatus = 1; +} + static bool ValidateLegacyEvent(const TRestDetectorSignalEvent* event, Long64_t entry, std::uint64_t& signalCount, std::uint64_t& pointCount, Long64_t* suspectValues, std::ostream& errors) { @@ -168,23 +172,21 @@ static bool ValidateLegacyEvent(const TRestDetectorSignalEvent* event, Long64_t } void recoverLegacySignalData(const char* inputFile, const char* outputFile = "") { + gLegacySignalExtractionStatus = 1; // Refuse to run if the real REST libraries are loaded (restRoot session): // the replica classes above would clash with the compiled ones. TString loadedLibraries = gSystem->GetLibraries(); if (loadedLibraries.Contains("libRestFramework") || loadedLibraries.Contains("libRestDetector")) { std::cout << "ERROR: REST libraries are loaded in this session." << std::endl; - std::cout << "Run this macro with plain root, not restRoot:" << std::endl; - std::cout << " root -l -b -q 'recoverLegacySignalData.C+(\"" << inputFile << "\")'" << std::endl; + std::cout << "Exit this session and use the isolated one-command workflow:" << std::endl; + std::cout << " restRoot --recover-legacy-signals INPUT" << std::endl; + std::cout << " input: " << inputFile << std::endl; return; } std::string outName = outputFile; - if (outName.empty()) { - outName = inputFile; - const size_t pos = outName.rfind(".root"); - if (pos != std::string::npos) outName = outName.substr(0, pos); - outName += "_LegacySignalData.root"; - } + if (outName.empty()) + outName = REST_LegacyRecovery::BuildSiblingRootPath(inputFile, "_LegacySignalData").string(); if (!REST_LegacyRecovery::ValidateNewOutputPath(inputFile, outName, "legacy signal data output", std::cout)) { return; @@ -379,7 +381,28 @@ void recoverLegacySignalData(const char* inputFile, const char* outputFile = "") << " suspicious values (|v| > 1e12) found — the recovered data may be corrupted!" << std::endl; std::cout << "Signal data written to: " << outName << std::endl; - std::cout << std::endl; - std::cout << "Next step — rebuild the fixed file with restRoot:" << std::endl; - std::cout << " restRoot -b -q 'REST_RebuildLegacySignalFile.C(\"" << inputFile << "\")'" << std::endl; + std::cout << "Run REST_RebuildLegacySignalFile.C in a fresh restRoot process to rebuild the final file." + << std::endl; + std::cout << " original: " << inputFile << std::endl; + std::cout << " intermediate: " << outName << std::endl; + gLegacySignalExtractionStatus = 0; +} + +int recoverLegacySignalDataWithStatus(const char* inputFile, const char* outputFile = "") { + recoverLegacySignalData(inputFile, outputFile); + return gLegacySignalExtractionStatus; +} + +// No-argument entry point used only by the restRoot one-command orchestrator. +// Paths come from the child environment and never enter ROOT's command parser. +void recoverLegacySignalData() { + const char* input = gSystem->Getenv("REST_LEGACY_RECOVERY_INPUT"); + const char* intermediate = gSystem->Getenv("REST_LEGACY_RECOVERY_INTERMEDIATE"); + const char* workDirectory = gSystem->Getenv("REST_LEGACY_RECOVERY_WORK_DIR"); + if (input == nullptr || intermediate == nullptr || workDirectory == nullptr) { + std::cerr << "ERROR: incomplete stage-1 recovery environment." << std::endl; + gSystem->Exit(64); + return; + } + gSystem->Exit(recoverLegacySignalDataWithStatus(input, intermediate)); } diff --git a/source/bin/CMakeLists.txt b/source/bin/CMakeLists.txt index 4d5599f65..282629a99 100644 --- a/source/bin/CMakeLists.txt +++ b/source/bin/CMakeLists.txt @@ -10,13 +10,47 @@ foreach (file ${files}) string(REGEX MATCH "[^/\\]*cxx" temp ${file}) string(REPLACE ".cxx" "" name ${temp}) - add_executable(${name} ${file}) + if (name STREQUAL "restRoot") + add_executable(${name} ${file} LegacySignalRecoveryCLI.cxx) + target_compile_definitions( + ${name} PRIVATE REST_ROOT_EXECUTABLE="${ROOT_root_CMD}") + else () + add_executable(${name} ${file}) + endif () target_link_libraries(${name} ${rest_libraries} ${external_libs}) install(TARGETS ${name} RUNTIME DESTINATION bin) set(rest_exes ${rest_exes} ${name}) endforeach (file) +if (TEST) + set(LEGACY_SIGNAL_RECOVERY_CLI_TEST_SOURCE + test/LegacySignalRecoveryCLI.cxx) + add_executable( + testRestRootLegacySignalRecoveryProcessProbe + test/LegacySignalRecoveryProcessProbe.cxx) + add_executable( + testRestRootLegacySignalRecovery + ${LEGACY_SIGNAL_RECOVERY_CLI_TEST_SOURCE} + LegacySignalRecoveryCLI.cxx) + target_include_directories( + testRestRootLegacySignalRecovery PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}) + target_link_libraries( + testRestRootLegacySignalRecovery PRIVATE gtest_main) + target_compile_definitions( + testRestRootLegacySignalRecovery + PRIVATE + REST_LEGACY_RECOVERY_PROCESS_PROBE="$" + ) + add_dependencies( + testRestRootLegacySignalRecovery + testRestRootLegacySignalRecoveryProcessProbe) + include(GoogleTest) + gtest_add_tests( + TARGET testRestRootLegacySignalRecovery + SOURCES ${LEGACY_SIGNAL_RECOVERY_CLI_TEST_SOURCE}) +endif () + set(rest_exes ${rest_exes} PARENT_SCOPE) diff --git a/source/bin/LegacySignalRecoveryCLI.cxx b/source/bin/LegacySignalRecoveryCLI.cxx new file mode 100644 index 000000000..0fe58553a --- /dev/null +++ b/source/bin/LegacySignalRecoveryCLI.cxx @@ -0,0 +1,586 @@ +#include "LegacySignalRecoveryCLI.h" + +#include +#include +#include +#include +#include +#include +#include + +#ifdef _WIN32 +#include +#else +#include +#include +#include +#include +#endif + +namespace REST_LegacySignalRecoveryCLI { +namespace { + +constexpr int kUsageError = 2; +constexpr int kSetupError = 3; +constexpr int kUnsupportedError = 4; + +std::filesystem::path DefaultFixedOutput(const std::filesystem::path& input) { + std::filesystem::path output = input; + if (output.extension() == ".root") { + output.replace_filename(output.stem().string() + "_Fixed.root"); + } else { + output += "_Fixed.root"; + } + return output; +} + +bool PathEntryExists(const std::filesystem::path& path, std::error_code& error) { + const auto status = std::filesystem::symlink_status(path, error); + if (status.type() == std::filesystem::file_type::not_found) { + error.clear(); + return false; + } + return !error; +} + +bool RequireUnused(const std::filesystem::path& path, const char* description, std::ostream& errors) { + std::error_code error; + const bool exists = PathEntryExists(path, error); + if (error) { + errors << "ERROR: cannot inspect " << description << " '" << path.string() << "': " << error.message() + << ".\n"; + return false; + } + if (exists) { + errors << "ERROR: " << description << " already exists: " << path.string() << "\n"; + return false; + } + return true; +} + +bool RequireMacro(const std::filesystem::path& path, std::ostream& errors) { + std::error_code error; + if (std::filesystem::is_regular_file(path, error) && !error) return true; + errors << "ERROR: recovery macro is not installed at " << path.string() << ".\n" + << "Check REST_PATH and reinstall REST if necessary.\n"; + return false; +} + +void ReportRetainedWorkDirectory(const std::filesystem::path& workDirectory, std::ostream& errors) { + errors << "Recovery work files were retained for diagnosis at: " << workDirectory.string() << "\n"; +} + +std::string PrependSearchPath(const std::filesystem::path& entry, const char* variable) { + const char* inherited = std::getenv(variable); +#ifdef _WIN32 + constexpr char separator = ';'; +#else + constexpr char separator = ':'; +#endif + return inherited == nullptr || inherited[0] == '\0' ? entry.string() + : entry.string() + separator + std::string(inherited); +} + +#ifndef _WIN32 +volatile std::sig_atomic_t gActiveChildProcessGroup = -1; + +void ForwardSignalToChild(int signal) { + const auto group = gActiveChildProcessGroup; + if (group > 0) kill(-group, signal); +} + +class SignalForwardingGuard { + public: + explicit SignalForwardingGuard(pid_t child) { + gActiveChildProcessGroup = child; + struct sigaction action{}; + action.sa_handler = ForwardSignalToChild; + sigemptyset(&action.sa_mask); + action.sa_flags = 0; + sigaction(SIGINT, &action, &fPreviousInterrupt); + sigaction(SIGTERM, &action, &fPreviousTerminate); + sigaction(SIGHUP, &action, &fPreviousHangup); + } + + ~SignalForwardingGuard() { + gActiveChildProcessGroup = -1; + sigaction(SIGINT, &fPreviousInterrupt, nullptr); + sigaction(SIGTERM, &fPreviousTerminate, nullptr); + sigaction(SIGHUP, &fPreviousHangup, nullptr); + } + + private: + struct sigaction fPreviousInterrupt{}; + struct sigaction fPreviousTerminate{}; + struct sigaction fPreviousHangup{}; +}; +#endif + +} // namespace + +ParseResult ParseArguments(const Arguments& arguments) { + ParseResult result; + auto recoveryFlag = arguments.end(); + for (auto argument = arguments.begin() + (arguments.empty() ? 0 : 1); argument != arguments.end(); + ++argument) { + if (*argument == "--recover-legacy-signals") { + recoveryFlag = argument; + break; + } + } + if (recoveryFlag == arguments.end()) return result; + if (recoveryFlag != arguments.begin() + 1) { + result.action = ParseAction::kError; + result.error = "--recover-legacy-signals must be the first restRoot option"; + return result; + } + + if (arguments.size() == 3 && (arguments[2] == "--help" || arguments[2] == "-h")) { + result.action = ParseAction::kHelp; + return result; + } + + result.action = ParseAction::kError; + bool positionalOnly = false; + for (std::size_t index = 2; index < arguments.size(); ++index) { + const auto& argument = arguments[index]; + if (!positionalOnly && argument == "--") { + positionalOnly = true; + } else if (!positionalOnly && argument == "--in-place") { + if (result.options.inPlace) { + result.error = "--in-place was specified more than once"; + return result; + } + result.options.inPlace = true; + } else if (!positionalOnly && argument == "--output") { + if (result.options.outputWasSpecified) { + result.error = "--output was specified more than once"; + return result; + } + if (++index >= arguments.size()) { + result.error = "--output requires a path"; + return result; + } + if (arguments[index].rfind("-", 0) == 0) { + result.error = "--output requires a path (prefix an option-like filename with ./)"; + return result; + } + result.options.output = arguments[index]; + result.options.outputWasSpecified = true; + } else if (!positionalOnly && argument.rfind("-", 0) == 0) { + result.error = "unknown recovery option: " + argument; + return result; + } else if (result.options.input.empty()) { + result.options.input = argument; + } else { + result.error = "more than one input file was specified"; + return result; + } + } + + if (result.options.input.empty()) { + result.error = "an input ROOT file is required"; + } else if (result.options.inPlace && result.options.outputWasSpecified) { + result.error = "--output and --in-place are mutually exclusive"; + } else if (result.options.outputWasSpecified && result.options.output.empty()) { + result.error = "--output requires a non-empty path"; + } else { + result.action = ParseAction::kRun; + } + return result; +} + +void PrintHelp(std::ostream& output) { + output << "Usage:\n" + << " restRoot --recover-legacy-signals INPUT [--output OUTPUT | --in-place]\n\n" + << "Safely convert a legacy TRestDetectorSignalEvent branch from vector\n" + << "to the current vector schema. Stage 1 runs in an isolated plain ROOT\n" + << "process; stage 2 runs in a fresh REST process.\n\n" + << "By default the fixed file is written beside INPUT as _Fixed.root.\n" + << "Existing outputs are never overwritten. --in-place must be explicit and\n" + << "keeps the original as INPUT.bak.\n"; +} + +ProcessSpec BuildStage1Process(const Runtime& runtime, const std::filesystem::path& wrapper, + const std::filesystem::path& workDirectory, const std::filesystem::path& input, + const std::filesystem::path& intermediate) { + ProcessSpec process; + const auto rootPath = std::filesystem::path(runtime.rootExecutable).parent_path().parent_path(); + process.arguments = {runtime.rootExecutable, + "-l", + "-b", + "-n", + "-x", + "-q", + "-e", + "gSystem->SetBuildDir(gSystem->Getenv(\"REST_LEGACY_RECOVERY_WORK_DIR\"),kTRUE)", + wrapper.string() + "+"}; + process.environment = {{"REST_LEGACY_RECOVERY_INPUT", input.string()}, + {"REST_LEGACY_RECOVERY_INTERMEDIATE", intermediate.string()}, + {"REST_LEGACY_RECOVERY_WORK_DIR", workDirectory.string()}, + {"ROOTSYS", rootPath.string()}, + {"PATH", PrependSearchPath(rootPath / "bin", "PATH")}, + {"LD_LIBRARY_PATH", PrependSearchPath(rootPath / "lib", "LD_LIBRARY_PATH")}}; +#ifdef __APPLE__ + process.environment.emplace_back("DYLD_LIBRARY_PATH", + PrependSearchPath(rootPath / "lib", "DYLD_LIBRARY_PATH")); +#endif + return process; +} + +ProcessSpec BuildStage2Process(const Runtime& runtime, const std::filesystem::path& wrapper, + const std::filesystem::path& input, const std::filesystem::path& intermediate, + const std::filesystem::path& output, bool inPlace) { + ProcessSpec process; + process.arguments = {runtime.restRootExecutable, "-l", "-b", "-n", "-x", "-q", wrapper.string()}; + process.environment = { + {"REST_PATH", runtime.restPath.string()}, + {"PATH", PrependSearchPath(runtime.restPath / "bin", "PATH")}, + {"LD_LIBRARY_PATH", PrependSearchPath(runtime.restPath / "lib", "LD_LIBRARY_PATH")}, + {"REST_LEGACY_RECOVERY_INPUT", input.string()}, + {"REST_LEGACY_RECOVERY_INTERMEDIATE", intermediate.string()}, + {"REST_LEGACY_RECOVERY_OUTPUT", output.string()}, + {"REST_LEGACY_RECOVERY_IN_PLACE", inPlace ? "1" : "0"}, + {"REST_LEGACY_RECOVERY_REQUIRE_COMPLETE", "1"}}; +#ifdef __APPLE__ + process.environment.emplace_back("DYLD_LIBRARY_PATH", + PrependSearchPath(runtime.restPath / "lib", "DYLD_LIBRARY_PATH")); +#endif + return process; +} + +int RunChildProcess(const ProcessSpec& process) { + if (process.arguments.empty() || process.arguments.front().empty()) return 127; + +#ifdef _WIN32 + return kUnsupportedError; +#else + std::vector childArguments; + childArguments.reserve(process.arguments.size() + 1); + for (const auto& argument : process.arguments) + childArguments.push_back(const_cast(argument.c_str())); + childArguments.push_back(nullptr); + + sigset_t forwardedSignals; + sigemptyset(&forwardedSignals); + sigaddset(&forwardedSignals, SIGINT); + sigaddset(&forwardedSignals, SIGTERM); + sigaddset(&forwardedSignals, SIGHUP); + sigset_t previousMask; + if (sigprocmask(SIG_BLOCK, &forwardedSignals, &previousMask) != 0) return 127; + + const pid_t child = fork(); + if (child < 0) { + sigprocmask(SIG_SETMASK, &previousMask, nullptr); + return 127; + } + if (child == 0) { + if (setpgid(0, 0) != 0 || sigprocmask(SIG_SETMASK, &previousMask, nullptr) != 0) _exit(126); + for (const auto& variable : process.environment) { + if (setenv(variable.first.c_str(), variable.second.c_str(), 1) != 0) _exit(126); + } + execvp(childArguments.front(), childArguments.data()); + _exit(errno == ENOENT ? 127 : 126); + } + + setpgid(child, child); + int status = 0; + int result = 127; + { + // Install forwarding while the relevant signals remain blocked. A + // signal received after fork is delivered only after the handler and + // child process group are ready. + SignalForwardingGuard signalForwarding(child); + if (sigprocmask(SIG_SETMASK, &previousMask, nullptr) == 0) { + pid_t waited; + do { + waited = waitpid(child, &status, 0); + } while (waited < 0 && errno == EINTR); + if (waited == child) { + if (WIFEXITED(status)) + result = WEXITSTATUS(status); + else if (WIFSIGNALED(status)) + result = 128 + WTERMSIG(status); + } + } else { + kill(-child, SIGTERM); + while (waitpid(child, &status, 0) < 0 && errno == EINTR) { + } + } + // Restore the previous handlers only while forwarding signals are + // blocked again, then restore the caller's exact original mask. + sigprocmask(SIG_BLOCK, &forwardedSignals, nullptr); + } + sigprocmask(SIG_SETMASK, &previousMask, nullptr); + return result; +#endif +} + +std::string ResolveExecutableFromSearchPath(const std::string& executable, const std::string& searchPath) { + const std::filesystem::path path(executable); +#ifdef _WIN32 + constexpr char separator = ';'; +#else + constexpr char separator = ':'; +#endif + std::istringstream entries(searchPath); + std::string entry; + std::error_code error; + while (std::getline(entries, entry, separator)) { + if (entry.empty()) entry = "."; + const auto candidate = std::filesystem::path(entry) / path; + bool usable = std::filesystem::is_regular_file(candidate, error) && !error; +#ifndef _WIN32 + usable = usable && access(candidate.c_str(), X_OK) == 0; +#endif + if (usable) { + const auto resolved = std::filesystem::canonical(candidate, error); + return error ? std::filesystem::absolute(candidate).lexically_normal().string() + : resolved.string(); + } + error.clear(); + } + return executable; +} + +std::string ResolveExecutablePath(const std::string& executable) { + std::error_code error; +#ifdef __linux__ + const auto procExecutable = std::filesystem::read_symlink("/proc/self/exe", error); + if (!error && !procExecutable.empty()) return procExecutable.string(); + error.clear(); +#endif + const std::filesystem::path path(executable); + if (path.has_parent_path()) { + const auto resolved = std::filesystem::canonical(path, error); + if (!error) return resolved.string(); + return std::filesystem::absolute(path).lexically_normal().string(); + } + + const char* searchPath = std::getenv("PATH"); + if (searchPath != nullptr) return ResolveExecutableFromSearchPath(executable, searchPath); + return executable; +} + +bool CreateUniqueWorkDirectory(const std::filesystem::path& parent, std::filesystem::path& result, + std::string& error) { + std::error_code filesystemError; + if (!std::filesystem::is_directory(parent, filesystemError) || filesystemError) { + error = "working directory parent is unavailable: " + parent.string(); + return false; + } + + const auto timestamp = std::chrono::steady_clock::now().time_since_epoch().count(); + for (unsigned int attempt = 0; attempt < 128; ++attempt) { + std::ostringstream name; + name << ".rest-legacy-signals-" << std::hex << timestamp << '-' << attempt; + const auto candidate = parent / name.str(); + filesystemError.clear(); +#ifdef _WIN32 + const bool created = std::filesystem::create_directory(candidate, filesystemError); +#else + const bool created = mkdir(candidate.c_str(), S_IRWXU) == 0; + if (!created && errno != EEXIST) filesystemError = std::error_code(errno, std::generic_category()); +#endif + if (created) { + std::filesystem::permissions(candidate, std::filesystem::perms::owner_all, + std::filesystem::perm_options::replace, filesystemError); + if (filesystemError) { + std::filesystem::remove(candidate); + error = + "cannot restrict permissions on recovery work directory: " + filesystemError.message(); + return false; + } +#ifndef _WIN32 + struct stat directoryStatus{}; + if (stat(candidate.c_str(), &directoryStatus) != 0 || + (directoryStatus.st_mode & 0777) != S_IRWXU) { + std::filesystem::remove(candidate); + error = "recovery work directory does not have mode 0700"; + return false; + } +#endif + result = candidate; + return true; + } + if (filesystemError && filesystemError != std::errc::file_exists) { + error = "cannot create recovery work directory in " + parent.string() + ": " + + filesystemError.message(); + return false; + } + } + error = "cannot allocate a unique recovery work directory in " + parent.string(); + return false; +} + +int Execute(const Options& options, Runtime runtime, std::ostream& output, std::ostream& errors) { +#ifdef _WIN32 + errors << "ERROR: legacy signal recovery orchestration is not supported on Windows yet.\n"; + return kUnsupportedError; +#endif + if (options.input.empty()) { + errors << "ERROR: an input ROOT file is required.\n"; + return kUsageError; + } + if (options.inPlace && options.outputWasSpecified) { + errors << "ERROR: --output and --in-place are mutually exclusive.\n"; + return kUsageError; + } + if (!runtime.runProcess) runtime.runProcess = RunChildProcess; + if (!runtime.createWorkDirectory) runtime.createWorkDirectory = CreateUniqueWorkDirectory; + + const auto inputText = options.input.string(); + if (inputText.find("://") != std::string::npos || inputText.rfind("file:", 0) == 0) { + errors << "ERROR: recovery accepts local filesystem paths only, not URLs or ROOT protocols.\n"; + return kUsageError; + } + + std::error_code filesystemError; + if (!std::filesystem::is_regular_file(options.input, filesystemError) || filesystemError) { + errors << "ERROR: input file is not a readable regular file: " << options.input.string() << "\n"; + return kSetupError; + } + if (options.inPlace && std::filesystem::is_symlink(options.input, filesystemError) && !filesystemError) { + errors << "ERROR: in-place recovery through a symbolic link is not supported; use the target path.\n"; + return kSetupError; + } + + filesystemError.clear(); + const auto suppliedInputPath = + std::filesystem::absolute(options.input, filesystemError).lexically_normal(); + if (filesystemError) { + errors << "ERROR: cannot resolve supplied input path '" << options.input.string() + << "': " << filesystemError.message() << ".\n"; + return kSetupError; + } + const auto inputPath = std::filesystem::canonical(suppliedInputPath, filesystemError); + if (filesystemError) { + errors << "ERROR: cannot resolve input file '" << options.input.string() + << "': " << filesystemError.message() << ".\n"; + return kSetupError; + } + auto outputPath = options.inPlace ? inputPath + : (options.outputWasSpecified ? options.output + : DefaultFixedOutput(suppliedInputPath)); + if (!options.inPlace) { + const auto outputText = outputPath.string(); + if (outputText.find("://") != std::string::npos || outputText.rfind("file:", 0) == 0) { + errors << "ERROR: recovery output must be a local filesystem path.\n"; + return kUsageError; + } + outputPath = + std::filesystem::weakly_canonical(std::filesystem::absolute(outputPath), filesystemError); + if (filesystemError) { + errors << "ERROR: cannot resolve fixed output path: " << filesystemError.message() << ".\n"; + return kSetupError; + } + } + if (!options.inPlace && inputPath == outputPath) { + errors << "ERROR: fixed output resolves to the input file; use --in-place explicitly.\n"; + return kUsageError; + } + + if (options.inPlace) { + if (!RequireUnused(inputPath.string() + ".bak", "backup", errors)) { + return kSetupError; + } + } else if (!RequireUnused(outputPath, "fixed output", errors)) { + return kSetupError; + } + + auto workParent = options.inPlace ? inputPath.parent_path() : outputPath.parent_path(); + if (workParent.empty()) workParent = "."; + if (!std::filesystem::is_directory(workParent, filesystemError) || filesystemError) { + errors << "ERROR: output directory is unavailable: " << workParent.string() << "\n"; + return kSetupError; + } + + filesystemError.clear(); + runtime.restPath = std::filesystem::canonical(runtime.restPath, filesystemError); + if (filesystemError) { + errors << "ERROR: cannot resolve the REST installation prefix: " << filesystemError.message() + << ". Install REST before using recovery.\n"; + return kSetupError; + } + const auto stage1Wrapper = runtime.restPath / "macros/legacy/recoverLegacySignalData.C"; + const auto stage2Wrapper = runtime.restPath / "macros/legacy/REST_RebuildLegacySignalFile.C"; + if (!RequireMacro(stage1Wrapper, errors) || !RequireMacro(stage2Wrapper, errors)) return kSetupError; + + std::filesystem::path workDirectory; + std::string workError; + if (!runtime.createWorkDirectory(workParent, workDirectory, workError)) { + errors << "ERROR: " << workError << ".\n"; + return kSetupError; + } + const auto intermediate = workDirectory / "LegacySignalData.root"; + const auto candidate = workDirectory / "FixedCandidate.root"; + + output << "Legacy signal recovery stage 1/2: extracting with plain ROOT.\n" << std::flush; + const int stage1Status = runtime.runProcess( + BuildStage1Process(runtime, stage1Wrapper, workDirectory, inputPath, intermediate)); + if (stage1Status != 0) { + errors << "ERROR: legacy signal extraction failed with exit status " << stage1Status << ".\n"; + ReportRetainedWorkDirectory(workDirectory, errors); + return stage1Status; + } + if (!std::filesystem::is_regular_file(intermediate, filesystemError) || filesystemError) { + errors << "ERROR: stage 1 reported success but did not create the intermediate file.\n"; + ReportRetainedWorkDirectory(workDirectory, errors); + return kSetupError; + } + + output << "Legacy signal recovery stage 2/2: rebuilding with REST.\n" << std::flush; + const int stage2Status = runtime.runProcess( + BuildStage2Process(runtime, stage2Wrapper, inputPath, intermediate, candidate, options.inPlace)); + if (stage2Status != 0) { + errors << "ERROR: REST rebuild failed with exit status " << stage2Status << ".\n"; + ReportRetainedWorkDirectory(workDirectory, errors); + return stage2Status; + } + + if (options.inPlace) { + if (!std::filesystem::is_regular_file(inputPath, filesystemError) || filesystemError || + !std::filesystem::is_regular_file(inputPath.string() + ".bak", filesystemError) || + filesystemError) { + errors << "ERROR: in-place recovery reported success but the fixed file or backup is " + "unavailable.\n"; + ReportRetainedWorkDirectory(workDirectory, errors); + return kSetupError; + } + } else { + if (!std::filesystem::is_regular_file(candidate, filesystemError) || filesystemError) { + errors << "ERROR: stage 2 reported success but the validated candidate is unavailable.\n"; + ReportRetainedWorkDirectory(workDirectory, errors); + return kSetupError; + } + if (!RequireUnused(outputPath, "fixed output", errors)) { + ReportRetainedWorkDirectory(workDirectory, errors); + return kSetupError; + } + std::filesystem::create_hard_link(candidate, outputPath, filesystemError); + if (filesystemError) { + errors << "ERROR: cannot install the validated candidate without overwriting '" + << outputPath.string() << "': " << filesystemError.message() << ".\n"; + ReportRetainedWorkDirectory(workDirectory, errors); + return kSetupError; + } + filesystemError.clear(); + std::filesystem::remove(candidate, filesystemError); + if (filesystemError) { + errors << "WARNING: fixed output was installed, but its work-directory hard link could not be " + "removed: " + << candidate.string() << " (" << filesystemError.message() << ").\n"; + } + } + + filesystemError.clear(); + std::filesystem::remove_all(workDirectory, filesystemError); + if (filesystemError) { + errors << "WARNING: recovery succeeded, but the owned work directory could not be removed: " + << workDirectory.string() << " (" << filesystemError.message() << ").\n"; + } + output << "Legacy signal recovery completed successfully: " << outputPath.string() << "\n"; + return 0; +} + +} // namespace REST_LegacySignalRecoveryCLI diff --git a/source/bin/LegacySignalRecoveryCLI.h b/source/bin/LegacySignalRecoveryCLI.h new file mode 100644 index 000000000..fa087bb3b --- /dev/null +++ b/source/bin/LegacySignalRecoveryCLI.h @@ -0,0 +1,65 @@ +#ifndef REST_LEGACY_SIGNAL_RECOVERY_CLI_H +#define REST_LEGACY_SIGNAL_RECOVERY_CLI_H + +#include +#include +#include +#include +#include +#include + +namespace REST_LegacySignalRecoveryCLI { + +enum class ParseAction { kNotRequested, kRun, kHelp, kError }; + +struct Options { + std::filesystem::path input; + std::filesystem::path output; + bool outputWasSpecified = false; + bool inPlace = false; +}; + +struct ParseResult { + ParseAction action = ParseAction::kNotRequested; + Options options; + std::string error; +}; + +using Arguments = std::vector; +struct ProcessSpec { + Arguments arguments; + std::vector> environment; +}; + +using ProcessRunner = std::function; +using WorkDirectoryFactory = + std::function; + +struct Runtime { + std::filesystem::path restPath; + std::string rootExecutable = "root"; + std::string restRootExecutable = "restRoot"; + ProcessRunner runProcess; + WorkDirectoryFactory createWorkDirectory; +}; + +ParseResult ParseArguments(const Arguments& arguments); +void PrintHelp(std::ostream& output); + +ProcessSpec BuildStage1Process(const Runtime& runtime, const std::filesystem::path& wrapper, + const std::filesystem::path& workDirectory, const std::filesystem::path& input, + const std::filesystem::path& intermediate); +ProcessSpec BuildStage2Process(const Runtime& runtime, const std::filesystem::path& wrapper, + const std::filesystem::path& input, const std::filesystem::path& intermediate, + const std::filesystem::path& output, bool inPlace); + +int RunChildProcess(const ProcessSpec& process); +std::string ResolveExecutableFromSearchPath(const std::string& executable, const std::string& searchPath); +std::string ResolveExecutablePath(const std::string& executable); +bool CreateUniqueWorkDirectory(const std::filesystem::path& parent, std::filesystem::path& result, + std::string& error); +int Execute(const Options& options, Runtime runtime, std::ostream& output, std::ostream& errors); + +} // namespace REST_LegacySignalRecoveryCLI + +#endif diff --git a/source/bin/restRoot.cxx b/source/bin/restRoot.cxx index 758516f47..39362efe0 100644 --- a/source/bin/restRoot.cxx +++ b/source/bin/restRoot.cxx @@ -3,6 +3,12 @@ #include #include +#include +#include +#include +#include + +#include "LegacySignalRecoveryCLI.h" #include "TRestStringHelper.h" #include "TRestStringOutput.h" #include "TRestTools.h" @@ -22,6 +28,25 @@ using namespace std; // Don't use cout in the main function! // This will make cout un-usable in the command line! int main(int argc, char* argv[]) { + const REST_LegacySignalRecoveryCLI::Arguments arguments(argv, argv + argc); + const auto recovery = REST_LegacySignalRecoveryCLI::ParseArguments(arguments); + if (recovery.action == REST_LegacySignalRecoveryCLI::ParseAction::kHelp) { + REST_LegacySignalRecoveryCLI::PrintHelp(std::cout); + return 0; + } + if (recovery.action == REST_LegacySignalRecoveryCLI::ParseAction::kError) { + std::cerr << "ERROR: " << recovery.error << ".\n\n"; + REST_LegacySignalRecoveryCLI::PrintHelp(std::cerr); + return 2; + } + if (recovery.action == REST_LegacySignalRecoveryCLI::ParseAction::kRun) { + REST_LegacySignalRecoveryCLI::Runtime runtime; + runtime.rootExecutable = REST_ROOT_EXECUTABLE; + runtime.restRootExecutable = REST_LegacySignalRecoveryCLI::ResolveExecutablePath(argv[0]); + runtime.restPath = std::filesystem::path(runtime.restRootExecutable).parent_path().parent_path(); + return REST_LegacySignalRecoveryCLI::Execute(recovery.options, runtime, std::cout, std::cerr); + } + // set the env and debug status setenv("REST_VERSION", REST_RELEASE, 1); @@ -63,6 +88,14 @@ int main(int argc, char* argv[]) { printf("\n"); printf(" Option 0 will disable macro loading. Option 0 is the default.\n"); printf("\n"); + printf("-----\n"); + printf("\n"); + printf(" To recover a legacy detector signal branch safely in one command:\n"); + printf("\n"); + printf(" restRoot --recover-legacy-signals INPUT [--output OUTPUT | --in-place]\n"); + printf("\n"); + printf(" Use `restRoot --recover-legacy-signals --help` for details.\n"); + printf("\n"); exit(0); } } diff --git a/source/bin/test/LegacySignalRecoveryCLI.cxx b/source/bin/test/LegacySignalRecoveryCLI.cxx new file mode 100644 index 000000000..02f0da4fa --- /dev/null +++ b/source/bin/test/LegacySignalRecoveryCLI.cxx @@ -0,0 +1,413 @@ +#include "LegacySignalRecoveryCLI.h" + +#include + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#ifndef _WIN32 +#include +#include +#include +#endif + +namespace { + +namespace fs = std::filesystem; +using namespace REST_LegacySignalRecoveryCLI; + +class TemporaryDirectory { + public: + TemporaryDirectory() { + static std::atomic sequence{0}; + path = fs::temp_directory_path() / + ("rest_legacy_cli_test_" + + std::to_string(std::chrono::high_resolution_clock::now().time_since_epoch().count()) + "_" + + std::to_string(sequence++)); + fs::create_directories(path); + } + + ~TemporaryDirectory() { + std::error_code error; + fs::remove_all(path, error); + } + + fs::path path; +}; + +void WriteText(const fs::path& path, const std::string& text) { + std::ofstream output(path); + ASSERT_TRUE(output.is_open()); + output << text; +} + +std::string ReadText(const fs::path& path) { + std::ifstream input(path); + return {std::istreambuf_iterator(input), std::istreambuf_iterator()}; +} + +std::string EnvironmentValue(const ProcessSpec& process, const std::string& name) { + for (const auto& variable : process.environment) { + if (variable.first == name) return variable.second; + } + return ""; +} + +Runtime MakeRuntime(const fs::path& prefix, const fs::path& workDirectory, const ProcessRunner& runner) { + const auto macros = prefix / "macros/legacy"; + fs::create_directories(macros); + WriteText(macros / "recoverLegacySignalData.C", "// test wrapper"); + WriteText(macros / "REST_RebuildLegacySignalFile.C", "// test wrapper"); + + Runtime runtime; + runtime.restPath = prefix; + runtime.rootExecutable = "/matched/root"; + runtime.restRootExecutable = "/matched/restRoot"; + runtime.runProcess = runner; + runtime.createWorkDirectory = [workDirectory](const fs::path& parent, fs::path& result, std::string&) { + EXPECT_EQ(parent, workDirectory.parent_path()); + std::error_code error; + const bool created = fs::create_directory(workDirectory, error); + if (!created || error) return false; + fs::permissions(workDirectory, fs::perms::owner_all, fs::perm_options::replace, error); + if (error) return false; + result = workDirectory; + return true; + }; + return runtime; +} + +TEST(LegacySignalRecoveryCLI, ParsesOneShotInterfaceAndRejectsAmbiguity) { + auto parsed = ParseArguments({"restRoot", "--recover-legacy-signals", "input.root"}); + ASSERT_EQ(parsed.action, ParseAction::kRun); + EXPECT_EQ(parsed.options.input, "input.root"); + EXPECT_FALSE(parsed.options.inPlace); + EXPECT_FALSE(parsed.options.outputWasSpecified); + + parsed = ParseArguments({"restRoot", "--recover-legacy-signals", "input.root", "--output", "fixed.root"}); + ASSERT_EQ(parsed.action, ParseAction::kRun); + EXPECT_EQ(parsed.options.output, "fixed.root"); + + parsed = ParseArguments({"restRoot", "--recover-legacy-signals", "input.root", "--in-place"}); + ASSERT_EQ(parsed.action, ParseAction::kRun); + EXPECT_TRUE(parsed.options.inPlace); + + EXPECT_EQ(ParseArguments({"restRoot", "--recover-legacy-signals", "--help"}).action, ParseAction::kHelp); + EXPECT_EQ(ParseArguments({"restRoot", "-l", "--recover-legacy-signals", "input.root"}).action, + ParseAction::kError); + EXPECT_EQ(ParseArguments({"restRoot", "--recover-legacy-signals"}).action, ParseAction::kError); + EXPECT_EQ(ParseArguments({"restRoot", "--recover-legacy-signals", "input.root", "--output"}).action, + ParseAction::kError); + EXPECT_EQ(ParseArguments({"restRoot", "--recover-legacy-signals", "input.root", "--output", "--in-place"}) + .action, + ParseAction::kError); + EXPECT_EQ( + ParseArguments({"restRoot", "--recover-legacy-signals", "input.root", "--output", "--help"}).action, + ParseAction::kError); + EXPECT_EQ(ParseArguments({"restRoot", "--recover-legacy-signals", "input.root", "--output", "fixed.root", + "--in-place"}) + .action, + ParseAction::kError); + EXPECT_EQ(ParseArguments({"restRoot", "--recover-legacy-signals", "--", "-input.root"}).action, + ParseAction::kRun); + EXPECT_EQ(ParseArguments({"restRoot", "--help"}).action, ParseAction::kNotRequested); +} + +TEST(LegacySignalRecoveryCLI, BuildsIsolatedProcessesWithoutPuttingDataPathsInRootExpressions) { + Runtime runtime; + runtime.restPath = "/matching REST;prefix"; + runtime.rootExecutable = "/matching ROOT/bin/root"; + runtime.restRootExecutable = "/matching REST/bin/restRoot"; + const fs::path input = "/data/a path/quote\"; gSystem->Exec(\"bad\").root"; + const fs::path intermediate = "/work/legacy; \"signal\".root"; + const fs::path candidate = "/work/fixed candidate.root"; + const fs::path work = "/work/a directory"; + + const auto stage1 = + BuildStage1Process(runtime, "/matching REST;prefix/stage1.C", work, input, intermediate); + ASSERT_FALSE(stage1.arguments.empty()); + EXPECT_EQ(stage1.arguments.front(), runtime.rootExecutable); + EXPECT_NE(std::find(stage1.arguments.begin(), stage1.arguments.end(), "-n"), stage1.arguments.end()); + EXPECT_NE(std::find(stage1.arguments.begin(), stage1.arguments.end(), "-x"), stage1.arguments.end()); + for (const auto& argument : stage1.arguments) { + EXPECT_EQ(argument.find(input.string()), std::string::npos); + EXPECT_EQ(argument.find(intermediate.string()), std::string::npos); + } + EXPECT_EQ(EnvironmentValue(stage1, "REST_LEGACY_RECOVERY_INPUT"), input); + EXPECT_EQ(EnvironmentValue(stage1, "REST_LEGACY_RECOVERY_INTERMEDIATE"), intermediate); + EXPECT_EQ(EnvironmentValue(stage1, "ROOTSYS"), "/matching ROOT"); + EXPECT_EQ(EnvironmentValue(stage1, "PATH").rfind("/matching ROOT/bin", 0), 0U); + EXPECT_EQ(EnvironmentValue(stage1, "LD_LIBRARY_PATH").rfind("/matching ROOT/lib", 0), 0U); + + const auto stage2 = + BuildStage2Process(runtime, "/matching REST;prefix/stage2.C", input, intermediate, candidate, true); + EXPECT_EQ(stage2.arguments.front(), runtime.restRootExecutable); + EXPECT_NE(std::find(stage2.arguments.begin(), stage2.arguments.end(), "-n"), stage2.arguments.end()); + EXPECT_NE(std::find(stage2.arguments.begin(), stage2.arguments.end(), "-x"), stage2.arguments.end()); + for (const auto& argument : stage2.arguments) { + EXPECT_EQ(argument.find(input.string()), std::string::npos); + EXPECT_EQ(argument.find(intermediate.string()), std::string::npos); + EXPECT_EQ(argument.find(candidate.string()), std::string::npos); + } + EXPECT_EQ(EnvironmentValue(stage2, "REST_PATH"), runtime.restPath); + EXPECT_EQ(EnvironmentValue(stage2, "PATH").rfind((runtime.restPath / "bin").string(), 0), 0U); + EXPECT_EQ(EnvironmentValue(stage2, "LD_LIBRARY_PATH").rfind((runtime.restPath / "lib").string(), 0), 0U); + EXPECT_EQ(EnvironmentValue(stage2, "REST_LEGACY_RECOVERY_OUTPUT"), candidate); + EXPECT_EQ(EnvironmentValue(stage2, "REST_LEGACY_RECOVERY_IN_PLACE"), "1"); + EXPECT_EQ(EnvironmentValue(stage2, "REST_LEGACY_RECOVERY_REQUIRE_COMPLETE"), "1"); +} + +#ifndef _WIN32 +TEST(LegacySignalRecoveryCLI, CreatesPrivateWorkDirectory) { + TemporaryDirectory temporary; + fs::path work; + std::string error; + ASSERT_TRUE(CreateUniqueWorkDirectory(temporary.path, work, error)) << error; + struct stat status{}; + ASSERT_EQ(stat(work.c_str(), &status), 0); + EXPECT_EQ(status.st_mode & 0777, 0700); +} + +TEST(LegacySignalRecoveryCLI, ChildProcessPreservesSpecialEnvironmentAndReportsSignals) { + const std::string special = "a path with spaces; quote\" and $()"; + ProcessSpec process{{REST_LEGACY_RECOVERY_PROCESS_PROBE, "REST_TEST_SPECIAL", special}, + {{"REST_TEST_SPECIAL", special}}}; + EXPECT_EQ(RunChildProcess(process), 0); + + process = {{REST_LEGACY_RECOVERY_PROCESS_PROBE, "--terminate"}, {}}; + EXPECT_EQ(RunChildProcess(process), 128 + SIGTERM); + + process = {{"/definitely/missing/rest-process"}, {}}; + EXPECT_EQ(RunChildProcess(process), 127); +} + +TEST(LegacySignalRecoveryCLI, ForwardsParentSignalToChildGroupAndReapsGrandchild) { + TemporaryDirectory temporary; + const auto grandchildPidFile = temporary.path / "grandchild.pid"; + const auto reapedFile = temporary.path / "grandchild.reaped"; + ProcessSpec process{{REST_LEGACY_RECOVERY_PROCESS_PROBE, "--request-parent-signal-with-grandchild", + grandchildPidFile.string(), reapedFile.string()}, + {}}; + + EXPECT_EQ(RunChildProcess(process), 128 + SIGTERM); + EXPECT_EQ(ReadText(reapedFile), "reaped"); + + std::ifstream pidInput(grandchildPidFile); + pid_t grandchild = -1; + ASSERT_TRUE(pidInput >> grandchild); + errno = 0; + EXPECT_EQ(kill(grandchild, 0), -1); + EXPECT_EQ(errno, ESRCH); +} + +TEST(LegacySignalRecoveryCLI, PathFallbackRequiresExecutableFile) { + TemporaryDirectory temporary; + const auto command = temporary.path / "candidate-command"; + WriteText(command, "#!/bin/sh\nexit 0\n"); + fs::permissions(command, fs::perms::owner_read | fs::perms::owner_write, fs::perm_options::replace); + const auto commandName = command.filename().string(); + EXPECT_EQ(ResolveExecutableFromSearchPath(commandName, temporary.path.string()), commandName); + + fs::permissions(command, fs::perms::owner_all, fs::perm_options::replace); + EXPECT_EQ(ResolveExecutableFromSearchPath(commandName, temporary.path.string()), + fs::canonical(command).string()); +} +#endif + +TEST(LegacySignalRecoveryCLI, SuccessfulSiblingRecoveryInstallsCandidateAndCleansOwnedWork) { + TemporaryDirectory temporary; + const auto input = temporary.path / "legacy input; \"quoted\".root"; + const auto output = temporary.path / "fixed output; \"quoted\".root"; + const auto work = temporary.path / ".owned-work"; + const auto prefix = temporary.path / "matching prefix"; + WriteText(input, "legacy"); + + std::vector calls; + const auto runner = [&](const ProcessSpec& process) { + calls.push_back(process); + if (calls.size() == 1) { + WriteText(EnvironmentValue(process, "REST_LEGACY_RECOVERY_INTERMEDIATE"), "intermediate"); + } else { + WriteText(EnvironmentValue(process, "REST_LEGACY_RECOVERY_OUTPUT"), "fixed"); + } + return 0; + }; + auto runtime = MakeRuntime(prefix, work, runner); + + Options options; + options.input = input; + options.output = output; + options.outputWasSpecified = true; + std::ostringstream messages; + std::ostringstream errors; + EXPECT_EQ(Execute(options, runtime, messages, errors), 0) << errors.str(); + EXPECT_EQ(calls.size(), 2U); + EXPECT_EQ(ReadText(output), "fixed"); + EXPECT_EQ(ReadText(input), "legacy"); + EXPECT_FALSE(fs::exists(work)); + EXPECT_TRUE(errors.str().empty()); +} + +TEST(LegacySignalRecoveryCLI, DefaultOutputStaysBesideSuppliedInputSymlink) { + TemporaryDirectory temporary; + const auto targetDirectory = temporary.path / "target"; + const auto linkDirectory = temporary.path / "links"; + fs::create_directories(targetDirectory); + fs::create_directories(linkDirectory); + const auto target = targetDirectory / "canonical.root"; + const auto link = linkDirectory / "friendly.root"; + const auto expectedOutput = linkDirectory / "friendly_Fixed.root"; + const auto wrongOutput = targetDirectory / "canonical_Fixed.root"; + const auto work = linkDirectory / ".owned-work"; + const auto prefix = temporary.path / "matching-prefix"; + WriteText(target, "legacy"); + std::error_code symlinkError; + fs::create_symlink(target, link, symlinkError); + if (symlinkError) GTEST_SKIP() << symlinkError.message(); + + int calls = 0; + const auto runner = [&](const ProcessSpec& process) { + ++calls; + if (calls == 1) { + EXPECT_EQ(EnvironmentValue(process, "REST_LEGACY_RECOVERY_INPUT"), fs::canonical(target)); + WriteText(EnvironmentValue(process, "REST_LEGACY_RECOVERY_INTERMEDIATE"), "intermediate"); + } else { + WriteText(EnvironmentValue(process, "REST_LEGACY_RECOVERY_OUTPUT"), "fixed"); + } + return 0; + }; + auto runtime = MakeRuntime(prefix, work, runner); + + Options options; + options.input = link; + std::ostringstream messages; + std::ostringstream errors; + EXPECT_EQ(Execute(options, runtime, messages, errors), 0) << errors.str(); + EXPECT_EQ(calls, 2); + EXPECT_EQ(ReadText(expectedOutput), "fixed"); + EXPECT_FALSE(fs::exists(wrongOutput)); + EXPECT_EQ(ReadText(target), "legacy"); +} + +TEST(LegacySignalRecoveryCLI, RetainsOwnedWorkAndLeavesOutputAbsentAfterEitherStageFails) { + for (const int failingStage : {1, 2}) { + TemporaryDirectory temporary; + const auto input = temporary.path / "input.root"; + const auto output = temporary.path / "fixed.root"; + const auto work = temporary.path / ".owned-work"; + const auto prefix = temporary.path / "prefix"; + WriteText(input, "legacy"); + + int call = 0; + const auto runner = [&](const ProcessSpec& process) { + ++call; + if (call == 1) { + WriteText(EnvironmentValue(process, "REST_LEGACY_RECOVERY_INTERMEDIATE"), + failingStage == 1 ? "partial" : "intermediate"); + } + return call == failingStage ? 17 : 0; + }; + auto runtime = MakeRuntime(prefix, work, runner); + Options options{input, output, true, false}; + std::ostringstream messages; + std::ostringstream errors; + EXPECT_EQ(Execute(options, runtime, messages, errors), 17); + EXPECT_EQ(call, failingStage); + EXPECT_FALSE(fs::exists(output)); + EXPECT_TRUE(fs::exists(work)); + EXPECT_NE(errors.str().find(work.string()), std::string::npos); + } +} + +TEST(LegacySignalRecoveryCLI, RefusesExistingAndRacingOutputsWithoutOverwriting) { + for (const bool outputExistsInitially : {true, false}) { + TemporaryDirectory temporary; + const auto input = temporary.path / "input.root"; + const auto output = temporary.path / "fixed.root"; + const auto work = temporary.path / ".owned-work"; + const auto prefix = temporary.path / "prefix"; + WriteText(input, "legacy"); + if (outputExistsInitially) WriteText(output, "existing"); + + int calls = 0; + const auto runner = [&](const ProcessSpec& process) { + ++calls; + if (calls == 1) { + WriteText(EnvironmentValue(process, "REST_LEGACY_RECOVERY_INTERMEDIATE"), "intermediate"); + } else { + WriteText(EnvironmentValue(process, "REST_LEGACY_RECOVERY_OUTPUT"), "fixed"); + WriteText(output, "racing writer"); + } + return 0; + }; + auto runtime = MakeRuntime(prefix, work, runner); + Options options{input, output, true, false}; + std::ostringstream messages; + std::ostringstream errors; + EXPECT_NE(Execute(options, runtime, messages, errors), 0); + EXPECT_EQ(ReadText(output), outputExistsInitially ? "existing" : "racing writer"); + EXPECT_EQ(calls, outputExistsInitially ? 0 : 2); + if (!outputExistsInitially) EXPECT_TRUE(fs::exists(work)); + } +} + +TEST(LegacySignalRecoveryCLI, RefusesDanglingOutputAndBackupSymlinks) { + TemporaryDirectory temporary; + const auto input = temporary.path / "input.root"; + WriteText(input, "legacy"); + std::error_code error; + + const auto output = temporary.path / "fixed.root"; + fs::create_symlink(temporary.path / "missing", output, error); + if (error) GTEST_SKIP() << error.message(); + Runtime runtime; + int calls = 0; + runtime.runProcess = [&](const ProcessSpec&) { + ++calls; + return 0; + }; + Options options{input, output, true, false}; + std::ostringstream messages; + std::ostringstream errors; + EXPECT_NE(Execute(options, runtime, messages, errors), 0); + EXPECT_EQ(calls, 0); + + fs::remove(output); + fs::create_symlink(temporary.path / "missing-backup", input.string() + ".bak", error); + options = {}; + options.input = input; + options.inPlace = true; + errors.str(""); + EXPECT_NE(Execute(options, runtime, messages, errors), 0); + EXPECT_EQ(calls, 0); +} + +TEST(LegacySignalRecoveryCLI, RejectsRemoteInputAndOutputProtocols) { + Runtime runtime; + std::ostringstream messages; + std::ostringstream errors; + + Options options; + options.input = "root://server/data.root"; + EXPECT_NE(Execute(options, runtime, messages, errors), 0); + EXPECT_NE(errors.str().find("local filesystem"), std::string::npos); + + TemporaryDirectory temporary; + const auto input = temporary.path / "input.root"; + WriteText(input, "legacy"); + options = {input, "https://server/fixed.root", true, false}; + errors.str(""); + EXPECT_NE(Execute(options, runtime, messages, errors), 0); + EXPECT_NE(errors.str().find("local filesystem"), std::string::npos); +} + +} // namespace diff --git a/source/bin/test/LegacySignalRecoveryProcessProbe.cxx b/source/bin/test/LegacySignalRecoveryProcessProbe.cxx new file mode 100644 index 000000000..36aa30d38 --- /dev/null +++ b/source/bin/test/LegacySignalRecoveryProcessProbe.cxx @@ -0,0 +1,60 @@ +#include +#include +#include +#include +#include + +#ifndef _WIN32 +#include +#include +#endif + +#ifndef _WIN32 +volatile std::sig_atomic_t gForwardedSignal = 0; + +void RecordSignal(int signal) { gForwardedSignal = signal; } +#endif + +int main(int argc, char* argv[]) { + if (argc == 2 && std::string(argv[1]) == "--terminate") { + std::raise(SIGTERM); + return 99; + } +#ifndef _WIN32 + if (argc == 4 && std::string(argv[1]) == "--request-parent-signal-with-grandchild") { + const pid_t grandchild = fork(); + if (grandchild < 0) return 4; + if (grandchild == 0) { + while (true) pause(); + } + + struct sigaction action{}; + action.sa_handler = RecordSignal; + sigemptyset(&action.sa_mask); + if (sigaction(SIGTERM, &action, nullptr) != 0) return 5; + + { + std::ofstream pidFile(argv[2]); + pidFile << grandchild; + if (!pidFile) return 6; + } + if (kill(getppid(), SIGTERM) != 0) return 7; + while (gForwardedSignal == 0) pause(); + + int status = 0; + pid_t waited; + do { + waited = waitpid(grandchild, &status, 0); + } while (waited < 0 && errno == EINTR); + if (waited != grandchild || !WIFSIGNALED(status) || WTERMSIG(status) != SIGTERM) return 8; + + std::ofstream reapedFile(argv[3]); + reapedFile << "reaped"; + if (!reapedFile) return 9; + return 128 + gForwardedSignal; + } +#endif + if (argc != 3) return 2; + const char* value = std::getenv(argv[1]); + return value != nullptr && value == std::string(argv[2]) ? 0 : 3; +} diff --git a/source/framework/core/src/TRestRun.cxx b/source/framework/core/src/TRestRun.cxx index e6e07a3a7..fc91b207b 100644 --- a/source/framework/core/src/TRestRun.cxx +++ b/source/framework/core/src/TRestRun.cxx @@ -131,13 +131,9 @@ void WarnUnsupportedLegacyDetectorSignalBranch() { "still be read, but this detector signal event branch is disabled to avoid excessive " "memory usage or a crash." << RESTendl; - RESTWarning << "The data is recoverable: convert the file once with the macros in " - "$REST_PATH/macros/legacy :" - << RESTendl; - RESTWarning << " 1) root -l -b -q 'recoverLegacySignalData.C+(\"yourFile.root\")' (plain root, NOT " - "restRoot)" - << RESTendl; - RESTWarning << " 2) restRoot -b -q 'REST_RebuildLegacySignalFile.C(\"yourFile.root\")'" << RESTendl; + RESTWarning << "The data is recoverable with a matching installed REST:" << RESTendl; + RESTWarning << " restRoot --recover-legacy-signals yourFile.root" << RESTendl; + RESTWarning << "Use --in-place only when you explicitly want replacement plus a .bak copy." << RESTendl; } } // namespace diff --git a/source/framework/test/integration/LegacyDetectorSignal.cxx b/source/framework/test/integration/LegacyDetectorSignal.cxx index 990bce3fe..3c80e4ce1 100644 --- a/source/framework/test/integration/LegacyDetectorSignal.cxx +++ b/source/framework/test/integration/LegacyDetectorSignal.cxx @@ -6,6 +6,7 @@ #include #include +#include #include #include #include @@ -15,6 +16,7 @@ #include #include #include +#include #include "../../../../macros/legacy/LegacyRecoveryCandidateValidation.h" #include "../../../../macros/legacy/LegacyRecoveryFileUtils.h" @@ -201,4 +203,96 @@ TEST(LegacyRecoveryCandidateValidation, InvalidCandidateLeavesOriginalUntouched) EXPECT_NE(errors.str().find("were not touched"), std::string::npos); } +TEST(LegacyRecoveryCandidateValidation, CopiesAndValidatesEveryAdditionalTopLevelTree) { + TemporaryDirectory temporary; + const auto sourcePath = temporary.path / "source.root"; + const auto candidatePath = temporary.path / "candidate.root"; + { + TFile source(sourcePath.string().c_str(), "CREATE"); + ASSERT_FALSE(source.IsZombie()); + TTree customTree("CustomTree", "custom data"); + int value = 1; + customTree.Branch("value", &value); + customTree.Fill(); + ASSERT_GT(customTree.Write(), 0); + value = 2; + customTree.Fill(); + ASSERT_GT(customTree.Write(), 0); // newest cycle has two entries + + TTree secondTree("SecondTree", "second custom tree"); + double measurement = 3.5; + secondTree.Branch("measurement", &measurement); + secondTree.Fill(); + ASSERT_GT(secondTree.Write(), 0); + source.Close(); + } + + std::vector expectations; + std::ostringstream errors; + { + TFile source(sourcePath.string().c_str(), "READ"); + TFile candidate(candidatePath.string().c_str(), "CREATE"); + ASSERT_TRUE(REST_LegacyRecovery::CopyAdditionalTopLevelTrees(source, candidate, expectations, errors)) + << errors.str(); + ASSERT_TRUE(REST_LegacyRecovery::CheckAndCloseOutputFile(candidate, errors)) << errors.str(); + source.Close(); + } + + ASSERT_EQ(expectations.size(), 2U); + const auto custom = std::find_if(expectations.begin(), expectations.end(), [](const auto& expectation) { + return expectation.keyName == "CustomTree"; + }); + ASSERT_NE(custom, expectations.end()); + EXPECT_EQ(custom->entries, 2); + EXPECT_EQ(custom->branches, std::vector({"value"})); + + { + TFile candidate(candidatePath.string().c_str(), "READ"); + ASSERT_TRUE(REST_LegacyRecovery::ValidateAdditionalTopLevelTrees(candidate, expectations, errors)) + << errors.str(); + } + + auto wrongInventory = expectations; + wrongInventory.front().branches.push_back("missing"); + { + TFile candidate(candidatePath.string().c_str(), "READ"); + errors.str(""); + EXPECT_FALSE(REST_LegacyRecovery::ValidateAdditionalTopLevelTrees(candidate, wrongInventory, errors)); + EXPECT_NE(errors.str().find("branch inventory"), std::string::npos); + } + + { + TFile candidate(candidatePath.string().c_str(), "UPDATE"); + TTree unexpected("UnexpectedTree", "unexpected"); + int extra = 1; + unexpected.Branch("extra", &extra); + unexpected.Fill(); + ASSERT_GT(unexpected.Write(), 0); + candidate.Close(); + } + { + TFile candidate(candidatePath.string().c_str(), "READ"); + errors.str(""); + EXPECT_FALSE(REST_LegacyRecovery::ValidateAdditionalTopLevelTrees(candidate, expectations, errors)); + EXPECT_NE(errors.str().find("inventory differs"), std::string::npos); + } + + const auto originalPath = temporary.path / "in-place.root"; + const auto backupPath = temporary.path / "in-place.root.bak"; + WriteText(originalPath, "original"); + const auto validateAdditionalTrees = [&expectations](const fs::path& path, + std::ostream& validationErrors) { + TFile candidate(path.string().c_str(), "READ"); + return REST_LegacyRecovery::ValidateAdditionalTopLevelTrees(candidate, expectations, + validationErrors); + }; + errors.str(""); + EXPECT_FALSE(REST_LegacyRecovery::ValidateAndReplaceFileWithBackup( + candidatePath, originalPath, backupPath, errors, validateAdditionalTrees)); + EXPECT_EQ(ReadText(originalPath), "original"); + EXPECT_TRUE(fs::is_regular_file(candidatePath)); + EXPECT_FALSE(fs::exists(backupPath)); + EXPECT_NE(errors.str().find("were not touched"), std::string::npos); +} + } // namespace diff --git a/source/framework/test/src/LegacyRecoveryFileUtils.cxx b/source/framework/test/src/LegacyRecoveryFileUtils.cxx index 132fb0115..29c7045dc 100644 --- a/source/framework/test/src/LegacyRecoveryFileUtils.cxx +++ b/source/framework/test/src/LegacyRecoveryFileUtils.cxx @@ -18,9 +18,11 @@ namespace { namespace fs = std::filesystem; +using REST_LegacyRecovery::BuildSiblingRootPath; using REST_LegacyRecovery::ComparePaths; using REST_LegacyRecovery::ParseInteger; using REST_LegacyRecovery::RecoveryProvenance; +using REST_LegacyRecovery::RenamePath; using REST_LegacyRecovery::ReplaceFileWithBackup; using REST_LegacyRecovery::ResolvePathIdentity; using REST_LegacyRecovery::SourceIdentity; @@ -101,6 +103,13 @@ TEST(LegacyRecoveryFileUtils, RejectsEquivalentAndExistingOutputPaths) { EXPECT_FALSE(fs::exists(newOutput)); } +TEST(LegacyRecoveryFileUtils, BuildsSiblingNamesFromTheFilenameExtensionOnly) { + EXPECT_EQ(BuildSiblingRootPath("/data/file.root", "_Fixed"), "/data/file_Fixed.root"); + EXPECT_EQ(BuildSiblingRootPath("/data.root/file", "_Fixed"), "/data.root/file_Fixed.root"); + EXPECT_EQ(BuildSiblingRootPath("/data/file.root.backup", "_Fixed"), "/data/file.root.backup_Fixed.root"); + EXPECT_EQ(BuildSiblingRootPath("relative.root", "_LegacySignalData"), "relative_LegacySignalData.root"); +} + TEST(LegacyRecoveryFileUtils, ReplacesOriginalAndKeepsBackup) { TemporaryDirectory temporary; const auto original = temporary.path / "input.root"; @@ -134,6 +143,21 @@ TEST(LegacyRecoveryFileUtils, RefusesExistingBackupWithoutChangingFiles) { EXPECT_NE(errors.str().find("Refusing to overwrite"), std::string::npos); } +TEST(LegacyRecoveryFileUtils, RenameDoesNotOverwriteDestinationCreatedAfterPreflight) { + TemporaryDirectory temporary; + const auto source = temporary.path / "source.root"; + const auto destination = temporary.path / "destination.root"; + WriteText(source, "source"); + WriteText(destination, "racer"); + + std::error_code error; + RenamePath(source, destination, error); + + EXPECT_TRUE(error); + EXPECT_EQ(ReadText(source), "source"); + EXPECT_EQ(ReadText(destination), "racer"); +} + TEST(LegacyRecoveryFileUtils, PreservesFilesWhenBackupMoveFails) { TemporaryDirectory temporary; const auto original = temporary.path / "input.root"; From 7241fc0bd2d00bc507e0af766d84586d18b2e87a Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Wed, 19 Aug 2026 18:25:25 +0000 Subject: [PATCH 10/10] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- source/CMakeLists.txt | 20 ++++++------- source/bin/CMakeLists.txt | 28 ++++++++----------- source/bin/LegacySignalRecoveryCLI.cxx | 10 +++---- source/bin/test/LegacySignalRecoveryCLI.cxx | 2 +- .../test/LegacySignalRecoveryProcessProbe.cxx | 2 +- 5 files changed, 27 insertions(+), 35 deletions(-) diff --git a/source/CMakeLists.txt b/source/CMakeLists.txt index 1795a4d65..7f3c231ff 100644 --- a/source/CMakeLists.txt +++ b/source/CMakeLists.txt @@ -114,11 +114,11 @@ if (NOT TARGET RestTrack) endif () if (BUILD_LEGACY_SIGNAL_TEST) - set( - REST_LEGACY_SIGNAL_TEST_FILE + set(REST_LEGACY_SIGNAL_TEST_FILE "" - CACHE FILEPATH - "Optional canonical legacy DetectorSignal ROOT file for integration tests" + CACHE + FILEPATH + "Optional canonical legacy DetectorSignal ROOT file for integration tests" ) set(LEGACY_SIGNAL_TEST_SOURCE framework/test/integration/LegacyDetectorSignal.cxx) @@ -129,15 +129,13 @@ if (BUILD_LEGACY_SIGNAL_TEST) framework/core/inc libraries/detector/inc libraries/raw/inc) target_compile_definitions( testLegacyDetectorSignal - PRIVATE - REST_LEGACY_SIGNAL_TEST_FILE="${REST_LEGACY_SIGNAL_TEST_FILE}") + PRIVATE REST_LEGACY_SIGNAL_TEST_FILE="${REST_LEGACY_SIGNAL_TEST_FILE}") target_link_libraries( - testLegacyDetectorSignal - PRIVATE RestDetector RestRaw RestTrack RestFramework gtest_main - stdc++fs) + testLegacyDetectorSignal PRIVATE RestDetector RestRaw RestTrack + RestFramework gtest_main stdc++fs) include(GoogleTest) - gtest_add_tests( - TARGET testLegacyDetectorSignal SOURCES ${LEGACY_SIGNAL_TEST_SOURCE}) + gtest_add_tests(TARGET testLegacyDetectorSignal + SOURCES ${LEGACY_SIGNAL_TEST_SOURCE}) endif () unset(BUILD_LEGACY_SIGNAL_TEST) diff --git a/source/bin/CMakeLists.txt b/source/bin/CMakeLists.txt index 282629a99..e8a5d31ec 100644 --- a/source/bin/CMakeLists.txt +++ b/source/bin/CMakeLists.txt @@ -24,31 +24,25 @@ foreach (file ${files}) endforeach (file) if (TEST) - set(LEGACY_SIGNAL_RECOVERY_CLI_TEST_SOURCE - test/LegacySignalRecoveryCLI.cxx) - add_executable( - testRestRootLegacySignalRecoveryProcessProbe - test/LegacySignalRecoveryProcessProbe.cxx) + set(LEGACY_SIGNAL_RECOVERY_CLI_TEST_SOURCE test/LegacySignalRecoveryCLI.cxx) + add_executable(testRestRootLegacySignalRecoveryProcessProbe + test/LegacySignalRecoveryProcessProbe.cxx) add_executable( testRestRootLegacySignalRecovery - ${LEGACY_SIGNAL_RECOVERY_CLI_TEST_SOURCE} - LegacySignalRecoveryCLI.cxx) - target_include_directories( - testRestRootLegacySignalRecovery PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}) - target_link_libraries( - testRestRootLegacySignalRecovery PRIVATE gtest_main) + ${LEGACY_SIGNAL_RECOVERY_CLI_TEST_SOURCE} LegacySignalRecoveryCLI.cxx) + target_include_directories(testRestRootLegacySignalRecovery + PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}) + target_link_libraries(testRestRootLegacySignalRecovery PRIVATE gtest_main) target_compile_definitions( testRestRootLegacySignalRecovery PRIVATE REST_LEGACY_RECOVERY_PROCESS_PROBE="$" ) - add_dependencies( - testRestRootLegacySignalRecovery - testRestRootLegacySignalRecoveryProcessProbe) + add_dependencies(testRestRootLegacySignalRecovery + testRestRootLegacySignalRecoveryProcessProbe) include(GoogleTest) - gtest_add_tests( - TARGET testRestRootLegacySignalRecovery - SOURCES ${LEGACY_SIGNAL_RECOVERY_CLI_TEST_SOURCE}) + gtest_add_tests(TARGET testRestRootLegacySignalRecovery + SOURCES ${LEGACY_SIGNAL_RECOVERY_CLI_TEST_SOURCE}) endif () set(rest_exes diff --git a/source/bin/LegacySignalRecoveryCLI.cxx b/source/bin/LegacySignalRecoveryCLI.cxx index 0fe58553a..71e98bbcd 100644 --- a/source/bin/LegacySignalRecoveryCLI.cxx +++ b/source/bin/LegacySignalRecoveryCLI.cxx @@ -93,7 +93,7 @@ class SignalForwardingGuard { public: explicit SignalForwardingGuard(pid_t child) { gActiveChildProcessGroup = child; - struct sigaction action{}; + struct sigaction action {}; action.sa_handler = ForwardSignalToChild; sigemptyset(&action.sa_mask); action.sa_flags = 0; @@ -110,9 +110,9 @@ class SignalForwardingGuard { } private: - struct sigaction fPreviousInterrupt{}; - struct sigaction fPreviousTerminate{}; - struct sigaction fPreviousHangup{}; + struct sigaction fPreviousInterrupt {}; + struct sigaction fPreviousTerminate {}; + struct sigaction fPreviousHangup {}; }; #endif @@ -392,7 +392,7 @@ bool CreateUniqueWorkDirectory(const std::filesystem::path& parent, std::filesys return false; } #ifndef _WIN32 - struct stat directoryStatus{}; + struct stat directoryStatus {}; if (stat(candidate.c_str(), &directoryStatus) != 0 || (directoryStatus.st_mode & 0777) != S_IRWXU) { std::filesystem::remove(candidate); diff --git a/source/bin/test/LegacySignalRecoveryCLI.cxx b/source/bin/test/LegacySignalRecoveryCLI.cxx index 02f0da4fa..e2de0ac6d 100644 --- a/source/bin/test/LegacySignalRecoveryCLI.cxx +++ b/source/bin/test/LegacySignalRecoveryCLI.cxx @@ -172,7 +172,7 @@ TEST(LegacySignalRecoveryCLI, CreatesPrivateWorkDirectory) { fs::path work; std::string error; ASSERT_TRUE(CreateUniqueWorkDirectory(temporary.path, work, error)) << error; - struct stat status{}; + struct stat status {}; ASSERT_EQ(stat(work.c_str(), &status), 0); EXPECT_EQ(status.st_mode & 0777, 0700); } diff --git a/source/bin/test/LegacySignalRecoveryProcessProbe.cxx b/source/bin/test/LegacySignalRecoveryProcessProbe.cxx index 36aa30d38..897153eea 100644 --- a/source/bin/test/LegacySignalRecoveryProcessProbe.cxx +++ b/source/bin/test/LegacySignalRecoveryProcessProbe.cxx @@ -28,7 +28,7 @@ int main(int argc, char* argv[]) { while (true) pause(); } - struct sigaction action{}; + struct sigaction action {}; action.sa_handler = RecordSignal; sigemptyset(&action.sa_mask); if (sigaction(SIGTERM, &action, nullptr) != 0) return 5;