-
Notifications
You must be signed in to change notification settings - Fork 4.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add edmDumpClassVersion and checkDictionaryUpdate.py scripts to check class version and checksum updates #45423
Merged
Merged
Changes from all commits
Commits
Show all changes
10 commits
Select commit
Hold shift + click to select a range
1556458
Replace atol with int
makortel e35c790
Add edmDumpClassVersion
makortel 58b6a52
Use XmlParser via ClassesDefUtils in edmCheckClassVersion
makortel 3b925b8
Abstract ROOT initialization
makortel 39a102a
Sort the output
makortel de1b07f
Output JSON, optionally to a file, and for now ignore errors from get…
makortel 1e910a3
Move edm{Check,Dump}ClassVersion to FWCore/Reflection
makortel 58f44d3
Add tests for edm{Check,Dump}ClassVersion
makortel cfe793c
Improve error messages for some cases where an expected XML attribute…
makortel 444d6a3
Add checkDictionaryUpdate.py script and its tests
makortel File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,143 @@ | ||
class XmlParser(object): | ||
"""Parses a classes_def.xml file looking for class declarations that contain | ||
ClassVersion attributes. Once found looks for sub-elements named 'version' | ||
which contain the ClassVersion to checksum mappings. | ||
""" | ||
|
||
#The following are constants used to describe what data is kept | ||
# in which index in the 'classes' member data | ||
originalNameIndex=0 | ||
classVersionIndex=1 | ||
versionsToChecksumIndex = 2 | ||
|
||
def __init__(self, filename, includeNonVersionedClasses=False, normalizeClassNames=True): | ||
self._file = filename | ||
self.classes = dict() | ||
self._presentClass = None | ||
self._presentClassForVersion = None | ||
self._includeNonVersionedClasses = includeNonVersionedClasses | ||
self._normalizeClassNames = normalizeClassNames | ||
self.readClassesDefXML() | ||
def readClassesDefXML(self): | ||
import xml.parsers.expat | ||
p = xml.parsers.expat.ParserCreate() | ||
p.StartElementHandler = self.start_element | ||
p.EndElementHandler = self.end_element | ||
f = open(self._file) | ||
# Replace any occurence of <>& in the attribute values by the xml parameter | ||
rxml, nxml = f.read(), '' | ||
q1,q2 = 0,0 | ||
for c in rxml : | ||
if (q1 or q2) and c == '<' : nxml += '<' | ||
elif (q1 or q2) and c == '>' : nxml += '>' | ||
# elif (q1 or q2) and c == '&' : nxml += '&' | ||
else : nxml += c | ||
if c == '"' : q1 = not q1 | ||
if c == "'" : q2 = not q2 | ||
try : p.Parse(nxml) | ||
except xml.parsers.expat.ExpatError as e : | ||
print ('--->> edmCheckClassVersion: ERROR: parsing selection file ',self._file) | ||
print ('--->> edmCheckClassVersion: ERROR: Error is:', e) | ||
raise | ||
f.close() | ||
def start_element(self,name,attrs): | ||
if name in ('class','struct'): | ||
if 'name' in attrs: | ||
self._presentClass=attrs['name'] | ||
normalizedName = self.genNName(attrs['name']) | ||
if 'ClassVersion' in attrs: | ||
self.classes[normalizedName]=[attrs['name'],int(attrs['ClassVersion']),[]] | ||
self._presentClassForVersion=normalizedName | ||
elif self._includeNonVersionedClasses: | ||
# skip transient data products | ||
if not ('persistent' in attrs and attrs['persistent'] == "false"): | ||
self.classes[normalizedName]=[attrs['name'],-1,[]] | ||
else: | ||
raise RuntimeError(f"There is an element '{name}' without 'name' attribute.") | ||
if name == 'version': | ||
if self._presentClassForVersion is None: | ||
raise RuntimeError(f"Class element for type '{self._presentClass}' contains a 'version' element, but 'ClassVersion' attribute is missing from the 'class' element") | ||
try: | ||
classVersion = int(attrs['ClassVersion']) | ||
except KeyError: | ||
raise RuntimeError(f"Version element for type '{self._presentClass}' is missing 'ClassVersion' attribute") | ||
try: | ||
checksum = int(attrs['checksum']) | ||
except KeyError: | ||
raise RuntimeError(f"Version element for type '{self._presentClass}' is missing 'checksum' attribute") | ||
self.classes[self._presentClassForVersion][XmlParser.versionsToChecksumIndex].append([classVersion, checksum]) | ||
pass | ||
def end_element(self,name): | ||
if name in ('class','struct'): | ||
self._presentClass = None | ||
self._presentClassForVersion = None | ||
def genNName(self, name ): | ||
if not self._normalizeClassNames: | ||
return name | ||
n_name = " ".join(name.split()) | ||
for e in [ ['long long unsigned int', 'unsigned long long'], | ||
['long long int', 'long long'], | ||
['unsigned short int', 'unsigned short'], | ||
['short unsigned int', 'unsigned short'], | ||
['short int', 'short'], | ||
['long unsigned int', 'unsigned long'], | ||
['unsigned long int', 'unsigned long'], | ||
['long int', 'long'], | ||
['std::string', 'std::basic_string<char>']] : | ||
n_name = n_name.replace(e[0],e[1]) | ||
n_name = n_name.replace(' ','') | ||
return n_name | ||
|
||
def initROOT(library): | ||
#Need to not have ROOT load .rootlogon.(C|py) since it can cause interference. | ||
import ROOT | ||
ROOT.PyConfig.DisableRootLogon = True | ||
|
||
#Keep ROOT from trying to use X11 | ||
ROOT.gROOT.SetBatch(True) | ||
ROOT.gROOT.ProcessLine(".autodict") | ||
if library is not None: | ||
if ROOT.gSystem.Load(library) < 0 : | ||
raise RuntimeError("failed to load library '"+library+"'") | ||
|
||
def initCheckClass(): | ||
"""Must be called before checkClass()""" | ||
import ROOT | ||
ROOT.gROOT.ProcessLine("class checkclass {public: int f(char const* name) {TClass* cl = TClass::GetClass(name); bool b = false; cl->GetCheckSum(b); return (int)b;} };") | ||
ROOT.gROOT.ProcessLine("checkclass checkTheClass;") | ||
|
||
|
||
#The following are error codes returned from checkClass | ||
noError = 0 | ||
errorRootDoesNotMatchClassDef =1 | ||
errorMustUpdateClassVersion=2 | ||
errorMustAddChecksum=3 | ||
|
||
def checkClass(name,version,versionsToChecksums): | ||
import ROOT | ||
c = ROOT.TClass.GetClass(name) | ||
if not c: | ||
raise RuntimeError("failed to load dictionary for class '"+name+"'") | ||
temp = "checkTheClass.f(" + '"' + name + '"' + ");" | ||
retval = ROOT.gROOT.ProcessLine(temp) | ||
if retval == 0 : | ||
raise RuntimeError("TClass::GetCheckSum: Failed to load dictionary for base class. See previous Error message") | ||
classChecksum = c.GetCheckSum() | ||
classVersion = c.GetClassVersion() | ||
|
||
#does this version match what is in the file? | ||
if version != classVersion: | ||
return (errorRootDoesNotMatchClassDef,classVersion,classChecksum) | ||
|
||
#is the version already in our list? | ||
found = False | ||
|
||
for v,cs in versionsToChecksums: | ||
if v == version: | ||
found = True | ||
if classChecksum != cs: | ||
return (errorMustUpdateClassVersion,classVersion,classChecksum) | ||
break | ||
if not found and classVersion != 0: | ||
return (errorMustAddChecksum,classVersion,classChecksum) | ||
return (noError,classVersion,classChecksum) |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,144 @@ | ||
#! /usr/bin/env python3 | ||
|
||
import sys | ||
import FWCore.Reflection.ClassesDefXmlUtils as ClassesDefUtils | ||
|
||
# recursively check the base classes for a class pointer | ||
# as building the streamer will crash if base classes are | ||
# incomplete | ||
def verifyBaseClasses(c) : | ||
missingBase = 0 | ||
|
||
# check that all bases are loaded | ||
bases = c.GetListOfBases() | ||
if not bases : | ||
print ("Incomplete class ", c.GetName()) | ||
return 1 | ||
|
||
for b in bases : | ||
bc = b.GetClassPointer() | ||
if bc : | ||
missingBase += verifyBaseClasses(bc) | ||
else : | ||
print ("Incomplete base class for ", c.GetName(), ": ", b.GetName()) | ||
missingBase += 1 | ||
|
||
return missingBase | ||
|
||
def checkDictionaries(name): | ||
c = ROOT.TClass.GetClass(name) | ||
if not c: | ||
raise RuntimeError("failed to load dictionary for class '"+name+"'") | ||
|
||
missingDict = verifyBaseClasses(c) | ||
if missingDict == 0 : | ||
si = c.GetStreamerInfo() | ||
if si : | ||
ts = si.GetElements() | ||
for telem in ts : | ||
clm = telem.GetClassPointer() | ||
if clm and not clm.IsLoaded() : | ||
print ("Missing dictionary for ", telem.GetName(), " type ", clm.GetName()) | ||
missingDict += 1 | ||
else : | ||
print ("No streamer info for ", c.GetName()) | ||
missingDict += 1 | ||
|
||
return missingDict | ||
|
||
#Setup the options | ||
from argparse import ArgumentParser, ArgumentDefaultsHelpFormatter | ||
oparser = ArgumentParser(formatter_class=ArgumentDefaultsHelpFormatter) | ||
oparser.add_argument("-d","--check_dictionaries", dest="checkdict",action="store_true",default=False, | ||
help="check that all required dictionaries are loaded") | ||
oparser.add_argument("-l","--lib", dest="library", type=str, | ||
help="specify the library to load. If not set classes are found using the PluginManager") | ||
oparser.add_argument("-x","--xml_file", dest="xmlfile",default="./classes_def.xml", type=str, | ||
help="the classes_def.xml file to read") | ||
oparser.add_argument("-g","--generate_new",dest="generate", action="store_true",default=False, | ||
help="instead of issuing errors, generate a new classes_def.xml file.") | ||
|
||
options=oparser.parse_args() | ||
|
||
ClassesDefUtils.initROOT(options.library) | ||
if options.library is None and options.checkdict: | ||
print ("Dictionary checks require a specific library") | ||
|
||
missingDict = 0 | ||
|
||
ClassesDefUtils.initCheckClass() | ||
|
||
try: | ||
p = ClassesDefUtils.XmlParser(options.xmlfile) | ||
except RuntimeError as e: | ||
print(f"Parsing {options.xmlfile} failed: {e}") | ||
sys.exit(1) | ||
foundErrors = dict() | ||
for name,info in p.classes.items(): | ||
errorCode,rootClassVersion,classChecksum = ClassesDefUtils.checkClass(name,info[ClassesDefUtils.XmlParser.classVersionIndex],info[ClassesDefUtils.XmlParser.versionsToChecksumIndex]) | ||
if errorCode != ClassesDefUtils.noError: | ||
foundErrors[name]=(errorCode,classChecksum,rootClassVersion) | ||
if options.checkdict : | ||
missingDict += checkDictionaries(name) | ||
|
||
foundRootDoesNotMatchError = False | ||
originalToNormalizedNames = dict() | ||
for name,retValues in foundErrors.items(): | ||
origName = p.classes[name][ClassesDefUtils.XmlParser.originalNameIndex] | ||
originalToNormalizedNames[origName]=name | ||
code = retValues[0] | ||
classVersion = p.classes[name][ClassesDefUtils.XmlParser.classVersionIndex] | ||
classChecksum = retValues[1] | ||
rootClassVersion = retValues[2] | ||
if code == ClassesDefUtils.errorRootDoesNotMatchClassDef: | ||
foundRootDoesNotMatchError=True | ||
print ("error: for class '"+name+"' ROOT says the ClassVersion is "+str(rootClassVersion)+" but classes_def.xml says it is "+str(classVersion)+". Are you sure everything compiled correctly?") | ||
elif code == ClassesDefUtils.errorMustUpdateClassVersion and not options.generate: | ||
print ("error: class '"+name+"' has a different checksum for ClassVersion "+str(classVersion)+". Increment ClassVersion to "+str(classVersion+1)+" and assign it to checksum "+str(classChecksum)) | ||
elif not options.generate: | ||
print ("error:class '"+name+"' needs to include the following as part of its 'class' declaration") | ||
print (' <version ClassVersion="'+str(classVersion)+'" checksum="'+str(classChecksum)+'"/>') | ||
|
||
|
||
if options.generate and not foundRootDoesNotMatchError and not missingDict: | ||
f = open(options.xmlfile) | ||
outFile = open('classes_def.xml.generated','w') | ||
out = '' | ||
for l in f.readlines(): | ||
newLine = l | ||
if -1 != l.find('<class') and -1 != l.find('ClassVersion'): | ||
splitArgs = l.split('"') | ||
name = splitArgs[1] | ||
normName = originalToNormalizedNames.get(name,None) | ||
if normName is not None: | ||
indent = l.find('<') | ||
#this is a class with a problem | ||
classVersion = p.classes[normName][XmlParser.classVersionIndex] | ||
code,checksum,rootClassVersion = foundErrors[normName] | ||
hasNoSubElements = (-1 != l.find('/>')) | ||
if code == ClassesDefUtils.errorMustUpdateClassVersion: | ||
classVersion += 1 | ||
parts = splitArgs[:] | ||
indexToClassVersion = 0 | ||
for pt in parts: | ||
indexToClassVersion +=1 | ||
if -1 != pt.find('ClassVersion'): | ||
break | ||
parts[indexToClassVersion]=str(classVersion) | ||
newLine = '"'.join(parts) | ||
|
||
if hasNoSubElements: | ||
newLine = newLine.replace('/','') | ||
out +=newLine | ||
newLine =' '*indent+' <version ClassVersion="'+str(classVersion)+'" checksum="'+str(checksum)+'"/>\n' | ||
if hasNoSubElements: | ||
out += newLine | ||
newLine=' '*indent+'</class>\n' | ||
out +=newLine | ||
|
||
outFile.writelines(out) | ||
|
||
if (len(foundErrors)>0 and not options.generate) or (options.generate and foundRootDoesNotMatchError) or missingDict: | ||
import sys | ||
sys.exit(1) | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,49 @@ | ||
#!/usr/bin/env python3 | ||
|
||
import sys | ||
import json | ||
import argparse | ||
|
||
import FWCore.Reflection.ClassesDefXmlUtils as ClassesDefUtils | ||
|
||
def main(args): | ||
ClassesDefUtils.initROOT(args.library) | ||
|
||
ClassesDefUtils.initCheckClass() | ||
try: | ||
p = ClassesDefUtils.XmlParser(args.xmlfile, includeNonVersionedClasses=True, normalizeClassNames=False) | ||
except RuntimeError as e: | ||
print(f"Parsing {args.xmlfile} failed: {e}") | ||
sys.exit(1) | ||
|
||
out = {} | ||
for name, info in p.classes.items(): | ||
try: | ||
(error, version, checksum) = ClassesDefUtils.checkClass(name, 0, {}) | ||
except RuntimeError as e: | ||
print(f"Ignoring class {name} as could not get its version and checksum, because: {e}") | ||
continue | ||
out[name] = dict( | ||
version = version, | ||
checksum = checksum | ||
) | ||
out_js = json.dumps(out, sort_keys=True, indent=1) | ||
if args.output is None: | ||
print(out_js) | ||
else: | ||
with open(args.output, "w") as f: | ||
f.write(out_js) | ||
return 0 | ||
|
||
if __name__ == "__main__": | ||
parser = argparse.ArgumentParser(formatter_class=argparse.ArgumentDefaultsHelpFormatter, | ||
description="Extracts class versions and checksums, in JSON format, for all non-transient clases defined in a given classes_def.xml file") | ||
parser.add_argument("-l","--lib", dest="library", type=str, | ||
help="specify the library to load. If not set classes are found using the PluginManager") | ||
parser.add_argument("-x","--xml_file", dest="xmlfile",default="./classes_def.xml", type=str, | ||
help="the classes_def.xml file to read") | ||
parser.add_argument("-o", "--output", type=str, default=None, | ||
help="File to save the output. If no file is specified, the JSON document is printed in stdout") | ||
|
||
args = parser.parse_args() | ||
sys.exit(main(args)) |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
<library file="stubs/*.cc" name="FWCoreReflectionTestObjects"> | ||
<flags LCG_DICT_HEADER="stubs/classes.h"/> | ||
<flags LCG_DICT_XML="stubs/classes_def.xml"/> | ||
<use name="DataFormats/Common"/> | ||
</library> | ||
|
||
<test name="TestFWCoreReflectionCheckClassVersion" command="run_checkClassVersion.sh"/> | ||
<test name="TestFWCoreReflectionDumpClassVersion" command="run_dumpClassVersion.sh"/> |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
{ | ||
"edm::Wrapper<edmtest::reflection::IntObject>": { | ||
"checksum": 536952283, | ||
"version": 4 | ||
}, | ||
"edmtest::reflection::IntObject": { | ||
"checksum": 427917710, | ||
"version": 3 | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,24 @@ | ||
#!/bin/bash | ||
|
||
function die { echo Failure $1: status $2 ; exit $2 ; } | ||
|
||
XMLPATH=${SCRAM_TEST_PATH}/stubs | ||
LIBFILE=${LOCALTOP}/lib/${SCRAM_ARCH}/libFWCoreReflectionTestObjects.so | ||
|
||
edmCheckClassVersion -l ${LIBFILE} -x ${XMLPATH}/classes_def.xml || die "edmCheckClassVersion failed" $? | ||
|
||
function runFailure { | ||
edmCheckClassVersion -l ${LIBFILE} -x ${XMLPATH}/$1 > log.txt && die "edmCheckClassVersion for $1 did not fail" 1 | ||
grep -q "$2" log.txt | ||
RET=$? | ||
if [ "$RET" != "0" ]; then | ||
echo "edmCheckClassVersion for $1 did not contain '$2', log is below" | ||
cat log.txt | ||
exit 1 | ||
fi | ||
} | ||
|
||
runFailure test_def_nameMissing.xml "There is an element 'class' without 'name' attribute" | ||
runFailure test_def_ClassVersionMissingInClass.xml "Class element for type 'edmtest::reflection::IntObject' contains a 'version' element, but 'ClassVersion' attribute is missing from the 'class' element" | ||
runFailure test_def_ClassVersionMissingInVersion.xml "Version element for type 'edmtest::reflection::IntObject' is missing 'ClassVersion' attribute" | ||
runFailure test_def_checksumMissingInVersion.xml "Version element for type 'edmtest::reflection::IntObject' is missing 'checksum' attribute" |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@smuzaffar By the way, is there a away to denote a dependence from a test to the build rule that copies
scripts/*
to$CMSSW_VERSION/bin/$SCRAM_ARCH
? Would e.g.do that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No @makortel , currently there is no way to provide such dependency
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@makortel , it should be trivial to add such a support e.g. running the unit test rule can depend on
bin
andscript
rules of its dependenciesThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, thanks. I don't think the support would be crucial for this PR, but would be nice to have at some point.