use latin-1 encoding when reading/writing .idl files, prepend BOM to …

…generated C/C++ files when necessary (#391) * use utf-8 encoding when reading/writing expanded templates Signed-off-by: Dirk Thomas <[email protected]> * inject BOM into generated C/C++ code when necessary Signed-off-by: Dirk Thomas <[email protected]> * use latin-1 encoding for .idl files Signed-off-by: Dirk Thomas <[email protected]> Signed-off-by: Siddharth Kucheria <[email protected]>
ros2 · Aug 2, 2019 · 4c3cd0c · 4c3cd0c
1 parent 9f72de1
commit 4c3cd0c
Show file tree

Hide file tree

Showing 37 changed files with 376 additions and 804 deletions.
diff --git a/rosidl_adapter/rosidl_adapter/action/__init__.py b/rosidl_adapter/rosidl_adapter/action/__init__.py
@@ -36,5 +36,5 @@ def convert_action_to_idl(package_dir, package_name, input_file, output_dir):
         'action': action,
     }
 
-    expand_template('action.idl.em', data, output_file)
+    expand_template('action.idl.em', data, output_file, encoding='iso-8859-1')
     return output_file
diff --git a/rosidl_adapter/rosidl_adapter/msg/__init__.py b/rosidl_adapter/rosidl_adapter/msg/__init__.py
@@ -36,7 +36,7 @@ def convert_msg_to_idl(package_dir, package_name, input_file, output_dir):
         'msg': msg,
     }
 
-    expand_template('msg.idl.em', data, output_file)
+    expand_template('msg.idl.em', data, output_file, encoding='iso-8859-1')
     return output_file
 
 

diff --git a/rosidl_adapter/rosidl_adapter/resource/__init__.py b/rosidl_adapter/rosidl_adapter/resource/__init__.py
@@ -19,17 +19,17 @@
 import em
 
 
-def expand_template(template_name, data, output_file):
+def expand_template(template_name, data, output_file, encoding='utf-8'):
     content = evaluate_template(template_name, data)
 
     if output_file.exists():
-        existing_content = output_file.read_text(encoding='utf-8')
+        existing_content = output_file.read_text(encoding=encoding)
         if existing_content == content:
             return
     elif output_file.parent:
         os.makedirs(str(output_file.parent), exist_ok=True)
 
-    output_file.write_text(content, encoding='utf-8')
+    output_file.write_text(content, encoding=encoding)
 
 
 _interpreter = None

diff --git a/rosidl_adapter/rosidl_adapter/srv/__init__.py b/rosidl_adapter/rosidl_adapter/srv/__init__.py
@@ -36,5 +36,5 @@ def convert_srv_to_idl(package_dir, package_name, input_file, output_dir):
         'srv': srv,
     }
 
-    expand_template('srv.idl.em', data, output_file)
+    expand_template('srv.idl.em', data, output_file, encoding='iso-8859-1')
     return output_file
diff --git a/rosidl_cmake/rosidl_cmake/__init__.py b/rosidl_cmake/rosidl_cmake/__init__.py
@@ -48,7 +48,10 @@ def get_newest_modification_time(target_dependencies):
     return newest_timestamp
 
 
-def generate_files(generator_arguments_file, mapping, additional_context=None, keep_case=False):
+def generate_files(
+    generator_arguments_file, mapping, additional_context=None,
+    keep_case=False, post_process_callback=None
+):
     args = read_generator_arguments(generator_arguments_file)
 
     template_basepath = pathlib.Path(args['template_dir'])
@@ -82,7 +85,8 @@ def generate_files(generator_arguments_file, mapping, additional_context=None, k
                 expand_template(
                     os.path.basename(template_file), data,
                     generated_file, minimum_timestamp=latest_target_timestamp,
-                    template_basepath=template_basepath)
+                    template_basepath=template_basepath,
+                    post_process_callback=post_process_callback)
         except Exception as e:
             print(
                 'Error processing idl file: ' +
@@ -110,7 +114,7 @@ def get_template_path(template_name):
 
 def expand_template(
     template_name, data, output_file, minimum_timestamp=None,
-    template_basepath=None
+    template_basepath=None, post_process_callback=None
 ):
     # in the legacy API the first argument was the path to the template
     if template_basepath is None:
@@ -155,12 +159,15 @@ def expand_template(
     content = output.getvalue()
     interpreter.shutdown()
 
+    if post_process_callback:
+        content = post_process_callback(content)
+
     # only overwrite file if necessary
     # which is either when the timestamp is too old or when the content is different
     if os.path.exists(output_file):
         timestamp = os.path.getmtime(output_file)
         if minimum_timestamp is None or timestamp > minimum_timestamp:
-            with open(output_file, 'r') as h:
+            with open(output_file, 'r', encoding='utf-8') as h:
                 if h.read() == content:
                     return
     else:
@@ -170,7 +177,7 @@ def expand_template(
         except FileExistsError:
             pass
 
-    with open(output_file, 'w') as h:
+    with open(output_file, 'w', encoding='utf-8') as h:
         h.write(content)
 
 

diff --git a/rosidl_generator_c/rosidl_generator_c/__init__.py b/rosidl_generator_c/rosidl_generator_c/__init__.py
@@ -34,7 +34,20 @@ def generate_c(generator_arguments_file):
         'idl__struct.h.em': '%s__struct.h',
         'idl__type_support.h.em': '%s__type_support.h',
     }
-    generate_files(generator_arguments_file, mapping)
+    generate_files(
+        generator_arguments_file, mapping,
+        post_process_callback=prefix_with_bom_if_necessary)
+
+
+def prefix_with_bom_if_necessary(content):
+    try:
+        content.encode('ASCII')
+    except UnicodeError:
+        prefix = '\ufeff' + \
+            '// NOLINT: This file starts with a BOM ' + \
+            'since it contain non-ASCII characters\n'
+        content = prefix + content
+    return content
 
 
 BASIC_IDL_TYPES_TO_C = {

diff --git a/rosidl_generator_cpp/CMakeLists.txt b/rosidl_generator_cpp/CMakeLists.txt
@@ -17,47 +17,9 @@ ament_python_install_package(${PROJECT_NAME})
 if(BUILD_TESTING)
   find_package(ament_lint_auto REQUIRED)
   find_package(rosidl_generator_c REQUIRED)
+  find_package(test_interface_files REQUIRED)
   ament_lint_auto_find_test_dependencies()
 
-  set(message_files
-    "msg/BoundedArrayBounded.msg"
-    "msg/BoundedArrayStatic.msg"
-    "msg/BoundedArrayUnbounded.msg"
-
-    "msg/Empty.msg"
-
-    "msg/FieldsWithSameTypeSomeDefaults.msg"
-
-    "msg/PrimitiveStaticArrays.msg"
-
-    "msg/PrimitivesBounded.msg"
-    "msg/PrimitivesConstants.msg"
-    "msg/PrimitivesDefault.msg"
-    "msg/PrimitivesStatic.msg"
-    "msg/PrimitivesUnbounded.msg"
-
-    "msg/StaticArrayBounded.msg"
-    "msg/StaticArrayStatic.msg"
-    "msg/StaticArrayUnbounded.msg"
-
-    "msg/String.msg"
-    "msg/StringBounded.msg"
-    "msg/StringArrayStatic.msg"
-    "msg/StringArrays.msg"
-
-    "msg/UnboundedArrayBounded.msg"
-    "msg/UnboundedArrayStatic.msg"
-    "msg/UnboundedArrayUnbounded.msg"
-
-    "msg/Various.msg"
-
-    "msg/WString.msg"
-  )
-
-  set(srv_files
-    "srv/ComplexTypeZeroFill.srv"
-  )
-
   include(cmake/register_cpp.cmake)
   set(rosidl_generator_cpp_DIR "${CMAKE_CURRENT_SOURCE_DIR}/cmake")
 
@@ -68,8 +30,8 @@ if(BUILD_TESTING)
   )
 
   rosidl_generate_interfaces(${PROJECT_NAME}
-    ${message_files}
-    ${srv_files}
+    ${test_interface_files_MSG_FILES}
+    ${test_interface_files_SRV_FILES}
     ADD_LINTER_TESTS
     SKIP_INSTALL
   )

diff --git a/rosidl_generator_cpp/msg/BoundedArrayBounded.msg b/rosidl_generator_cpp/msg/BoundedArrayBounded.msg
diff --git a/rosidl_generator_cpp/msg/BoundedArrayStatic.msg b/rosidl_generator_cpp/msg/BoundedArrayStatic.msg
diff --git a/rosidl_generator_cpp/msg/BoundedArrayUnbounded.msg b/rosidl_generator_cpp/msg/BoundedArrayUnbounded.msg
diff --git a/rosidl_generator_cpp/msg/Empty.msg b/rosidl_generator_cpp/msg/Empty.msg
diff --git a/rosidl_generator_cpp/msg/FieldsWithSameTypeSomeDefaults.msg b/rosidl_generator_cpp/msg/FieldsWithSameTypeSomeDefaults.msg
diff --git a/rosidl_generator_cpp/msg/PrimitiveStaticArrays.msg b/rosidl_generator_cpp/msg/PrimitiveStaticArrays.msg
diff --git a/rosidl_generator_cpp/msg/PrimitivesBounded.msg b/rosidl_generator_cpp/msg/PrimitivesBounded.msg
diff --git a/rosidl_generator_cpp/msg/PrimitivesConstants.msg b/rosidl_generator_cpp/msg/PrimitivesConstants.msg
diff --git a/rosidl_generator_cpp/msg/PrimitivesDefault.msg b/rosidl_generator_cpp/msg/PrimitivesDefault.msg
diff --git a/rosidl_generator_cpp/msg/PrimitivesStatic.msg b/rosidl_generator_cpp/msg/PrimitivesStatic.msg
diff --git a/rosidl_generator_cpp/msg/PrimitivesUnbounded.msg b/rosidl_generator_cpp/msg/PrimitivesUnbounded.msg
diff --git a/rosidl_generator_cpp/msg/StaticArrayBounded.msg b/rosidl_generator_cpp/msg/StaticArrayBounded.msg
diff --git a/rosidl_generator_cpp/msg/StaticArrayStatic.msg b/rosidl_generator_cpp/msg/StaticArrayStatic.msg
diff --git a/rosidl_generator_cpp/msg/StaticArrayUnbounded.msg b/rosidl_generator_cpp/msg/StaticArrayUnbounded.msg
diff --git a/rosidl_generator_cpp/msg/String.msg b/rosidl_generator_cpp/msg/String.msg
diff --git a/rosidl_generator_cpp/msg/StringArrayStatic.msg b/rosidl_generator_cpp/msg/StringArrayStatic.msg
diff --git a/rosidl_generator_cpp/msg/StringArrays.msg b/rosidl_generator_cpp/msg/StringArrays.msg
diff --git a/rosidl_generator_cpp/msg/StringBounded.msg b/rosidl_generator_cpp/msg/StringBounded.msg
diff --git a/rosidl_generator_cpp/msg/UnboundedArrayBounded.msg b/rosidl_generator_cpp/msg/UnboundedArrayBounded.msg
diff --git a/rosidl_generator_cpp/msg/UnboundedArrayStatic.msg b/rosidl_generator_cpp/msg/UnboundedArrayStatic.msg
diff --git a/rosidl_generator_cpp/msg/UnboundedArrayUnbounded.msg b/rosidl_generator_cpp/msg/UnboundedArrayUnbounded.msg
diff --git a/rosidl_generator_cpp/msg/Various.msg b/rosidl_generator_cpp/msg/Various.msg
diff --git a/rosidl_generator_cpp/msg/WString.msg b/rosidl_generator_cpp/msg/WString.msg
diff --git a/rosidl_generator_cpp/package.xml b/rosidl_generator_cpp/package.xml
@@ -22,6 +22,7 @@
   <test_depend>ament_lint_common</test_depend>
   <test_depend>rosidl_cmake</test_depend>
   <test_depend>rosidl_generator_c</test_depend>
+  <test_depend>test_interface_files</test_depend>
 
   <member_of_group>rosidl_generator_packages</member_of_group>
   <member_of_group>rosidl_runtime_packages</member_of_group>

diff --git a/rosidl_generator_cpp/rosidl_generator_cpp/__init__.py b/rosidl_generator_cpp/rosidl_generator_cpp/__init__.py
@@ -34,7 +34,20 @@ def generate_cpp(generator_arguments_file):
         'idl__struct.hpp.em': '%s__struct.hpp',
         'idl__traits.hpp.em': '%s__traits.hpp',
     }
-    generate_files(generator_arguments_file, mapping)
+    generate_files(
+        generator_arguments_file, mapping,
+        post_process_callback=prefix_with_bom_if_necessary)
+
+
+def prefix_with_bom_if_necessary(content):
+    try:
+        content.encode('ASCII')
+    except UnicodeError:
+        prefix = '\ufeff' + \
+            '// NOLINT: This file starts with a BOM ' + \
+            'since it contain non-ASCII characters\n'
+        content = prefix + content
+    return content
 
 
 MSG_TYPE_TO_CPP = {

diff --git a/rosidl_generator_cpp/srv/ComplexTypeZeroFill.srv b/rosidl_generator_cpp/srv/ComplexTypeZeroFill.srv