Skip to content

Commit c64df4c

Browse files
cmsbuildcmsbuild
authored
Merge pull request #45610 from makortel/edmCheckClassVersionRefactor
Refactor `edmCheckClassVersion`
2 parents 9eb6120 + a2d77f2 commit c64df4c

File tree

6 files changed

+209
-95
lines changed

6 files changed

+209
-95
lines changed

‎FWCore/Reflection/scripts/edmCheckClassVersion‎

Lines changed: 106 additions & 95 deletions
Original file line numberDiff line numberDiff line change
@@ -46,99 +46,110 @@ def checkDictionaries(name):
4646

4747
return missingDict
4848

49-
#Setup the options
50-
from argparse import ArgumentParser, ArgumentDefaultsHelpFormatter
51-
oparser = ArgumentParser(formatter_class=ArgumentDefaultsHelpFormatter)
52-
oparser.add_argument("-d","--check_dictionaries", dest="checkdict",action="store_true",default=False,
53-
help="check that all required dictionaries are loaded")
54-
oparser.add_argument("-l","--lib", dest="library", type=str,
55-
help="specify the library to load. If not set classes are found using the PluginManager")
56-
oparser.add_argument("-x","--xml_file", dest="xmlfile",default="./classes_def.xml", type=str,
57-
help="the classes_def.xml file to read")
58-
oparser.add_argument("-g","--generate_new",dest="generate", action="store_true",default=False,
59-
help="instead of issuing errors, generate a new classes_def.xml file.")
60-
61-
options=oparser.parse_args()
62-
63-
ClassesDefUtils.initROOT(options.library)
64-
if options.library is None and options.checkdict:
65-
print ("Dictionary checks require a specific library")
66-
67-
missingDict = 0
68-
69-
ClassesDefUtils.initCheckClass()
70-
71-
try:
72-
p = ClassesDefUtils.XmlParser(options.xmlfile)
73-
except RuntimeError as e:
74-
print(f"Parsing {options.xmlfile} failed: {e}")
75-
sys.exit(1)
76-
foundErrors = dict()
77-
for name,info in p.classes.items():
78-
errorCode,rootClassVersion,classChecksum = ClassesDefUtils.checkClass(name,info[ClassesDefUtils.XmlParser.classVersionIndex],info[ClassesDefUtils.XmlParser.versionsToChecksumIndex])
79-
if errorCode != ClassesDefUtils.noError:
80-
foundErrors[name]=(errorCode,classChecksum,rootClassVersion)
81-
if options.checkdict :
82-
missingDict += checkDictionaries(name)
83-
84-
foundRootDoesNotMatchError = False
85-
originalToNormalizedNames = dict()
86-
for name,retValues in foundErrors.items():
87-
origName = p.classes[name][ClassesDefUtils.XmlParser.originalNameIndex]
88-
originalToNormalizedNames[origName]=name
89-
code = retValues[0]
90-
classVersion = p.classes[name][ClassesDefUtils.XmlParser.classVersionIndex]
91-
classChecksum = retValues[1]
92-
rootClassVersion = retValues[2]
93-
if code == ClassesDefUtils.errorRootDoesNotMatchClassDef:
94-
foundRootDoesNotMatchError=True
95-
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?")
96-
elif code == ClassesDefUtils.errorMustUpdateClassVersion and not options.generate:
97-
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))
98-
elif not options.generate:
99-
print ("error:class '"+name+"' needs to include the following as part of its 'class' declaration")
100-
print (' <version ClassVersion="'+str(classVersion)+'" checksum="'+str(classChecksum)+'"/>')
101-
102-
103-
if options.generate and not foundRootDoesNotMatchError and not missingDict:
104-
f = open(options.xmlfile)
105-
outFile = open('classes_def.xml.generated','w')
106-
out = ''
107-
for l in f.readlines():
108-
newLine = l
109-
if -1 != l.find('<class') and -1 != l.find('ClassVersion'):
110-
splitArgs = l.split('"')
111-
name = splitArgs[1]
112-
normName = originalToNormalizedNames.get(name,None)
113-
if normName is not None:
114-
indent = l.find('<')
115-
#this is a class with a problem
116-
classVersion = p.classes[normName][XmlParser.classVersionIndex]
117-
code,checksum,rootClassVersion = foundErrors[normName]
118-
hasNoSubElements = (-1 != l.find('/>'))
119-
if code == ClassesDefUtils.errorMustUpdateClassVersion:
120-
classVersion += 1
121-
parts = splitArgs[:]
122-
indexToClassVersion = 0
123-
for pt in parts:
124-
indexToClassVersion +=1
125-
if -1 != pt.find('ClassVersion'):
126-
break
127-
parts[indexToClassVersion]=str(classVersion)
128-
newLine = '"'.join(parts)
129-
130-
if hasNoSubElements:
131-
newLine = newLine.replace('/','')
132-
out +=newLine
133-
newLine =' '*indent+' <version ClassVersion="'+str(classVersion)+'" checksum="'+str(checksum)+'"/>\n'
134-
if hasNoSubElements:
135-
out += newLine
136-
newLine=' '*indent+'</class>\n'
137-
out +=newLine
138-
139-
outFile.writelines(out)
140-
141-
if (len(foundErrors)>0 and not options.generate) or (options.generate and foundRootDoesNotMatchError) or missingDict:
142-
import sys
143-
sys.exit(1)
49+
def checkClassDefinitions(classes, checkdict):
50+
missingDict = 0
51+
foundErrors = dict()
52+
for name,info in classes.items():
53+
errorCode,rootClassVersion,classChecksum = ClassesDefUtils.checkClass(name,info[ClassesDefUtils.XmlParser.classVersionIndex],info[ClassesDefUtils.XmlParser.versionsToChecksumIndex])
54+
if errorCode != ClassesDefUtils.noError:
55+
foundErrors[name]=(errorCode,classChecksum,rootClassVersion)
56+
if checkdict :
57+
missingDict += checkDictionaries(name)
58+
return (missingDict, foundErrors)
59+
60+
def checkErrors(foundErrors, classes, generate):
61+
foundRootDoesNotMatchError = False
62+
originalToNormalizedNames = dict()
63+
for name,retValues in foundErrors.items():
64+
origName = classes[name][ClassesDefUtils.XmlParser.originalNameIndex]
65+
originalToNormalizedNames[origName]=name
66+
code = retValues[0]
67+
classVersion = classes[name][ClassesDefUtils.XmlParser.classVersionIndex]
68+
classChecksum = retValues[1]
69+
rootClassVersion = retValues[2]
70+
if code == ClassesDefUtils.errorRootDoesNotMatchClassDef:
71+
foundRootDoesNotMatchError=True
72+
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?")
73+
elif code == ClassesDefUtils.errorMustUpdateClassVersion and not generate:
74+
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))
75+
elif not generate:
76+
print ("error:class '"+name+"' needs to include the following as part of its 'class' declaration")
77+
print (' <version ClassVersion="'+str(classVersion)+'" checksum="'+str(classChecksum)+'"/>')
78+
return (foundRootDoesNotMatchError, originalToNormalizedNames)
79+
80+
81+
def generate(oldfile, newfile, originalToNormalizedNames, classes, foundErrors):
82+
with open(oldfile) as f, open(newfile, 'w') as outFile:
83+
out = ''
84+
for l in f.readlines():
85+
newLine = l
86+
if -1 != l.find('<class') and -1 != l.find('ClassVersion'):
87+
splitArgs = l.split('"')
88+
name = splitArgs[1]
89+
normName = originalToNormalizedNames.get(name,None)
90+
if normName is not None:
91+
indent = l.find('<')
92+
#this is a class with a problem
93+
classVersion = classes[normName][ClassesDefUtils.XmlParser.classVersionIndex]
94+
code,checksum,rootClassVersion = foundErrors[normName]
95+
hasNoSubElements = (-1 != l.find('/>'))
96+
if code == ClassesDefUtils.errorMustUpdateClassVersion:
97+
classVersion += 1
98+
parts = splitArgs[:]
99+
indexToClassVersion = 0
100+
for pt in parts:
101+
indexToClassVersion +=1
102+
if -1 != pt.find('ClassVersion'):
103+
break
104+
parts[indexToClassVersion]=str(classVersion)
105+
newLine = '"'.join(parts)
106+
107+
if hasNoSubElements:
108+
newLine = newLine.replace('/','')
109+
out +=newLine
110+
newLine =' '*indent+' <version ClassVersion="'+str(classVersion)+'" checksum="'+str(checksum)+'"/>\n'
111+
if hasNoSubElements:
112+
out += newLine
113+
newLine=' '*indent+'</class>\n'
114+
out +=newLine
115+
116+
outFile.writelines(out)
117+
118+
def main(args):
119+
ClassesDefUtils.initROOT(args.library)
120+
if args.library is None and args.checkdict:
121+
print ("Dictionary checks require a specific library")
122+
ClassesDefUtils.initCheckClass()
123+
124+
try:
125+
p = ClassesDefUtils.XmlParser(args.xmlfile)
126+
except RuntimeError as e:
127+
print(f"Parsing {args.xmlfile} failed: {e}")
128+
return(1)
129+
130+
(missingDict, foundErrors) = checkClassDefinitions(p.classes, args.checkdict)
131+
(foundRootDoesNotMatchError, originalToNormalizedNames) = checkErrors(foundErrors, p.classes, args.generate)
132+
133+
if (len(foundErrors)>0 and not args.generate) or (args.generate and foundRootDoesNotMatchError) or missingDict:
134+
return 1
135+
136+
if args.generate:
137+
generate(args.xmlfile, 'classes_def.xml.generated', originalToNormalizedNames, p.classes, foundErrors)
138+
139+
return 0
140+
141+
if __name__ == "__main__":
142+
from argparse import ArgumentParser, ArgumentDefaultsHelpFormatter
143+
parser = ArgumentParser(formatter_class=ArgumentDefaultsHelpFormatter)
144+
parser.add_argument("-d","--check_dictionaries", dest="checkdict",action="store_true",default=False,
145+
help="check that all required dictionaries are loaded")
146+
parser.add_argument("-l","--lib", dest="library", type=str,
147+
help="specify the library to load. If not set classes are found using the PluginManager")
148+
parser.add_argument("-x","--xml_file", dest="xmlfile",default="./classes_def.xml", type=str,
149+
help="the classes_def.xml file to read")
150+
parser.add_argument("-g","--generate_new",dest="generate", action="store_true",default=False,
151+
help="instead of issuing errors, generate a new classes_def.xml file.")
152+
153+
args = parser.parse_args()
154+
sys.exit(main(args))
144155

