From 032fa5a746efa88b5001cd11af6ed6552577faba Mon Sep 17 00:00:00 2001 From: Ken Conley Date: Sat, 23 Oct 2010 04:39:47 +0000 Subject: [PATCH] rospy: #2990 bug fix for md5sum calculation when fieldname collides with Python keyword. Also made keyword list more robust by pulling from python keywords module instead of custom file. This required cleaning up example files and tests. --- core/roslib/scripts/python-reserved-words.txt | 32 -------------- core/roslib/src/roslib/genpy.py | 44 +++++++------------ test/test_roslaunch/test/test_xmlloader.py | 2 +- .../test/xml/test-params-valid.xml | 2 +- test/test_rospy/msg/PythonKeyword.msg | 1 + test/test_rospy/test/test_genmsg_py.py | 8 ++++ tools/roslaunch/example.launch | 2 +- 7 files changed, 27 insertions(+), 64 deletions(-) delete mode 100644 core/roslib/scripts/python-reserved-words.txt create mode 100644 test/test_rospy/msg/PythonKeyword.msg diff --git a/core/roslib/scripts/python-reserved-words.txt b/core/roslib/scripts/python-reserved-words.txt deleted file mode 100644 index 14ac6ed5..00000000 --- a/core/roslib/scripts/python-reserved-words.txt +++ /dev/null @@ -1,32 +0,0 @@ -and -as -assert -break -class -continue -def -del -elif -else -except -exec -finally -for -from -global -import -if -in -is -lambda -not -or -pass -print -raise -return -self -try -while -with -yield diff --git a/core/roslib/src/roslib/genpy.py b/core/roslib/src/roslib/genpy.py index 4ec61bf4..1f5bb038 100644 --- a/core/roslib/src/roslib/genpy.py +++ b/core/roslib/src/roslib/genpy.py @@ -49,6 +49,7 @@ The structure of the serialization descends several levels of serializers: # creates circular deps import os +import keyword import shutil import atexit import itertools @@ -254,30 +255,9 @@ def _remap_reserved(field_name): @rtype: str """ #doing this lazy saves 0.05s on up-to-date builds - if not _reserved_words: - _load_reserved_words() - if field_name in _reserved_words: + if field_name in keyword.kwlist: return field_name + "_" return field_name - -_reserved_words = [] -def _load_reserved_words(): - """ - load python reserved words file into global _reserved_words. these - reserved words cannot be field names without being altered first - """ - global _reserved_words - roslib_dir = roslib.packages.get_pkg_dir('roslib') - reserved_words_file = os.path.join(roslib_dir, 'scripts', 'python-reserved-words.txt') - if not os.path.exists(reserved_words_file): - print >> sys.stderr, "Cannot load python reserved words file [%s]"%reserved_words_file - return False - f = open(reserved_words_file, 'r') - try: - _reserved_words = [w.strip() for w in f.readlines() if w] - return True - finally: - f.close() ################################################################################ # (de)serialization routines @@ -951,7 +931,17 @@ def msg_generator(package, name, spec): @param spec: parsed .msg specification @type spec: L{MsgSpec} """ - spec = make_python_safe(spec) # remap spec names to be Python-safe + + # #2990: have to compute md5sum before any calls to make_python_safe + + # generate dependencies dictionary. omit files calculation as we + # rely on in-memory MsgSpecs instead so that we can generate code + # for older versions of msg files + gendeps_dict = roslib.gentools.get_dependencies(spec, package, compute_files=False) + md5sum = roslib.gentools.compute_md5(gendeps_dict) + + # remap spec names to be Python-safe + spec = make_python_safe(spec) spec_names = spec.names # #1807 : this will be much cleaner when msggenerator library is @@ -972,14 +962,10 @@ def msg_generator(package, name, spec): fulltype = '%s%s%s'%(package, roslib.msgs.SEP, name) - # generate dependencies dictionary. omit files calculation as we - # rely on in-memory MsgSpecs instead so that we can generate code - # for older versions of msg files - gendeps_dict = roslib.gentools.get_dependencies(spec, package, compute_files=False) #Yield data class first, e.g. Point2D yield 'class %s(roslib.message.Message):'%name - yield ' _md5sum = "%s"'%roslib.gentools.compute_md5(gendeps_dict) - yield ' _type = "%s"'%fulltype + yield ' _md5sum = "%s"'%(md5sum) + yield ' _type = "%s"'%(fulltype) yield ' _has_header = %s #flag to mark the presence of a Header object'%spec.has_header() # note: we introduce an extra newline to protect the escaping from quotes in the message yield ' _full_text = """%s\n"""'%compute_full_text_escaped(gendeps_dict) diff --git a/test/test_roslaunch/test/test_xmlloader.py b/test/test_roslaunch/test/test_xmlloader.py index b11aea1a..f62031d8 100755 --- a/test/test_roslaunch/test/test_xmlloader.py +++ b/test/test_roslaunch/test/test_xmlloader.py @@ -212,7 +212,7 @@ class TestXmlLoader(unittest.TestCase): p = [p for p in mock.params if p.key == '/binaryfile'][0] self.assertEquals(xmlrpclib.Binary(contents), p.value, 1) - f = open(os.path.join(get_pkg_dir('roslib'), 'scripts', 'python-reserved-words.txt')) + f = open(os.path.join(get_pkg_dir('roslaunch'), 'example.launch')) try: contents = f.read() finally: diff --git a/test/test_roslaunch/test/xml/test-params-valid.xml b/test/test_roslaunch/test/xml/test-params-valid.xml index ff76f9f4..96fe3d66 100644 --- a/test/test_roslaunch/test/xml/test-params-valid.xml +++ b/test/test_roslaunch/test/xml/test-params-valid.xml @@ -30,7 +30,7 @@ - + diff --git a/test/test_rospy/msg/PythonKeyword.msg b/test/test_rospy/msg/PythonKeyword.msg new file mode 100644 index 00000000..0949ea88 --- /dev/null +++ b/test/test_rospy/msg/PythonKeyword.msg @@ -0,0 +1 @@ +int32 yield diff --git a/test/test_rospy/test/test_genmsg_py.py b/test/test_rospy/test/test_genmsg_py.py index 36b817df..1cec1f15 100644 --- a/test/test_rospy/test/test_genmsg_py.py +++ b/test/test_rospy/test/test_genmsg_py.py @@ -48,6 +48,14 @@ from roslib.message import SerializationError class TestGenmsgPy(unittest.TestCase): + def test_PythonKeyword(self): + from test_ros.msg import PythonKeyword + # the md5sum is pulled from the c++ message generator. The + # test here is that the Python msg generator didn't + # accidentally mutate a md5sum based on a message that has its + # fieldname remapped. + self.assertEquals(PythonKeyword._md5sum, "1330d6bbfad8e75334346fec949d5133") + ## Utility for testing roundtrip serialization ## @param orig Message to test roundtrip serialization of ## @param blank Uninitialized instance of message to deserialize into diff --git a/tools/roslaunch/example.launch b/tools/roslaunch/example.launch index ca15f5f4..5bca7f33 100644 --- a/tools/roslaunch/example.launch +++ b/tools/roslaunch/example.launch @@ -30,7 +30,7 @@ - +