Skip to content

fix: reject deprecated node-name frontend key (backport #538)#545

Open
mergify[bot] wants to merge 1 commit into
lyricalfrom
mergify/bp/lyrical/pr-538
Open

fix: reject deprecated node-name frontend key (backport #538)#545
mergify[bot] wants to merge 1 commit into
lyricalfrom
mergify/bp/lyrical/pr-538

Conversation

@mergify
Copy link
Copy Markdown

@mergify mergify Bot commented May 10, 2026

Description

Stop accepting the deprecated node-name frontend key in Node.parse() so YAML and XML launch files report it as an unexpected key/attribute, matching the existing name API.

Fixes #537

Is this user-facing behavior change?

Yes. Launch files that still use the deprecated node-name frontend key/attribute will now fail frontend parsing as an unexpected key/attribute. Users should use the existing name key/attribute instead.

Did you use Generative AI?

Yes. I used OpenAI Codex (GPT-5) to help prepare this PR, including drafting the PR description, checking the diff, and addressing reviewer feedback. The code change itself is limited to removing the deprecated node-name parsing path; the initially added regression tests were removed after review feedback.

Additional Information

Before
$ git show upstream/rolling:launch_ros/launch_ros/actions/node.py | sed -n "300,320p"
        return normalized_params

    @classmethod
    def parse(cls, entity: Entity, parser: Parser):
        """Parse node."""
        # See parse method of `ExecuteProcess`
        _, kwargs = super().parse(entity, parser, ignore=['cmd'])
        args = entity.get_attr('args', optional=True)
        if args is not None:
            kwargs['arguments'] = super()._parse_cmdline(args, parser)
        ros_args = entity.get_attr('ros_args', optional=True)
        if ros_args is not None:
            kwargs['ros_arguments'] = super()._parse_cmdline(ros_args, parser)
        node_name = entity.get_attr('node-name', optional=True)
        if node_name is not None:
            kwargs['node_name'] = parser.parse_substitution(node_name)
        node_name = entity.get_attr('name', optional=True)
        if node_name is not None:
            kwargs['name'] = parser.parse_substitution(node_name)
        exec_name = entity.get_attr('exec_name', optional=True)
        if exec_name is not None:

$ git show upstream/rolling:test_launch_ros/test/test_launch_ros/frontend/test_node_frontend.py | rg -n "node-name|rejects_node_name" || true
After
$ git diff --stat upstream/rolling...HEAD
 launch_ros/launch_ros/actions/node.py | 3 ---
 1 file changed, 3 deletions(-)

$ git diff upstream/rolling...HEAD -- launch_ros/launch_ros/actions/node.py
@@ -310,9 +310,6 @@ class Node(ExecuteProcess):
         ros_args = entity.get_attr('ros_args', optional=True)
         if ros_args is not None:
             kwargs['ros_arguments'] = super()._parse_cmdline(ros_args, parser)
-        node_name = entity.get_attr('node-name', optional=True)
-        if node_name is not None:
-            kwargs['node_name'] = parser.parse_substitution(node_name)
         node_name = entity.get_attr('name', optional=True)
         if node_name is not None:
             kwargs['name'] = parser.parse_substitution(node_name)

$ git diff --check

$ python3 -m py_compile launch_ros/launch_ros/actions/node.py test_launch_ros/test/test_launch_ros/frontend/test_node_frontend.py
$WORKSPACE/python310_torch25_cuda/lib/python3.10/site-packages/xpytorch_import_hook.py:6: UserWarning: pkg_resources is deprecated as an API. See https://setuptools.pypa.io/en/latest/pkg_resources.html. The pkg_resources package is slated for removal as early as 2025-11-30. Refrain from using this package or pin to Setuptools<81.
  import pkg_resources

$ ruff check launch_ros/launch_ros/actions/node.py test_launch_ros/test/test_launch_ros/frontend/test_node_frontend.py
All checks passed!

$ $WORKSPACE/ros2-contrib/envs/launch-py310/bin/python -m pytest -q test_launch_ros/test/test_launch_ros/frontend/test_node_frontend.py::test_launch_frontend_yaml test_launch_ros/test/test_launch_ros/frontend/test_node_frontend.py::test_launch_frontend_xml
ERROR: found no collectors for $WORKSPACE/ros2-contrib/repos/launch_ros/test_launch_ros/test/test_launch_ros/frontend/test_node_frontend.py::test_launch_frontend_yaml

ERROR: found no collectors for $WORKSPACE/ros2-contrib/repos/launch_ros/test_launch_ros/test/test_launch_ros/frontend/test_node_frontend.py::test_launch_frontend_xml


==================================== ERRORS ====================================
_____ ERROR collecting test/test_launch_ros/frontend/test_node_frontend.py _____
ImportError while importing test module '$WORKSPACE/ros2-contrib/repos/launch_ros/test_launch_ros/test/test_launch_ros/frontend/test_node_frontend.py'.
Hint: make sure your test modules/packages have valid Python names.
Traceback:
../../../python310_torch25_cuda/lib/python3.10/importlib/__init__.py:126: in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
test_launch_ros/test/test_launch_ros/frontend/test_node_frontend.py:21: in <module>
    from launch import LaunchService
../../worktrees/launch-968/launch/launch/__init__.py:17: in <module>
    from . import actions
../../worktrees/launch-968/launch/launch/actions/__init__.py:17: in <module>
    from .declare_boolean_launch_argument import DeclareBooleanLaunchArgument
../../worktrees/launch-968/launch/launch/actions/declare_boolean_launch_argument.py:25: in <module>
    from .declare_launch_argument import DeclareLaunchArgument
../../worktrees/launch-968/launch/launch/actions/declare_launch_argument.py:28: in <module>
    from ..frontend import Entity
../../worktrees/launch-968/launch/launch/frontend/__init__.py:20: in <module>
    from .parser import InvalidFrontendLaunchFileError, Parser
../../worktrees/launch-968/launch/launch/frontend/parser.py:36: in <module>
    from .parse_substitution import parse_if_substitutions
../../worktrees/launch-968/launch/launch/frontend/parse_substitution.py:21: in <module>
    from ament_index_python.packages import get_package_share_directory
E   ModuleNotFoundError: No module named 'ament_index_python'
=============================== warnings summary ===============================
../../envs/launch-py310/lib/python3.10/site-packages/_pytest/config/__init__.py:1434
  $WORKSPACE/ros2-contrib/envs/launch-py310/lib/python3.10/site-packages/_pytest/config/__init__.py:1434: PytestConfigWarning: Unknown config option: timeout
  
    self._warn_or_fail_if_strict(f"Unknown config option: {key}\n")

../../envs/launch-py310/lib/python3.10/site-packages/_pytest/config/__init__.py:1434
  $WORKSPACE/ros2-contrib/envs/launch-py310/lib/python3.10/site-packages/_pytest/config/__init__.py:1434: PytestConfigWarning: Unknown config option: timeout_method
  
    self._warn_or_fail_if_strict(f"Unknown config option: {key}\n")

-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html
=========================== short test summary info ============================
ERROR test_launch_ros/test/test_launch_ros/frontend/test_node_frontend.py
2 warnings, 1 error in 0.07s

Test plan

  • git diff --check
  • python3 -m py_compile launch_ros/launch_ros/actions/node.py test_launch_ros/test/test_launch_ros/frontend/test_node_frontend.py
  • ruff check launch_ros/launch_ros/actions/node.py test_launch_ros/test/test_launch_ros/frontend/test_node_frontend.py
  • Target frontend pytest cannot complete in this local environment because ROS Python dependency ament_index_python is not installed
    This is an automatic backport of pull request fix: reject deprecated node-name frontend key #538 done by Mergify.

Signed-off-by: Lidang Jiang <lidangjiang@gmail.com>
(cherry picked from commit 5a8cbb2)
@christophebedard
Copy link
Copy Markdown
Member

Pulls: #545
Gist: https://gist.githubusercontent.com/christophebedard/47bfc781f598b31112e2439f017e953d/raw/a30916bf62af20486d3224e87cbddb30c7733fa5/ros2.repos
BUILD args: --packages-above-and-dependencies launch_ros
TEST args: --packages-above launch_ros
ROS Distro: lyrical
Job: ci_launcher
ci_launcher ran: https://ci.ros2.org/job/ci_launcher/19207

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Linux-rhel Build Status
  • Windows Build Status

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants