From 1c05b70e2a687f5586bbd7b3a7859c11cdf6fd86 Mon Sep 17 00:00:00 2001 From: OttoMation-Movai Date: Thu, 20 Feb 2025 18:18:23 +0000 Subject: [PATCH 1/3] fix rosdep returning space separated list --- .../ros_install_build_deps/catkin_package.py | 50 ++++++++++--------- mobros/utils/utilitary.py | 21 +++++--- 2 files changed, 39 insertions(+), 32 deletions(-) diff --git a/mobros/commands/ros_install_build_deps/catkin_package.py b/mobros/commands/ros_install_build_deps/catkin_package.py index d6cfa45..9d51673 100644 --- a/mobros/commands/ros_install_build_deps/catkin_package.py +++ b/mobros/commands/ros_install_build_deps/catkin_package.py @@ -84,35 +84,37 @@ def _find_dependencies(self, dependency_type, dependency_object, xml_root, black if dependency_name in blacklist: continue - deb_name = utilitary.translate_package_name(dependency_name) + deb_names = utilitary.translate_package_name(dependency_name) - logging.debug( - "[Dependency_Manager - check_colisions] Dependency: " - + dependency_name - + " has been translated to " - + deb_name - ) + for deb_name in deb_names: - if deb_name not in dependency_object: - dependency_object[deb_name] = [] - - if child.attrib: - for key in child.attrib: - dependency_operator = key - dependency_version = child.attrib[key] + logging.debug( + "[Dependency_Manager - check_colisions] Dependency: " + + dependency_name + + " has been translated to " + + deb_name + ) + if deb_name not in dependency_object: + dependency_object[deb_name] = [] + + if child.attrib: + for key in child.attrib: + dependency_operator = key + dependency_version = child.attrib[key] + + dependency_object[deb_name].append( + { + "operator": dependency_operator, + "version": dependency_version, + "from": self.package_name, + } + ) + else: dependency_object[deb_name].append( { - "operator": dependency_operator, - "version": dependency_version, + "operator": "", + "version": None, "from": self.package_name, } ) - else: - dependency_object[deb_name].append( - { - "operator": "", - "version": None, - "from": self.package_name, - } - ) diff --git a/mobros/utils/utilitary.py b/mobros/utils/utilitary.py index e7806df..12fff6d 100644 --- a/mobros/utils/utilitary.py +++ b/mobros/utils/utilitary.py @@ -143,21 +143,26 @@ def translate_package_name(rosdep_key): rosdep_key (str): catkin package name Returns: - debian_pkg_name : debian package name + debian_pkg_name : list of debian package names """ output_lines = execute_shell_command( ["rosdep", "resolve", rosdep_key], stop_on_error=True, log_output=False ) + translation = [] + for line in output_lines: if ROSDEP_RESULT_HEADER not in line: - translation = line.strip() - logging.debug( - "[rosdep translate] Found translation for " - + rosdep_key - + ". It is " - + translation - ) + translation = line.strip().split(" ") + + if translation: + logging.debug( + "[rosdep translate] Found translation for " + + rosdep_key + + ". It is " + + str(translation) + ) + return translation def write_to_file(path_to_file, content): From 275214d3b1aa1e9b3ac3cc46525e2207a3c7f1bf Mon Sep 17 00:00:00 2001 From: Alex Fernandes Date: Thu, 20 Feb 2025 19:26:01 +0000 Subject: [PATCH 2/3] adapt test --- tests/test_executers/test_catkin_package_manager.py | 4 ++-- tests/test_executers/test_install_build_deps_executer.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/test_executers/test_catkin_package_manager.py b/tests/test_executers/test_catkin_package_manager.py index 01b426a..32f5fbd 100644 --- a/tests/test_executers/test_catkin_package_manager.py +++ b/tests/test_executers/test_catkin_package_manager.py @@ -4,8 +4,8 @@ from mobros.commands.ros_install_build_deps.catkin_package import CatkinPackage mock_rosdep_translate_map = { - "ompl": "ros-noetic-ompl", - "movai_navigation": "ros-noetic-movai-navigation", + "ompl": ["ros-noetic-ompl"], + "movai_navigation": ["ros-noetic-movai-navigation"], } diff --git a/tests/test_executers/test_install_build_deps_executer.py b/tests/test_executers/test_install_build_deps_executer.py index 0108582..86903f7 100644 --- a/tests/test_executers/test_install_build_deps_executer.py +++ b/tests/test_executers/test_install_build_deps_executer.py @@ -32,7 +32,7 @@ def mock_inspect_package(deb_name, version): return_value=None, ) @mock.patch( - "mobros.utils.utilitary.translate_package_name", return_value="ros-noetic-mobros" + "mobros.utils.utilitary.translate_package_name", return_value=["ros-noetic-mobros"] ) @mock.patch( "mobros.utils.apt_utils.execute_shell_command", From d0a09707f2550f64cb20721ea1f6935a4be03443 Mon Sep 17 00:00:00 2001 From: Alex Fernandes Date: Fri, 21 Feb 2025 09:35:06 +0000 Subject: [PATCH 3/3] fix sonar issues --- .pre-commit-config.yaml | 46 ++++++++++ .../ros_install_build_deps/catkin_package.py | 86 +++++++++++-------- 2 files changed, 98 insertions(+), 34 deletions(-) create mode 100644 .pre-commit-config.yaml diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml new file mode 100644 index 0000000..14a5501 --- /dev/null +++ b/.pre-commit-config.yaml @@ -0,0 +1,46 @@ +repos: + - repo: https://github.com/pre-commit/pre-commit-hooks + rev: v5.0.0 + hooks: + - id: check-docstring-first # checks if docstring is before code + - id: check-json # checks if json files are valid + - id: check-added-large-files # checks if large files were added + - id: check-merge-conflict # checks if all merge conflicts were resolved + - id: check-yaml # checks if yaml files are valid + - id: debug-statements # checks if debug statements are to be commited + - id: end-of-file-fixer # fixes missing line ending in end of file + - id: mixed-line-ending # fixes line files line ending + args: [--fix=lf] + - id: trailing-whitespace # removes trailing whitespaces from text files + exclude: .bumpversion.cfg # a tool edits this file and leaves trailing spaces, must be ignored + - repo: https://github.com/koalaman/shellcheck-precommit + rev: v0.10.0 + hooks: + - id: shellcheck + args: ["--severity=warning"] # Optionally only show errors and warnings + + - repo: https://github.com/ambv/black # formats Python code + rev: 25.1.0 + hooks: + - id: black + args: ["--line-length=120", "-S"] + + - repo: https://github.com/pycqa/flake8 # static tests Python code + rev: 7.1.2 + hooks: + - id: flake8 + args: ['--config', '.flake8'] + + # pylint does dynamic testing using data from imported modules + # thus the modules need to be found by it + # the easiest way to accomplish this is using it as a local hook + # see https://stackoverflow.com/questions/61238318/pylint-and-pre-commit-hook-unable-to-import + - repo: local + hooks: + - id: pylint + name: pylint + entry: pylint + language: python + types: [python] + require_serial: true + args: [ "-E"] diff --git a/mobros/commands/ros_install_build_deps/catkin_package.py b/mobros/commands/ros_install_build_deps/catkin_package.py index 9d51673..abfa075 100644 --- a/mobros/commands/ros_install_build_deps/catkin_package.py +++ b/mobros/commands/ros_install_build_deps/catkin_package.py @@ -1,5 +1,5 @@ -""" Module with the ability to interpret a catkin package.xml into a runtime object -""" +"""Module with the ability to interpret a catkin package.xml into a runtime object""" + import xml.etree.ElementTree as ET from os.path import isfile, join @@ -9,7 +9,7 @@ def is_catkin_blacklisted(path): - """ Function that checks if a given path contains a catkin blacklist file + """Function that checks if a given path contains a catkin blacklist file Args: path (os.path): OS path @@ -24,7 +24,7 @@ def is_catkin_blacklisted(path): class CatkinPackage: - """ Class that represents a catkin package and its dependencies""" + """Class that represents a catkin package and its dependencies""" def __init__(self, package_path, workspace_pkg_list=None): @@ -85,36 +85,54 @@ def _find_dependencies(self, dependency_type, dependency_object, xml_root, black continue deb_names = utilitary.translate_package_name(dependency_name) + self._process_deb_names(deb_names, dependency_object, child) - for deb_name in deb_names: + def _process_deb_names(self, deb_names, dependency_object, child): + """Helper function to process debian package names - logging.debug( - "[Dependency_Manager - check_colisions] Dependency: " - + dependency_name - + " has been translated to " - + deb_name - ) + Args: + deb_names (list): List of debian package names + dependency_object (dict): Dictionary to store dependencies + child (xml_obj): XML element representing the dependency + """ + for deb_name in deb_names: + logging.debug( + "[Dependency_Manager - check_colisions] Dependency: " + + child.text.strip() + + " has been translated to " + + deb_name + ) + + if deb_name not in dependency_object: + dependency_object[deb_name] = [] - if deb_name not in dependency_object: - dependency_object[deb_name] = [] - - if child.attrib: - for key in child.attrib: - dependency_operator = key - dependency_version = child.attrib[key] - - dependency_object[deb_name].append( - { - "operator": dependency_operator, - "version": dependency_version, - "from": self.package_name, - } - ) - else: - dependency_object[deb_name].append( - { - "operator": "", - "version": None, - "from": self.package_name, - } - ) + self._add_dependency(deb_name, dependency_object, child) + + def _add_dependency(self, deb_name, dependency_object, child): + """Helper function to add a dependency to the dependency object + + Args: + deb_name (str): Debian package name + dependency_object (dict): Dictionary to store dependencies + child (xml_obj): XML element representing the dependency + """ + if child.attrib: + for key in child.attrib: + dependency_operator = key + dependency_version = child.attrib[key] + + dependency_object[deb_name].append( + { + "operator": dependency_operator, + "version": dependency_version, + "from": self.package_name, + } + ) + else: + dependency_object[deb_name].append( + { + "operator": "", + "version": None, + "from": self.package_name, + } + )