‎FWCore/Reflection/test/BuildFile.xml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,3 +6,4 @@
66

77
<test name="TestFWCoreReflectionCheckClassVersion" command="run_checkClassVersion.sh"/>
88
<test name="TestFWCoreReflectionDumpClassVersion" command="run_dumpClassVersion.sh"/>
9+
<test name="TestFWCoreReflectionClassVersionUpdate" command="run_classVersionUpdate.sh"/>

‎FWCore/Reflection/test/dumpClassVersion_reference_afterUpdate.json‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
{
2+
"edm::Wrapper<edmtest::reflection::IntObject>": {
3+
"checksum": 536952283,
4+
"version": 4
5+
},
6+
"edmtest::reflection::IntObject": {
7+
"checksum": 2954816125,
8+
"version": 3
9+
}
10+
}

‎FWCore/Reflection/test/run_classVersionUpdate.sh‎

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
#!/bin/bash -e
2+
3+
SCRAM_TEST_NAME=TestFWCoreReflectionClassVersionUpdate
4+
rm -rf $SCRAM_TEST_NAME
5+
mkdir $SCRAM_TEST_NAME
6+
cd $SCRAM_TEST_NAME
7+
8+
# Create a new CMSSW dev area and build modified DataFormats/TestObjects in it
9+
NEW_CMSSW_BASE=$(/bin/pwd -P)/$CMSSW_VERSION
10+
scram -a $SCRAM_ARCH project $CMSSW_VERSION
11+
pushd $CMSSW_VERSION/src
12+
13+
# Copy FWCore/Reflection code to be able to edit it to make ROOT header parsing to fail
14+
for DIR in ${CMSSW_BASE} ${CMSSW_RELEASE_BASE} ${CMSSW_FULL_RELEASE_BASE} ; do
15+
if [ -d ${DIR}/src/FWCore/Reflection ]; then
16+
mkdir FWCore
17+
cp -Lr ${DIR}/src/FWCore/Reflection FWCore/
18+
break
19+
fi
20+
done
21+
if [ ! -e FWCore/Reflection ]; then
22+
echo "Failed to symlink FWCore/Reflection from local or release area"
23+
exit 1;
24+
fi
25+
26+
# The original src/ tree is protected from writes in PR tests
27+
chmod -R u+w FWCore/Reflection/test/stubs
28+
29+
# Modify the IntObject class to trigger a new version
30+
#
31+
# Just setting USER_CXXFLAGS for scram is not sufficient,because
32+
# somehow ROOT (as used by edmCheckClassVersion) still picks up the
33+
# version 3 of the class
34+
echo "#define FWCORE_REFLECTION_TEST_INTOBJECT_V4" | cat - FWCore/Reflection/test/stubs/TestObjects.h > TestObjects.h.tmp
35+
mv TestObjects.h.tmp FWCore/Reflection/test/stubs/TestObjects.h
36+
37+
38+
#Set env and build in sub-shel
39+
(eval $(scram run -sh) ; scram build -j $(nproc))
40+
41+
popd
42+
43+
# Prepend NEW_CMSSW_BASE's lib/src paths in to LD_LIBRARY_PATH and ROOT_INCLUDE_PATH
44+
export LD_LIBRARY_PATH=${NEW_CMSSW_BASE}/lib/${SCRAM_ARCH}:${LD_LIBRARY_PATH}
45+
export ROOT_INCLUDE_PATH=${NEW_CMSSW_BASE}/src:${ROOT_INCLUDE_PATH}
46+
47+
# Make the actual test
48+
echo "Initial setup complete, now for the actual test"
49+
XMLPATH=${SCRAM_TEST_PATH}/stubs
50+
LIBFILE=libFWCoreReflectionTestObjects.so
51+
52+
function die { echo Failure $1: status $2 ; exit $2 ; }
53+
function runFailure {
54+
$1 -l ${LIBFILE} -x ${XMLPATH}/$2 > log.txt && die "$1 for $2 did not fail" 1
55+
grep -q "$3" log.txt
56+
RET=$?
57+
if [ "$RET" != "0" ]; then
58+
echo "$1 for $2 did not contain '$3', log is below"
59+
cat log.txt
60+
exit 1
61+
fi
62+
}
63+
64+
echo "edmCheckClassVersion tests"
65+
66+
runFailure edmCheckClassVersion classes_def.xml "error: class 'edmtest::reflection::IntObject' has a different checksum for ClassVersion 3. Increment ClassVersion to 4 and assign it to checksum 2954816125"
67+
runFailure edmCheckClassVersion test_def_v4.xml "error: for class 'edmtest::reflection::IntObject' ROOT says the ClassVersion is 3 but classes_def.xml says it is 4. Are you sure everything compiled correctly?"
68+
69+
edmCheckClassVersion -l ${LIBFILE} -x ${XMLPATH}/classes_def.xml -g || die "edmCheckClassVersion -g failed" $?
70+
diff -u ${XMLPATH}/test_def_v4.xml classes_def.xml.generated || die "classes_def.xml.generated differs from expectation" $?
71+
72+
73+
echo "edmDumpClassVersion tests"
74+
75+
edmDumpClassVersion -l ${LIBFILE} -x ${XMLPATH}/classes_def.xml -o dump.json
76+
diff -u ${SCRAM_TEST_PATH}/dumpClassVersion_reference_afterUpdate.json dump.json || die "Unexpected class version dump" $?

‎FWCore/Reflection/test/stubs/TestObjects.h‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,10 +7,19 @@ namespace edmtest::reflection {
77
IntObject();
88
IntObject(int v) : value_(v) {}
99

10+
#ifdef FWCORE_REFLECTION_TEST_INTOBJECT_V4
11+
void set(int v) {
12+
value_ = v;
13+
set_ = true;
14+
}
15+
#endif
1016
int get() const { return value_; }
1117

1218
private:
1319
int value_ = 0;
20+
#ifdef FWCORE_REFLECTION_TEST_INTOBJECT_V4
21+
bool set_ = false;
22+
#endif
1423
};
1524
} // namespace edmtest::reflection
1625

‎FWCore/Reflection/test/stubs/test_def_v4.xml‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
<lcgdict>
2+
<class name="edmtest::reflection::IntObject" ClassVersion="4">
3+
<version ClassVersion="4" checksum="2954816125"/>
4+
<version ClassVersion="3" checksum="427917710"/>
5+
</class>
6+
<class name="edm::Wrapper<edmtest::reflection::IntObject>"/>
7+
</lcgdict>

0 commit comments

Comments
 (0)