ros / genmsg

Standalone Python library for generating ROS message and service data structures for various languages
http://wiki.ros.org/genmsg
29 stars 74 forks source link

template_tools: 'interpreter globals collision' with multiple entries in template_map #53

Closed gavanderhoorn closed 10 years ago

gavanderhoorn commented 10 years ago

While implementing a new message generator for a language which needs multiple files generated per message, I get the following traceback when my generator script is invoked:

Traceback (most recent call last):
  File "/home/user/ros/msg_gen_ws/src/gentxt/scripts/genmsg_txt.py", line 23, in <module>
    {})
  File "/home/user/ros/msg_gen_ws/src/genmsg/src/genmsg/template_tools.py", line 206, in generate_from_command_line_options
    generate_from_file(argv[1], options.package, options.outdir, options.emdir, options.includepath, msg_template_dict, srv_template_dict)
  File "/home/user/ros/msg_gen_ws/src/genmsg/src/genmsg/template_tools.py", line 147, in generate_from_file
    _generate_msg_from_file(input_file, output_dir, template_dir, search_path, package_name, msg_template_dict)
  File "/home/user/ros/msg_gen_ws/src/genmsg/src/genmsg/template_tools.py", line 92, in _generate_msg_from_file
    search_path)
  File "/home/user/ros/msg_gen_ws/src/genmsg/src/genmsg/template_tools.py", line 70, in _generate_from_spec
    interpreter = em.Interpreter(output=ofile, globals=g, options={em.RAW_OPT:True,em.BUFFERED_OPT:True})
  File "/usr/lib/pymodules/python2.7/em.py", line 2054, in __init__
    self.fix()
  File "/usr/lib/pymodules/python2.7/em.py", line 2091, in fix
    raise Error, "interpreter globals collision"
em.Error: interpreter globals collision

Contents of language specific generator wrapper script:

msg_template_map = { 'msg.a.template' : '@NAME@.a.txt', 'msg.b.template': '@NAME@.b.txt', 'msg.template': '@NAME@.txt' }
srv_template_map = {}

if __name__ == "__main__":
    genmsg.template_tools.generate_from_command_line_options(sys.argv, msg_template_map, srv_template_map)

Playing around a bit I noticed that completely empty templates let the generator run to completion (producing just empty files, obviously).

I'm not too familiar with empy internals, but it looks like the interpreter is doing something to the dictionary (g) which causes some empy internal test to fail when a second interpreter is passed the dictionary, leading to the exception. Moving the dictionary initialisation inside the for loop that iterates over the template_map lets message generation successfully complete, but I'm not sure that is actually a solution.

gavanderhoorn commented 10 years ago

Current work around:

diff --git a/src/genmsg/template_tools.py b/src/genmsg/template_tools.py
index 6ac9323..1d0eb9d 100644
--- a/src/genmsg/template_tools.py
+++ b/src/genmsg/template_tools.py
@@ -48,17 +48,18 @@ def _generate_from_spec(input_file, output_dir, template_dir, msg_context, spec,

     md5sum = genmsg.gentools.compute_md5(msg_context, spec)

-    # Set dictionary for the generator interpreter
-    g = { "file_name_in":input_file,
-          "spec":spec,
-          "md5sum":md5sum,
-          "search_path":search_path,
-          "msg_context" : msg_context }
-    if isinstance(spec, genmsg.msgs.MsgSpec):
-        g['msg_definition'] = genmsg.gentools.compute_full_text(msg_context, spec)

     # Loop over all files to generate
     for template_file_name, output_file_name in template_map.items():
+        # Set dictionary for the generator interpreter
+        g = { "file_name_in":input_file,
+              "spec":spec,
+              "md5sum":md5sum,
+              "search_path":search_path,
+              "msg_context" : msg_context }
+        if isinstance(spec, genmsg.msgs.MsgSpec):
+            g['msg_definition'] = genmsg.gentools.compute_full_text(msg_context, spec)
+
         template_file = os.path.join(template_dir, template_file_name)
         output_file = os.path.join(output_dir, output_file_name.replace("@NAME@", spec.short_name))
dirk-thomas commented 10 years ago

Can you please provide a reproducible example which demonstrates the problem?

I would rather not rebuild the dictionary multiple times since the content is the same every time and calling compute_full_text multiple time is not necessary.

gavanderhoorn commented 10 years ago

Sure: mwe53.rosinstall (default std_msgs, forked gencpp (just duplicated msg.h.template, added to msg_template_map)). genmsg is binary install from distribution deb (originally on hydro/precise, but indigo/trusty shows the same problem).

Building the source space should get you the traceback immediately when trying to generate CPP for Float64MultiArray.msg (at least, that's the first message it tries to generate on my system).

I would rather not rebuild the dictionary multiple times since the content is the same every time and calling compute_full_text multiple time is not necessary.

As I wrote: I'm not really familiar with empy, so I was just brute forcing.

gavanderhoorn commented 10 years ago

I would rather not rebuild the dictionary multiple times since the content is the same every time and calling compute_full_text multiple time is not necessary.

Passing a fresh copy of g to every interpreter might be another option:

diff --git a/src/genmsg/template_tools.py b/src/genmsg/template_tools.py
index 6ac9323..3021731 100644
--- a/src/genmsg/template_tools.py
+++ b/src/genmsg/template_tools.py
@@ -67,7 +67,7 @@ def _generate_from_spec(input_file, output_dir, template_dir, msg_context, spec,
         ofile = open(output_file, 'w') #todo try

         # todo, reuse interpreter
-        interpreter = em.Interpreter(output=ofile, globals=g, options={em.RAW_OPT:True,em.BUFFERED_OPT:True})
+        interpreter = em.Interpreter(output=ofile, globals=dict(g), options={em.RAW_OPT:True,em.BUFFERED_OPT:True})
         if not os.path.isfile(template_file):
             ofile.close()
             os.remove(output_file)
dirk-thomas commented 10 years ago

Thank you for the easy to reproduce example. I created a pull request with your proposed solution (#54). I only slightly modified it to call compute_full_text() only once (https://github.com/ros/genmsg/pull/54/files#diff-d803838e83d1bf75185ead59abb40ab8R53).

Can you please confirm that it works for you too?

gavanderhoorn commented 10 years ago

I've commented on #54. Thanks for the quick fix.

Out of curiosity: a copy of the entire dict is too expensive?

dirk-thomas commented 10 years ago

The dict copy as you suggested would also be fine. It is just a little bit less obvious when reading the code imo (easy to miss it, looks like "conversion" to dict). It was only important to not invoke the functions multiple time (though I am not sure how "expensive" it is).