Bug fixes with new test data (MetroRobots#15)

* Update test zip hash * Exclude hidden files * Replace manifest with package_xml * Rework sync_package_xml_and_setup_py * Check for cmake existence * Update precommit config
edu-rossrobotics · Jun 1, 2024 · bd6d5a7 · bd6d5a7
1 parent 16020d8
commit bd6d5a7
Show file tree

Hide file tree

Showing 6 changed files with 34 additions and 24 deletions.
diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml
@@ -30,7 +30,7 @@ repos:
     - --format
     - parsable
     - --strict
-  rev: v1.33.0
+  rev: v1.35.1
 - repo: https://github.com/jumanjihouse/pre-commit-hook-yamlfmt
   hooks:
   - id: yamlfmt
@@ -50,10 +50,10 @@ repos:
 - repo: https://github.com/hhatto/autopep8
   hooks:
   - id: autopep8
-  rev: v2.0.4
+  rev: v2.1.0
 - repo: https://github.com/PyCQA/flake8
   hooks:
   - id: flake8
-  rev: 6.1.0
+  rev: 7.0.0
 ci:
   autoupdate_schedule: quarterly
diff --git a/src/ros_glint/glinters/cmake.py b/src/ros_glint/glinters/cmake.py
@@ -61,6 +61,8 @@ def remove_cmake_comments_helper(cmake, ignorables, replacement=''):
 @glinter
 def remove_empty_cmake_lines(package):
     cmake = package.cmake
+    if not cmake:
+        return
     for i, content in enumerate(cmake.contents[:-2]):
         if str(content)[-1] == '\n' and cmake.contents[i + 1] == '\n' and cmake.contents[i + 2] == '\n':
             cmake.contents[i + 1] = ''

diff --git a/src/ros_glint/glinters/cmake_installs.py b/src/ros_glint/glinters/cmake_installs.py
@@ -241,7 +241,7 @@ def update_cplusplus_installs(package):
 
 @glinter
 def export_cplusplus_libraries(package):
-    if package.ros_version == 1:
+    if package.ros_version == 1 or not package.cmake:
         return
     libraries = package.cmake.get_libraries()
     if not libraries:
@@ -308,8 +308,7 @@ def check_cmake_python_buildtype(package):
 
         install_cmake_dependencies(package, {acp})
 
-    if package.setup_py is None:
-        create_setup_py(package)
+    create_setup_py(package)
 
 
 @glinter

diff --git a/src/ros_glint/glinters/python_setup.py b/src/ros_glint/glinters/python_setup.py
@@ -1,5 +1,6 @@
 from stylish_cmake_parser import Command, SectionStyle
 from ros_introspect.package import MiscPackageFile
+from ros_introspect.components.package_xml import PEOPLE_TAGS
 from ros_introspect.components.setup_py import create_setup_py, contains_quoted_string, quote_string, unquote_string
 from ros_introspect.components.setup_cfg import SetupCFG
 from ros_introspect.components.source_code import SourceCode
@@ -73,23 +74,28 @@ def sync_package_xml_and_setup_py(package):
     if package.setup_py is None and package.build_type != 'ament_python':
         return
 
-    if package.setup_py is None:
-        create_setup_py(package)
+    create_setup_py(package)  # If needed
 
+    setup_py = package.setup_py
     for tag in ['version', 'description', 'license']:
-        tags = package.manifest.get_elements_by_tags([tag])
+        tags = package.package_xml.get_elements_by_tags([tag])
         if not tags:
             continue
-        value = tags[0].childNodes[0].nodeValue
-        package.setup_py.args[tag] = repr(value)
-
-    for maintainer in package.manifest.get_elements_by_tags(['maintainer']):
-        # TODO: Expand for author
-        # TODO: Joint multiple people?
-        package.setup_py.args['maintainer'] = repr(maintainer.childNodes[0].nodeValue)
-        package.setup_py.args['maintainer_email'] = repr(maintainer.getAttribute('email'))
-
-    package.setup_py.changed = True
+        setup_py.set_arg(tag, tags[0].childNodes[0].nodeValue)
+
+    for tag_name in PEOPLE_TAGS:
+        names = []
+        emails = []
+        for person in package.package_xml.get_people_by_tag(tag_name):
+            if person.name:
+                names.append(person.name)
+            if person.email:
+                emails.append(person.email)
+
+        if names:
+            setup_py.set_arg(tag_name, ', '.join(names))
+        if emails:
+            setup_py.set_arg(f'{tag_name}_email', ', '.join(emails))
 
 
 def has_python_library(package_name, py_src):

diff --git a/test/test_from_zip.py b/test/test_from_zip.py
@@ -13,7 +13,7 @@
 TEST_DATA = [
     # (branch, known_hash)
     ('ros1', '84bf97b92d2ceb8d4369ed0d6fd8a01fabaf3006034fb7710d448b8cfe9885d9'),
-    ('ros2', 'cf23d832e0cf56c8e160c0178af81e865c615fb330fb26604ddf031fc51a280a'),
+    ('ros2', '5cfbd9c39ee239674d5c9e79f759f72655f869f43a0d0562dcc187cba7cc8efa'),
 ]
 
 linters = get_linters()

diff --git a/test/zip_interface.py b/test/zip_interface.py
@@ -5,7 +5,7 @@
 import yaml
 import zipfile
 
-from ros_introspect.finder import walk
+from ros_introspect.finder import walk, is_hidden
 from ros_glint.util import set_executable
 
 
@@ -34,13 +34,16 @@ def __exit__(self, a_type, value, traceback):
         self.tempdir = None
 
     def get_filenames(self):
+        the_files = []
         if self.is_written:
-            the_files = []
             for subpath in walk(self.root):
                 the_files.append(subpath)
-            return set(the_files)
         else:
-            return set(self.pkg_files.keys())
+            for subpath in self.pkg_files.keys():
+                if is_hidden(subpath):
+                    continue
+                the_files.append(subpath)
+        return set(the_files)
 
     def get_contents(self, filename):
         if self.is_written: