Merge pull request #28561 from makortel/enforceGUIDInFileName

Add enforceGUIDInFileName option to PoolSource and EmbeddedRootSource
cms-sw · Dec 7, 2019 · b7e3009 · b7e3009
2 parents a582b56 + d7ff852
commit b7e3009
Show file tree

Hide file tree

Showing 20 changed files with 207 additions and 23 deletions.
diff --git a/FWCore/Framework/python/test/cmsExceptionsFatalOption_cff.py b/FWCore/Framework/python/test/cmsExceptionsFatalOption_cff.py
@@ -30,5 +30,6 @@
   'ProductDoesNotSupportViews',
   'ProductDoesNotSupportPtr',
   'NotFound',
-  'FormatIncompatibility'
+  'FormatIncompatibility',
+  'FileNameInconsistentWithGUID',
 )
diff --git a/FWCore/MessageLogger/scripts/edmFjrDump b/FWCore/MessageLogger/scripts/edmFjrDump
@@ -0,0 +1,36 @@
+#!/usr/bin/env python
+
+from __future__ import print_function
+import xml.etree.ElementTree as ET
+import argparse
+
+def printGUID(root):
+    for f in root.findall("File"):
+        print(f.find("GUID").text)
+
+def printExitCode(root):
+    error = root.find("FrameworkError")
+    if error is None:
+        print("0")
+        return
+    print(error.get("ExitStatus"))
+
+def main(opts):
+    tree = ET.parse(opts.file)
+    root = tree.getroot()
+    if opts.guid:
+        printGUID(root)
+    if opts.exitCode:
+        printExitCode(root)
+
+if __name__ == "__main__":
+    parser = argparse.ArgumentParser(description="Extract some values from the Framework Job Report")
+    parser.add_argument("file", type=str,
+                        help="Framework Job Report XML file")
+    parser.add_argument("--guid", action="store_true",
+                        help="GUID of the output file")
+    parser.add_argument("--exitCode", action="store_true",
+                        help="Job exit code")
+
+    opts = parser.parse_args()
+    main(opts)
diff --git a/FWCore/Utilities/interface/EDMException.h b/FWCore/Utilities/interface/EDMException.h
@@ -16,8 +16,7 @@ namespace edm {
   namespace errors {
 
     // If you add a new entry to the set of values, make sure to
-    // update the translation map in EDMException.cc, the actions
-    // table in FWCore/Framework/src/Actions.cc, and the configuration
+    // update the translation map in EDMException.cc, and the configuration
     // fragment FWCore/Framework/python/test/cmsExceptionsFatalOption_cff.py.
 
     enum ErrorCodes {
@@ -66,6 +65,8 @@ namespace edm {
 
       FileWriteError = 8033,
 
+      FileNameInconsistentWithGUID = 8034,
+
       EventGenerationFailure = 8501,
 
       CaughtSignal = 9000

diff --git a/FWCore/Utilities/interface/stemFromPath.h b/FWCore/Utilities/interface/stemFromPath.h
@@ -0,0 +1,19 @@
+#ifndef FWCore_Utilities_stemFromPath_h
+#define FWCore_Utilities_stemFromPath_h
+
+#include <string_view>
+
+namespace edm {
+  // This functions extracts the stem of a file from the path (= file
+  // name without the extension). The returned value is a string_view
+  // to the input string. Caller should ensure that the input string
+  // object lives long enough.
+  //
+  // The reason to have our own function instead of
+  // std/boost::filesystem is that tehcnically these paths are not
+  // filesystem paths, but paths in CMS LFN/PFN space that (may) have
+  // different rules.
+  std::string_view stemFromPath(std::string_view path);
+}  // namespace edm
+
+#endif
diff --git a/FWCore/Utilities/src/EDMException.cc b/FWCore/Utilities/src/EDMException.cc
@@ -42,6 +42,7 @@ namespace edm {
                                                               FILLENTRY(ExceededResourceRSS),
                                                               FILLENTRY(ExceededResourceTime),
                                                               FILLENTRY(FileWriteError),
+                                                              FILLENTRY(FileNameInconsistentWithGUID),
                                                               FILLENTRY(EventGenerationFailure),
                                                               FILLENTRY(CaughtSignal)};
     static const std::string kUnknownCode("unknownCode");

diff --git a/FWCore/Utilities/src/stemFromPath.cc b/FWCore/Utilities/src/stemFromPath.cc
@@ -0,0 +1,20 @@
+#include "FWCore/Utilities/interface/stemFromPath.h"
+
+namespace edm {
+  std::string_view stemFromPath(std::string_view path) {
+    auto begin = path.rfind("/");
+    if (begin == std::string_view::npos) {
+      begin = path.rfind(":");
+      if (begin == std::string_view::npos) {
+        // shouldn't really happen?
+        begin = 0;
+      } else {
+        begin += 1;
+      }
+    } else {
+      begin += 1;
+    }
+    auto end = path.find(".", begin);
+    return path.substr(begin, end - begin);
+  }
+}  // namespace edm
diff --git a/FWCore/Utilities/test/BuildFile.xml b/FWCore/Utilities/test/BuildFile.xml
@@ -34,6 +34,10 @@
 <bin   name="testFWCoreUtilities" file="typeidbase_t.cppunit.cpp,typeid_t.cppunit.cpp,cputimer_t.cppunit.cpp,esinputtag.cppunit.cpp,extensioncord_t.cppunit.cpp,friendlyname_t.cppunit.cpp,signal_t.cppunit.cpp,soatuple_t.cppunit.cpp,transform.cppunit.cpp,callxnowait_t.cppunit.cpp,vecarray.cppunit.cpp,reusableobjectholder_t.cppunit.cpp,propagate_const_t.cppunit.cpp,indexset.cppunit.cpp">
   <use   name="cppunit"/>
 </bin>
+<bin    file="test_catch2_*.cc" name="testFWCoreUtilitiesCatch2">
+  <use name="catch2"/>
+  <use name="FWCore/Utilities"/>
+</bin>
 
 <bin   file="InputTag_t.cpp">
   <use name="tbb"/>

diff --git a/FWCore/Utilities/test/test_catch2_main.cc b/FWCore/Utilities/test/test_catch2_main.cc
@@ -0,0 +1,2 @@
+#define CATCH_CONFIG_MAIN
+#include "catch.hpp"
diff --git a/FWCore/Utilities/test/test_catch2_stemFromPath.cc b/FWCore/Utilities/test/test_catch2_stemFromPath.cc
@@ -0,0 +1,15 @@
+#include "FWCore/Utilities/interface/stemFromPath.h"
+
+#include "catch.hpp"
+
+TEST_CASE("Test stemFromPath", "[sources]") {
+  CHECK(edm::stemFromPath("foo.root") == "foo");
+  CHECK(edm::stemFromPath("/foo.root") == "foo");
+  CHECK(edm::stemFromPath("/bar/foo.root") == "foo");
+  CHECK(edm::stemFromPath("/bar///....//...///foo.root") == "foo");
+  CHECK(edm::stemFromPath("/bar/foo.xyzzy") == "foo");
+  CHECK(edm::stemFromPath("/bar/xyzzy.foo.root") == "xyzzy");
+  CHECK(edm::stemFromPath("file:foo.root") == "foo");
+  CHECK(edm::stemFromPath("file:/path/to/bar.txt") == "bar");
+  CHECK(edm::stemFromPath("root://server.somewhere:port/whatever?param=path/to/bar.txt") == "bar");
+}
diff --git a/IOPool/Input/src/RootEmbeddedFileSequence.cc b/IOPool/Input/src/RootEmbeddedFileSequence.cc
@@ -42,7 +42,8 @@ namespace edm {
         // and should be deleted from the code.
         initialNumberOfEventsToSkip_(pset.getUntrackedParameter<unsigned int>("skipEvents", 0U)),
         treeCacheSize_(pset.getUntrackedParameter<unsigned int>("cacheSize", roottree::defaultCacheSize)),
-        enablePrefetching_(false) {
+        enablePrefetching_(false),
+        enforceGUIDInFileName_(pset.getUntrackedParameter<bool>("enforceGUIDInFileName", false)) {
     if (noFiles()) {
       throw Exception(errors::NoSecondaryFiles)
           << "RootEmbeddedFileSequence no input files specified for secondary input source.\n";
@@ -138,7 +139,8 @@ namespace edm {
                                       currentIndexIntoFile,
                                       orderedProcessHistoryIDs_,
                                       input_.bypassVersionCheck(),
-                                      enablePrefetching_);
+                                      enablePrefetching_,
+                                      enforceGUIDInFileName_);
   }
 
   void RootEmbeddedFileSequence::skipEntries(unsigned int offset) {
@@ -336,5 +338,9 @@ namespace edm {
             "Skip the first 'skipEvents' events. Used only if 'sequential' is True and 'sameLumiBlock' is False");
     desc.addUntracked<unsigned int>("cacheSize", roottree::defaultCacheSize)
         ->setComment("Size of ROOT TTree prefetch cache.  Affects performance.");
+    desc.addUntracked<bool>("enforceGUIDInFileName", false)
+        ->setComment(
+            "True:  file name part is required to be equal to the GUID of the file\n"
+            "False: file name can be anything");
   }
 }  // namespace edm
diff --git a/IOPool/Input/src/RootEmbeddedFileSequence.h b/IOPool/Input/src/RootEmbeddedFileSequence.h
@@ -70,6 +70,7 @@ namespace edm {
     int initialNumberOfEventsToSkip_;
     unsigned int treeCacheSize_;
     bool enablePrefetching_;
+    bool enforceGUIDInFileName_;
   };  // class RootEmbeddedFileSequence
 }  // namespace edm
 #endif
diff --git a/IOPool/Input/src/RootFile.cc b/IOPool/Input/src/RootFile.cc
@@ -43,6 +43,7 @@
 #include "FWCore/Utilities/interface/FriendlyName.h"
 #include "FWCore/Utilities/interface/GlobalIdentifier.h"
 #include "FWCore/Utilities/interface/ReleaseVersion.h"
+#include "FWCore/Utilities/interface/stemFromPath.h"
 #include "FWCore/Version/interface/GetReleaseVersion.h"
 #include "IOPool/Common/interface/getWrapperBasePtr.h"
 
@@ -164,7 +165,8 @@ namespace edm {
                      bool bypassVersionCheck,
                      bool labelRawDataLikeMC,
                      bool usingGoToEvent,
-                     bool enablePrefetching)
+                     bool enablePrefetching,
+                     bool enforceGUIDInFileName)
       : file_(fileName),
         logicalFile_(logicalFileName),
         processConfiguration_(processConfiguration),
@@ -187,6 +189,7 @@ namespace edm {
         savedRunAuxiliary_(),
         skipAnyEvents_(skipAnyEvents),
         noEventSort_(noEventSort),
+        enforceGUIDInFileName_(enforceGUIDInFileName),
         whyNotFastClonable_(0),
         hasNewlyDroppedBranch_(),
         branchListIndexesUnchanged_(false),
@@ -1139,6 +1142,14 @@ namespace edm {
       throw Exception(errors::EventCorruption) << "'Events' tree is corrupted or not present\n"
                                                << "in the input file.\n";
     }
+    if (enforceGUIDInFileName_) {
+      auto guidFromName = stemFromPath(file_);
+      if (guidFromName != fid_.fid()) {
+        throw edm::Exception(edm::errors::FileNameInconsistentWithGUID)
+            << "GUID " << guidFromName << " extracted from file name " << file_
+            << " is inconsistent with the GUID read from the file " << fid_.fid();
+      }
+    }
 
     if (fileFormatVersion().hasIndexIntoFile()) {
       if (runTree().entries() > 0) {

diff --git a/IOPool/Input/src/RootFile.h b/IOPool/Input/src/RootFile.h
@@ -91,7 +91,8 @@ namespace edm {
              bool bypassVersionCheck,
              bool labelRawDataLikeMC,
              bool usingGoToEvent,
-             bool enablePrefetching);
+             bool enablePrefetching,
+             bool enforceGUIDInFileName);
 
     RootFile(std::string const& fileName,
              ProcessConfiguration const& processConfiguration,
@@ -113,7 +114,8 @@ namespace edm {
              std::vector<ProcessHistoryID>& orderedProcessHistoryIDs,
              bool bypassVersionCheck,
              bool labelRawDataLikeMC,
-             bool enablePrefetching)
+             bool enablePrefetching,
+             bool enforceGUIDInFileName)
         : RootFile(fileName,
                    processConfiguration,
                    logicalFileName,
@@ -142,7 +144,8 @@ namespace edm {
                    bypassVersionCheck,
                    labelRawDataLikeMC,
                    false,
-                   enablePrefetching) {}
+                   enablePrefetching,
+                   enforceGUIDInFileName) {}
 
     RootFile(std::string const& fileName,
              ProcessConfiguration const& processConfiguration,
@@ -159,7 +162,8 @@ namespace edm {
              std::vector<std::shared_ptr<IndexIntoFile>>::size_type currentIndexIntoFile,
              std::vector<ProcessHistoryID>& orderedProcessHistoryIDs,
              bool bypassVersionCheck,
-             bool enablePrefetching)
+             bool enablePrefetching,
+             bool enforceGUIDInFileName)
         : RootFile(fileName,
                    processConfiguration,
                    logicalFileName,
@@ -188,7 +192,8 @@ namespace edm {
                    bypassVersionCheck,
                    false,
                    false,
-                   enablePrefetching) {}
+                   enablePrefetching,
+                   enforceGUIDInFileName) {}
 
     ~RootFile();
 
@@ -325,6 +330,7 @@ namespace edm {
     edm::propagate_const<std::shared_ptr<RunAuxiliary>> savedRunAuxiliary_;
     bool skipAnyEvents_;
     bool noEventSort_;
+    bool enforceGUIDInFileName_;
     int whyNotFastClonable_;
     std::array<bool, NumBranchTypes> hasNewlyDroppedBranch_;
     bool branchListIndexesUnchanged_;

diff --git a/IOPool/Input/src/RootPrimaryFileSequence.cc b/IOPool/Input/src/RootPrimaryFileSequence.cc
@@ -32,7 +32,8 @@ namespace edm {
         treeCacheSize_(noEventSort_ ? pset.getUntrackedParameter<unsigned int>("cacheSize") : 0U),
         duplicateChecker_(new DuplicateChecker(pset)),
         usingGoToEvent_(false),
-        enablePrefetching_(false) {
+        enablePrefetching_(false),
+        enforceGUIDInFileName_(pset.getUntrackedParameter<bool>("enforceGUIDInFileName")) {
     // The SiteLocalConfig controls the TTreeCache size and the prefetching settings.
     Service<SiteLocalConfig> pSLC;
     if (pSLC.isAvailable()) {
@@ -137,7 +138,8 @@ namespace edm {
                                       input_.bypassVersionCheck(),
                                       input_.labelRawDataLikeMC(),
                                       usingGoToEvent_,
-                                      enablePrefetching_);
+                                      enablePrefetching_,
+                                      enforceGUIDInFileName_);
   }
 
   bool RootPrimaryFileSequence::nextFile() {
@@ -322,6 +324,10 @@ namespace edm {
         ->setComment(
             "'strict':     Branches in each input file must match those in the first file.\n"
             "'permissive': Branches in each input file may be any subset of those in the first file.");
+    desc.addUntracked<bool>("enforceGUIDInFileName", false)
+        ->setComment(
+            "True:  file name part is required to be equal to the GUID of the file\n"
+            "False: file name can be anything");
 
     EventSkipperByID::fillDescription(desc);
     DuplicateChecker::fillDescription(desc);

diff --git a/IOPool/Input/src/RootPrimaryFileSequence.h b/IOPool/Input/src/RootPrimaryFileSequence.h
@@ -77,6 +77,7 @@ namespace edm {
     edm::propagate_const<std::shared_ptr<DuplicateChecker>> duplicateChecker_;
     bool usingGoToEvent_;
     bool enablePrefetching_;
+    bool enforceGUIDInFileName_;
   };  // class RootPrimaryFileSequence
 }  // namespace edm
 #endif
diff --git a/IOPool/Input/src/RootSecondaryFileSequence.cc b/IOPool/Input/src/RootSecondaryFileSequence.cc
@@ -21,7 +21,11 @@ namespace edm {
   RootSecondaryFileSequence::RootSecondaryFileSequence(ParameterSet const& pset,
                                                        PoolSource& input,
                                                        InputFileCatalog const& catalog)
-      : RootInputFileSequence(pset, catalog), input_(input), orderedProcessHistoryIDs_(), enablePrefetching_(false) {
+      : RootInputFileSequence(pset, catalog),
+        input_(input),
+        orderedProcessHistoryIDs_(),
+        enablePrefetching_(false),
+        enforceGUIDInFileName_(pset.getUntrackedParameter<bool>("enforceGUIDInFileName")) {
     // The SiteLocalConfig controls the TTreeCache size and the prefetching settings.
     Service<SiteLocalConfig> pSLC;
     if (pSLC.isAvailable()) {
@@ -85,7 +89,8 @@ namespace edm {
                                       orderedProcessHistoryIDs_,
                                       input_.bypassVersionCheck(),
                                       input_.labelRawDataLikeMC(),
-                                      enablePrefetching_);
+                                      enablePrefetching_,
+                                      enforceGUIDInFileName_);
   }
 
   void RootSecondaryFileSequence::initAssociationsFromSecondary(std::set<BranchID> const& associationsFromSecondary) {

diff --git a/IOPool/Input/src/RootSecondaryFileSequence.h b/IOPool/Input/src/RootSecondaryFileSequence.h
@@ -45,6 +45,7 @@ namespace edm {
     std::vector<BranchID> associationsFromSecondary_;
     std::vector<ProcessHistoryID> orderedProcessHistoryIDs_;
     bool enablePrefetching_;
+    bool enforceGUIDInFileName_;
   };  // class RootSecondaryFileSequence
 }  // namespace edm
 #endif
diff --git a/IOPool/Input/test/PoolGUIDTest_cfg.py b/IOPool/Input/test/PoolGUIDTest_cfg.py
@@ -0,0 +1,29 @@
+# Configuration file for PoolInputTest
+
+import FWCore.ParameterSet.Config as cms
+import sys
+
+argv = []
+foundpy = False
+for a in sys.argv:
+    if foundpy:
+        argv.append(a)
+    if ".py" in a:
+        foundpy = True
+
+process = cms.Process("TESTRECO")
+process.load("FWCore.Framework.test.cmsExceptionsFatal_cff")
+
+process.maxEvents.input = -1
+process.OtherThing = cms.EDProducer("OtherThingProducer")
+process.Analysis = cms.EDAnalyzer("OtherThingAnalyzer")
+
+process.source = cms.Source("PoolSource",
+    setRunNumber = cms.untracked.uint32(621),
+    fileNames = cms.untracked.vstring(argv[0]),
+    enforceGUIDInFileName = cms.untracked.bool(True)
+)
+
+process.p = cms.Path(process.OtherThing*process.Analysis)
+
+
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1,2 @@
		#define CATCH_CONFIG_MAIN
		#include "catch.hpp"