Fix crash in ROOTLegacyReader when reading actual legacy files

cmake/podioTest.cmake

@@ -28,14 +28,14 @@ function(CREATE_PODIO_TEST sourcefile additional_libs)
 endfunction()
 
 #--- utility macro to facilitate the downloading of legacy input data
-macro(PODIO_DOWNLOAD_LEGACY_INPUTS)
+macro(PODIO_DOWNLOAD_LEGACY_INPUTS legacy_versions)
   # Avoid fetching these everytime cmake is run by caching the directory the first
   # time the inputs are fetched or if the expected file does not exist in the
   # expected directory
   if (NOT DEFINED CACHE{PODIO_TEST_INPUT_DATA_DIR} OR NOT EXISTS ${PODIO_TEST_INPUT_DATA_DIR}/v00-16-05/example_frame.root)
     message(STATUS "Getting test input files")
     execute_process(
-      COMMAND bash ${CMAKE_SOURCE_DIR}/tests/scripts/get_test_inputs.sh
+      COMMAND bash ${CMAKE_SOURCE_DIR}/tests/scripts/get_test_inputs.sh ${legacy_versions}
       OUTPUT_VARIABLE podio_test_input_data_dir
       RESULT_VARIABLE test_inputs_available
     )

src/ROOTLegacyReader.cc

