diff --git a/FWCore/Framework/python/test/cmsExceptionsFatalOption_cff.py b/FWCore/Framework/python/test/cmsExceptionsFatalOption_cff.py index c78bf7f515d12..9253a546b5fbd 100644 --- 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 new file mode 100755 index 0000000000000..7ba788686f5e6 --- /dev/null +++ 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 index d449058593e3a..a2489bf811e49 100644 --- 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 { @@ -63,6 +62,8 @@ namespace edm { ExceededResourceRSS = 8031, ExceededResourceTime = 8032, + FileNameInconsistentWithGUID = 8034, + CaughtSignal = 9000 }; diff --git a/FWCore/Utilities/interface/stemFromPath.h b/FWCore/Utilities/interface/stemFromPath.h new file mode 100644 index 0000000000000..cf7470fb076c2 --- /dev/null +++ b/FWCore/Utilities/interface/stemFromPath.h @@ -0,0 +1,17 @@ +#ifndef FWCore_Utilities_stemFromPath_h +#define FWCore_Utilities_stemFromPath_h + +#include + +namespace edm { + // This functions extracts the stem of a file from the path (= file + // name without the extension). + // + // 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 stemFromPath(const std::string& path); +} // namespace edm + +#endif diff --git a/FWCore/Utilities/src/EDMException.cc b/FWCore/Utilities/src/EDMException.cc index 4bd5df73f7ea1..312c8cf8add78 100644 --- a/FWCore/Utilities/src/EDMException.cc +++ b/FWCore/Utilities/src/EDMException.cc @@ -41,6 +41,7 @@ namespace edm { FILLENTRY(ExceededResourceVSize), FILLENTRY(ExceededResourceRSS), FILLENTRY(ExceededResourceTime), + FILLENTRY(FileNameInconsistentWithGUID), FILLENTRY(CaughtSignal) }; static const std::string kUnknownCode("unknownCode"); diff --git a/FWCore/Utilities/src/stemFromPath.cc b/FWCore/Utilities/src/stemFromPath.cc new file mode 100644 index 0000000000000..dc1791e0b47ef --- /dev/null +++ b/FWCore/Utilities/src/stemFromPath.cc @@ -0,0 +1,20 @@ +#include "FWCore/Utilities/interface/stemFromPath.h" + +namespace edm { + std::string stemFromPath(const std::string& path) { + auto begin = path.rfind("/"); + if (begin == std::string::npos) { + begin = path.rfind(":"); + if (begin == std::string::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 index 207c51bcffe83..030fb016eb4c4 100644 --- a/FWCore/Utilities/test/BuildFile.xml +++ b/FWCore/Utilities/test/BuildFile.xml @@ -34,6 +34,8 @@ + + diff --git a/FWCore/Utilities/test/test_stemFromPath.cc b/FWCore/Utilities/test/test_stemFromPath.cc new file mode 100644 index 0000000000000..71fb9582f4e5f --- /dev/null +++ b/FWCore/Utilities/test/test_stemFromPath.cc @@ -0,0 +1,16 @@ +#include "FWCore/Utilities/interface/stemFromPath.h" + +#include + +int main() { + assert(edm::stemFromPath("foo.root") == "foo"); + assert(edm::stemFromPath("/foo.root") == "foo"); + assert(edm::stemFromPath("/bar/foo.root") == "foo"); + assert(edm::stemFromPath("/bar///....//...///foo.root") == "foo"); + assert(edm::stemFromPath("/bar/foo.xyzzy") == "foo"); + assert(edm::stemFromPath("/bar/xyzzy.foo.root") == "xyzzy"); + assert(edm::stemFromPath("file:foo.root") == "foo"); + assert(edm::stemFromPath("file:/path/to/bar.txt") == "bar"); + assert(edm::stemFromPath("root://server.somewhere:port/whatever?param=path/to/bar.txt") == "bar"); + return 0; +} diff --git a/IOPool/Input/src/RootEmbeddedFileSequence.cc b/IOPool/Input/src/RootEmbeddedFileSequence.cc index aade4e83fe0bf..520846243ccda 100644 --- a/IOPool/Input/src/RootEmbeddedFileSequence.cc +++ b/IOPool/Input/src/RootEmbeddedFileSequence.cc @@ -43,7 +43,8 @@ namespace edm { // and should be deleted from the code. initialNumberOfEventsToSkip_(pset.getUntrackedParameter("skipEvents", 0U)), treeCacheSize_(pset.getUntrackedParameter("cacheSize", roottree::defaultCacheSize)), - enablePrefetching_(false) { + enablePrefetching_(false), + enforceGUIDInFileName_(pset.getUntrackedParameter("enforceGUIDInFileName", false)) { if(noFiles()) { throw Exception(errors::Configuration) << "RootEmbeddedFileSequence no input files specified for secondary input source.\n"; @@ -144,7 +145,8 @@ namespace edm { currentIndexIntoFile, orderedProcessHistoryIDs_, input_.bypassVersionCheck(), - enablePrefetching_); + enablePrefetching_, + enforceGUIDInFileName_); } void @@ -336,5 +338,9 @@ namespace edm { ->setComment("Skip the first 'skipEvents' events. Used only if 'sequential' is True and 'sameLumiBlock' is False"); desc.addUntracked("cacheSize", roottree::defaultCacheSize) ->setComment("Size of ROOT TTree prefetch cache. Affects performance."); + desc.addUntracked("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"); } } diff --git a/IOPool/Input/src/RootEmbeddedFileSequence.h b/IOPool/Input/src/RootEmbeddedFileSequence.h index b01b268d9ff24..62b6bb0e0b8df 100644 --- a/IOPool/Input/src/RootEmbeddedFileSequence.h +++ b/IOPool/Input/src/RootEmbeddedFileSequence.h @@ -65,6 +65,7 @@ namespace edm { int initialNumberOfEventsToSkip_; unsigned int treeCacheSize_; bool enablePrefetching_; + bool enforceGUIDInFileName_; }; // class RootEmbeddedFileSequence } #endif diff --git a/IOPool/Input/src/RootFile.cc b/IOPool/Input/src/RootFile.cc index 005f1d8a616df..a8654de2a316e 100644 --- a/IOPool/Input/src/RootFile.cc +++ b/IOPool/Input/src/RootFile.cc @@ -41,6 +41,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" @@ -155,7 +156,8 @@ namespace edm { bool bypassVersionCheck, bool labelRawDataLikeMC, bool usingGoToEvent, - bool enablePrefetching) : + bool enablePrefetching, + bool enforceGUIDInFileName) : file_(fileName), logicalFile_(logicalFileName), processConfiguration_(processConfiguration), @@ -175,6 +177,7 @@ namespace edm { savedRunAuxiliary_(), skipAnyEvents_(skipAnyEvents), noEventSort_(noEventSort), + enforceGUIDInFileName_(enforceGUIDInFileName), whyNotFastClonable_(0), hasNewlyDroppedBranch_(), branchListIndexesUnchanged_(false), @@ -1097,6 +1100,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 index 12d3e1cef900c..e3354b6584c52 100644 --- a/IOPool/Input/src/RootFile.h +++ b/IOPool/Input/src/RootFile.h @@ -89,7 +89,8 @@ namespace edm { bool bypassVersionCheck, bool labelRawDataLikeMC, bool usingGoToEvent, - bool enablePrefetching); + bool enablePrefetching, + bool enforceGUIDInFileName); RootFile(std::string const& fileName, ProcessConfiguration const& processConfiguration, @@ -111,7 +112,8 @@ namespace edm { std::vector& orderedProcessHistoryIDs, bool bypassVersionCheck, bool labelRawDataLikeMC, - bool enablePrefetching) : RootFile( + bool enablePrefetching, + bool enforceGUIDInFileName) : RootFile( fileName, processConfiguration, logicalFileName, filePtr, nullptr, false, -1, -1, nStreams, 0U, treeMaxVirtualSize, processingMode, runHelper, @@ -120,7 +122,7 @@ namespace edm { nullptr, dropDescendantsOfDroppedProducts, processHistoryRegistry, indexesIntoFiles, currentIndexIntoFile, orderedProcessHistoryIDs, bypassVersionCheck, labelRawDataLikeMC, - false, enablePrefetching) {} + false, enablePrefetching, enforceGUIDInFileName) {} RootFile(std::string const& fileName, ProcessConfiguration const& processConfiguration, @@ -137,7 +139,8 @@ namespace edm { std::vector >::size_type currentIndexIntoFile, std::vector& orderedProcessHistoryIDs, bool bypassVersionCheck, - bool enablePrefetching) : RootFile( + bool enablePrefetching, + bool enforceGUIDInFileName) : RootFile( fileName, processConfiguration, logicalFileName, filePtr, nullptr, false, -1, -1, nStreams, treeCacheSize, treeMaxVirtualSize, InputSource::RunsLumisAndEvents, runHelper, @@ -145,7 +148,7 @@ namespace edm { nullptr, nullptr, false, processHistoryRegistry, indexesIntoFiles, currentIndexIntoFile, orderedProcessHistoryIDs, bypassVersionCheck, false, - false, enablePrefetching) {} + false, enablePrefetching, enforceGUIDInFileName) {} ~RootFile(); @@ -269,6 +272,7 @@ namespace edm { edm::propagate_const> savedRunAuxiliary_; bool skipAnyEvents_; bool noEventSort_; + bool enforceGUIDInFileName_; int whyNotFastClonable_; std::array hasNewlyDroppedBranch_; bool branchListIndexesUnchanged_; diff --git a/IOPool/Input/src/RootPrimaryFileSequence.cc b/IOPool/Input/src/RootPrimaryFileSequence.cc index 867c11b13436f..7e3a8d1ba46ee 100644 --- a/IOPool/Input/src/RootPrimaryFileSequence.cc +++ b/IOPool/Input/src/RootPrimaryFileSequence.cc @@ -33,7 +33,8 @@ namespace edm { treeCacheSize_(noEventSort_ ? pset.getUntrackedParameter("cacheSize") : 0U), duplicateChecker_(new DuplicateChecker(pset)), usingGoToEvent_(false), - enablePrefetching_(false) { + enablePrefetching_(false), + enforceGUIDInFileName_(pset.getUntrackedParameter("enforceGUIDInFileName")) { // The SiteLocalConfig controls the TTreeCache size and the prefetching settings. Service pSLC; @@ -143,7 +144,8 @@ namespace edm { input_.bypassVersionCheck(), input_.labelRawDataLikeMC(), usingGoToEvent_, - enablePrefetching_); + enablePrefetching_, + enforceGUIDInFileName_); } bool RootPrimaryFileSequence::nextFile() { @@ -334,6 +336,10 @@ namespace edm { desc.addUntracked("branchesMustMatch", defaultString) ->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("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 index 35308d0679a91..d919159690dd2 100644 --- a/IOPool/Input/src/RootPrimaryFileSequence.h +++ b/IOPool/Input/src/RootPrimaryFileSequence.h @@ -78,6 +78,7 @@ namespace edm { edm::propagate_const> duplicateChecker_; bool usingGoToEvent_; bool enablePrefetching_; + bool enforceGUIDInFileName_; }; // class RootPrimaryFileSequence } #endif diff --git a/IOPool/Input/src/RootSecondaryFileSequence.cc b/IOPool/Input/src/RootSecondaryFileSequence.cc index aca06e96856a3..6ad41390185ca 100644 --- a/IOPool/Input/src/RootSecondaryFileSequence.cc +++ b/IOPool/Input/src/RootSecondaryFileSequence.cc @@ -25,7 +25,8 @@ namespace edm { RootInputFileSequence(pset, catalog), input_(input), orderedProcessHistoryIDs_(), - enablePrefetching_(false) { + enablePrefetching_(false), + enforceGUIDInFileName_(pset.getUntrackedParameter("enforceGUIDInFileName")) { // The SiteLocalConfig controls the TTreeCache size and the prefetching settings. Service pSLC; @@ -95,7 +96,8 @@ namespace edm { orderedProcessHistoryIDs_, input_.bypassVersionCheck(), input_.labelRawDataLikeMC(), - enablePrefetching_); + enablePrefetching_, + enforceGUIDInFileName_); } void diff --git a/IOPool/Input/src/RootSecondaryFileSequence.h b/IOPool/Input/src/RootSecondaryFileSequence.h index d74f2ad85e272..af91b30f0d5b5 100644 --- a/IOPool/Input/src/RootSecondaryFileSequence.h +++ b/IOPool/Input/src/RootSecondaryFileSequence.h @@ -46,6 +46,7 @@ namespace edm { std::vector associationsFromSecondary_; std::vector orderedProcessHistoryIDs_; bool enablePrefetching_; + bool enforceGUIDInFileName_; }; // class RootSecondaryFileSequence } #endif diff --git a/IOPool/Input/test/PoolGUIDTest_cfg.py b/IOPool/Input/test/PoolGUIDTest_cfg.py new file mode 100644 index 0000000000000..cc0db354e3559 --- /dev/null +++ b/IOPool/Input/test/PoolGUIDTest_cfg.py @@ -0,0 +1,31 @@ +# 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 = cms.untracked.PSet( + input = cms.untracked.int32(-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) + + diff --git a/IOPool/Input/test/PrePoolInputTest_cfg.py b/IOPool/Input/test/PrePoolInputTest_cfg.py index dc56d73909128..767a704daf0fc 100644 --- a/IOPool/Input/test/PrePoolInputTest_cfg.py +++ b/IOPool/Input/test/PrePoolInputTest_cfg.py @@ -3,27 +3,34 @@ # Configuration file for PrePoolInputTest import FWCore.ParameterSet.Config as cms -from sys import argv -from string import atoi +import sys + +argv = [] +foundpy = False +for a in sys.argv: + if foundpy: + argv.append(a) + if ".py" in a: + foundpy = True process = cms.Process("TESTPROD") process.load("FWCore.Framework.test.cmsExceptionsFatal_cff") process.maxEvents = cms.untracked.PSet( - input = cms.untracked.int32(atoi(argv[3])) + input = cms.untracked.int32(int(argv[1])) ) process.Thing = cms.EDProducer("ThingProducer") process.output = cms.OutputModule("PoolOutputModule", - fileName = cms.untracked.string(argv[2]) + fileName = cms.untracked.string(argv[0]) ) process.source = cms.Source("EmptySource", - firstRun = cms.untracked.uint32(atoi(argv[4])), - numberEventsInRun = cms.untracked.uint32(atoi(argv[5])), - firstLuminosityBlock = cms.untracked.uint32(atoi(argv[6])), - numberEventsInLuminosityBlock = cms.untracked.uint32(atoi(argv[7])) + firstRun = cms.untracked.uint32(int(argv[2])), + numberEventsInRun = cms.untracked.uint32(int(argv[3])), + firstLuminosityBlock = cms.untracked.uint32(int(argv[4])), + numberEventsInLuminosityBlock = cms.untracked.uint32(int(argv[5])) ) process.p = cms.Path(process.Thing) diff --git a/IOPool/Input/test/TestPoolInput.sh b/IOPool/Input/test/TestPoolInput.sh index 02ff48de6ad15..7f798c3956da1 100755 --- a/IOPool/Input/test/TestPoolInput.sh +++ b/IOPool/Input/test/TestPoolInput.sh @@ -4,7 +4,18 @@ function die { echo $1: status $2 ; exit $2; } pushd ${LOCAL_TMP_DIR} -cmsRun ${LOCAL_TEST_DIR}/PrePoolInputTest_cfg.py PoolInputTest.root 11 561 7 6 3 || die 'Failure using PrePoolInputTest_cfg.py' $? +cmsRun -j PoolInputTest_jobreport.xml ${LOCAL_TEST_DIR}/PrePoolInputTest_cfg.py PoolInputTest.root 11 561 7 6 3 || die 'Failure using PrePoolInputTest_cfg.py' $? + +cmsRun -j PoolGuidTest_jobreport.xml ${LOCAL_TEST_DIR}/PoolGUIDTest_cfg.py file:PoolInputTest.root && die 'PoolGUIDTest_cfg.py PoolInputTest.root did not throw an exception' 1 +GUID_EXIT_CODE=$(edmFjrDump --exitCode PoolGuidTest_jobreport.xml) +if [ "x${GUID_EXIT_CODE}" != "x8034" ]; then + echo "Inconsistent GUID test reported exit code ${GUID_EXIT_CODE} which is different from the expected 8034" + exit 1 +fi +GUID_NAME=$(edmFjrDump --guid PoolInputTest_jobreport.xml).root +cp PoolInputTest.root ${GUID_NAME} +cmsRun ${LOCAL_TEST_DIR}/PoolGUIDTest_cfg.py file:${GUID_NAME} || die 'Failure using PoolGUIDTest_cfg.py ${GUID_NAME}' $? + cp PoolInputTest.root PoolInputOther.root