@@ -123,19 +123,36 @@ void ROOTLegacyReader::openFiles(const std::vector<std::string>& filenames) {
   auto metadatatree = static_cast<TTree*>(m_chain->GetFile()->Get("metadata"));
   m_table = std::make_shared<CollectionIDTable>();
   auto* table = m_table.get();
-  metadatatree->SetBranchAddress("CollectionIDs", &table);
+  auto* tableBranch = root_utils::getBranch(metadatatree, "CollectionIDs");
+  tableBranch->SetAddress(&table);
+  tableBranch->GetEntry(0);
 
   podio::version::Version* versionPtr{nullptr};
   if (auto* versionBranch = root_utils::getBranch(metadatatree, "PodioVersion")) {
     versionBranch->SetAddress(&versionPtr);
+    versionBranch->GetEntry(0);
   }
+  m_fileVersion = versionPtr ? *versionPtr : podio::version::Version{0, 0, 0};
+  delete versionPtr;
 
   // Check if the CollectionTypeInfo branch is there and assume that the file
   // has been written with with podio pre #197 (<0.13.1) if that is not the case
   if (auto* collInfoBranch = root_utils::getBranch(metadatatree, "CollectionTypeInfo")) {
     auto collectionInfo = new std::vector<root_utils::CollectionInfoT>;
-    collInfoBranch->SetAddress(&collectionInfo);
-    metadatatree->GetEntry(0);
+
+    if (m_fileVersion < podio::version::Version{0, 16, 4}) {
+      auto oldCollInfo = new std::vector<root_utils::CollectionInfoWithoutSchemaT>();
+      collInfoBranch->SetAddress(&oldCollInfo);
+      collInfoBranch->GetEntry(0);
+      collectionInfo->reserve(oldCollInfo->size());
+      for (auto&& [collID, collType, isSubsetColl] : *oldCollInfo) {
+        collectionInfo->emplace_back(collID, std::move(collType), isSubsetColl, 1u);
+      }
+      delete oldCollInfo;
+    } else {
+      collInfoBranch->SetAddress(&collectionInfo);
+      collInfoBranch->GetEntry(0);
+    }
     createCollectionBranches(*collectionInfo);
     delete collectionInfo;
   } else {
@@ -145,9 +162,6 @@ void ROOTLegacyReader::openFiles(const std::vector<std::string>& filenames) {
     const auto collectionInfo = root_utils::reconstructCollectionInfo(m_chain.get(), *m_table);
     createCollectionBranches(collectionInfo);
   }
-
-  m_fileVersion = versionPtr ? *versionPtr : podio::version::Version{0, 0, 0};
-  delete versionPtr;
 }
 
 unsigned ROOTLegacyReader::getEntries(const std::string& name) const {

tests/CMakeLists.txt

@@ -41,8 +41,14 @@ PODIO_ADD_ROOT_IO_DICT(ExtensionDataModelDict ExtensionDataModel "${ext_headers}
 
 PODIO_ADD_SIO_IO_BLOCKS(ExtensionDataModel "${ext_headers}" "${ext_sources}")
 
+set(legacy_test_versions
+  v00-16
+  v00-16-02
+  v00-16-05
+)
+
 ### Define the actual tests
-PODIO_DOWNLOAD_LEGACY_INPUTS()
+PODIO_DOWNLOAD_LEGACY_INPUTS("${legacy_test_versions}")
 
 add_executable(check_benchmark_outputs check_benchmark_outputs.cpp)
 target_link_libraries(check_benchmark_outputs PRIVATE ROOT::Tree)

tests/CTestCustom.cmake

@@ -60,9 +60,10 @@ if ((NOT "@FORCE_RUN_ALL_TESTS@" STREQUAL "ON") AND (NOT "@USE_SANITIZER@" STREQ
     datamodel_def_store_roundtrip_sio_extension
   )
 
-  foreach(version in @legacy_versions@)
+  foreach(version in @legacy_test_versions@)
       list(APPEND CTEST_CUSTOM_TESTS_IGNORE read-legacy-files-root_${version})
       list(APPEND CTEST_CUSTOM_TESTS_IGNORE read_frame_root_${version})
+      list(APPEND CTEST_CUSTOM_TESTS_IGNORE read_frame_legacy_root_${version})
   endforeach()
 
   # ostream_operator is working with Memory sanitizer (at least locally)

tests/root_io/CMakeLists.txt

@@ -53,9 +53,9 @@ if (DEFINED CACHE{PODIO_TEST_INPUT_DATA_DIR})
 
   ADD_PODIO_LEGACY_TEST(v00-13 read-legacy-files-root example.root legacy_test_cases)
 
-  set(legacy_versions v00-16 v00-16-05 PARENT_SCOPE)
-  foreach(version IN LISTS legacy_versions)
+  foreach(version IN LISTS legacy_test_versions)
     ADD_PODIO_LEGACY_TEST(${version} read-legacy-files-root example.root legacy_test_cases)
     ADD_PODIO_LEGACY_TEST(${version} read_frame_root example_frame.root legacy_test_cases)
+    ADD_PODIO_LEGACY_TEST(${version} read_frame_legacy_root example.root legacy_test_cases)
   endforeach()
 endif()

tests/root_io/read_frame_legacy_root.cpp

@@ -5,16 +5,23 @@
 
 #include <iostream>
 
-int main() {
+int main(int argc, char* argv[]) {
+  std::string inputFile = "example.root";
+  bool assertBuildVersion = true;
+  if (argc == 2) {
+    inputFile = argv[1];
+    assertBuildVersion = false;
+  }
+
   auto reader = podio::ROOTLegacyReader();
   try {
-    reader.openFile("example.root");
+    reader.openFile(inputFile);
   } catch (const std::runtime_error& e) {
-    std::cout << "File could not be opened, aborting." << std::endl;
+    std::cout << "File (" << inputFile << ")could not be opened, aborting." << std::endl;
     return 1;
   }
 
-  if (reader.currentFileVersion() != podio::version::build_version) {
+  if (assertBuildVersion && reader.currentFileVersion() != podio::version::build_version) {
     std::cerr << "The podio build version could not be read back correctly. "
               << "(expected:" << podio::version::build_version << ", actual: " << reader.currentFileVersion() << ")"
               << std::endl;

tests/scripts/get_test_inputs.sh

@@ -11,7 +11,7 @@ cd ${PODIO_TEST_INPUT_DATA_DIR}
 mkdir v00-13 && cd v00-13
 wget https://key4hep.web.cern.ch:443/testFiles/podio/v00-13/example.root > /dev/null 2>&1
 
-for version in v00-16 v00-16-05; do
+for version in $@; do
     cd ${PODIO_TEST_INPUT_DATA_DIR}
     mkdir ${version} && cd ${version}
     for fileName in example.root example_frame.root; do