Skip to content

fix(substitution_args.py): _resolve_args() not finding commands#380

Merged
rhaschke merged 4 commits intoros:ros2from
Amronos:fix-resolve-args
Mar 17, 2026
Merged

fix(substitution_args.py): _resolve_args() not finding commands#380
rhaschke merged 4 commits intoros:ros2from
Amronos:fix-resolve-args

Conversation

@Amronos
Copy link
Copy Markdown
Contributor

@Amronos Amronos commented Mar 16, 2026

The below wasn't working for me; replacing the custom split logic in _resolve_args() with shlex seems to do the trick.

  <xacro:include filename="$(find some_package)/description/some.urdf.xacro" />

Error log:

Captured stderr output: error: <class 'xacro.substitution_args.SubstitutionException'>: Unknown substitution command [find some_package]. Valid commands are ['find', 'env', 'optenv', 'dirname', 'arg']

Copy link
Copy Markdown
Contributor

@rhaschke rhaschke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I cannot reproduce your issue. We do have a succeeding unit test for this:

xacro/test/test_xacro.py

Lines 570 to 573 in 1f24172

def test_substitution_args_find(self):
resolved = self.quick_xacro('''<a>$(find xacro)/test/test_xacro.py</a>''').firstChild.firstChild.data
self.assertEqual(os.path.realpath(resolved), os.path.realpath(__file__))

Could you please clarify under which circumstances you get the error?

Your solution introduces another dependency, which I would like to avoid.

@Amronos
Copy link
Copy Markdown
Contributor Author

Amronos commented Mar 16, 2026

I personally encountered it here.

The reason, though, may not be related to my code but rather to the fact that I am using Nix/NixOS with nix-ros-overlay. Perhaps something changed between Python versions?
I would therefore like to add @wentasah (the current maintainer of nix-ros-overlay) to the thread.

Also, shlex isn't an external dependency; it is part of the Python standard library.

@wentasah
Copy link
Copy Markdown

I've just run xacro tests under nix-ros-overlay and all of them seem to pass:

colcon test --event-handlers console_direct+
Starting >>> xacro   
UpdateCTestConfiguration  from :/home/wsh/src/github.com/ros/xacro/build/xacro/CTestConfiguration.ini
Parse Config file:/home/wsh/src/github.com/ros/xacro/build/xacro/CTestConfiguration.ini
   Site: steelpick
   Build name: (empty)
 Add coverage exclude regular expressions.
Create new tag: 20260316-1259 - Experimental
UpdateCTestConfiguration  from :/home/wsh/src/github.com/ros/xacro/build/xacro/CTestConfiguration.ini
Parse Config file:/home/wsh/src/github.com/ros/xacro/build/xacro/CTestConfiguration.ini
Test project /home/wsh/src/github.com/ros/xacro/build/xacro
Constructing a list of tests
Done constructing a list of tests
Updating test list for fixtures
Added 0 tests to meet fixture requirements
Checking test dependency graph...
Checking test dependency graph end
test 1
    Start 1: pytest

1: Test command: /nix/store/j0cxym3cn4nfqqvaw3zbx8mvyx5zgh4i-ros-env/bin/python3 "-u" "/nix/store/j0cxym3cn4nfqqvaw3zbx8mvyx5zgh4i-ros-env/share/ament_cmake_test/cmake/run_test.py" "/home/wsh/src/github.com/ros/xacro/build/xacro/test_results/xacro/pytest.xunit.xml" "--package-name" "xacro" "--output-file" "/home/wsh/src/github.com/ros/xacro/build/xacro/ament_cmake_pytest/pytest.txt" "--env" "PYTHONDONTWRITEBYTECODE=1" "AMENT_PREFIX_PATH=/home/wsh/src/github.com/ros/xacro/build/xacro/test/test_ament_index:/nix/store/j0cxym3cn4nfqqvaw3zbx8mvyx5zgh4i-ros-env" "--command" "/nix/store/j0cxym3cn4nfqqvaw3zbx8mvyx5zgh4i-ros-env/bin/python3" "-u" "-m" "pytest" "/home/wsh/src/github.com/ros/xacro/test/." "-o" "cache_dir=/home/wsh/src/github.com/ros/xacro/build/xacro/test/ament_cmake_pytest/pytest/.cache" "--junit-xml=/home/wsh/src/github.com/ros/xacro/build/xacro/test_results/xacro/pytest.xunit.xml" "--junit-prefix=xacro"
1: Working Directory: /home/wsh/src/github.com/ros/xacro/test
1: Test timeout computed to be: 60
1: -- run_test.py: extra environment variables:
1:  - AMENT_PREFIX_PATH=/home/wsh/src/github.com/ros/xacro/build/xacro/test/test_ament_index:/nix/store/j0cxym3cn4nfqqvaw3zbx8mvyx5zgh4i-ros-env
1:  - PYTHONDONTWRITEBYTECODE=1
1: -- run_test.py: invoking following command in '/home/wsh/src/github.com/ros/xacro/test':
1:  - /nix/store/j0cxym3cn4nfqqvaw3zbx8mvyx5zgh4i-ros-env/bin/python3 -u -m pytest /home/wsh/src/github.com/ros/xacro/test/. -o cache_dir=/home/wsh/src/github.com/ros/xacro/build/xacro/test/ament_cmake_pytest/pytest/.cache --junit-xml=/home/wsh/src/github.com/ros/xacro/build/xacro/test_results/xacro/pytest.xunit.xml --junit-prefix=xacro
1: ============================= test session starts ==============================
1: platform linux -- Python 3.13.7, pytest-8.4.1, pluggy-1.6.0
1: cachedir: build/xacro/test/ament_cmake_pytest/pytest/.cache
1: rootdir: /home/wsh/src/github.com/ros/xacro
1: configfile: pyproject.toml
1: plugins: colcon-core-0.20.1, cov-6.2.1, repeat-0.9.4, rerunfailures-15.1, launch_testing-3.9.6, launch_testing_ros-0.29.6, ament_copyright-0.20.3, ament_flake8-0.20.3, ament_lint-0.20.3, ament_mypy-0.20.3, ament_pep257-0.20.3, ament_xmllint-0.20.3, typeguard-4.4.4
1: collected 145 items
1: 
1: test_xacro.py .......................................................... [ 40%]
1: ........................................................................ [ 89%]
1: ...............                                                          [100%]
1: 
1: - generated xml file: /home/wsh/src/github.com/ros/xacro/build/xacro/test_results/xacro/pytest.xunit.xml -
1: ============================= 145 passed in 1.40s ==============================
1: -- run_test.py: return code 0    
1: -- run_test.py: verify result file '/home/wsh/src/github.com/ros/xacro/build/xacro/test_results/xacro/pytest.xunit.xml'
1/2 Test #1: pytest ...........................   Passed    2.13 sec
test 2                              
    Start 2: xacro_cmake

2: Test command: /home/wsh/src/github.com/ros/xacro/test/test-cmake.sh "/home/wsh/src/github.com/ros/xacro/test/test-xacro-cmake"
2: Working Directory: /home/wsh/src/github.com/ros/xacro/build/xacro/test
2: Test timeout computed to be: 10000000
2/2 Test #2: xacro_cmake ......................   Passed    3.22 sec

100% tests passed, 0 tests failed out of 2

Label Time Summary:
pytest    =   2.13 sec*proc (1 test)

Total Test time (real) =   5.45 sec
Finished <<< xacro [5.49s]

I checked that test_substitution_args_find test was executed in test_substitution_args_find and it was. Not sure, where is the problem.

@rhaschke
Copy link
Copy Markdown
Contributor

@Amronos: could you create a minimal example, please.

@Amronos
Copy link
Copy Markdown
Contributor Author

Amronos commented Mar 17, 2026

I wasn't able to create a minimal example, but I did find the below log upon appending --debug to my launch command:

<class 'xacro.substitution_args.SubstitutionException'>: Unknown substitution command [find\n      variobot_control]. Valid commands are ['find', 'env', 'optenv', 'dirname', 'arg'] Unknown substitution command [find\n      variobot_control]. Valid commands are ['find', 'env', 'optenv', 'dirname', 'arg']

Looks like a \n is inserted after the find command. This problem was simply solved by adding a .strip() after splits[0].

Should I make this change or should shlex be kept instead?

Sorry for needlessly pinging you @wentasah!

@rhaschke
Copy link
Copy Markdown
Contributor

I prefer to have the strip(). The question remains, where the newline comes from. split() should actually split on any whitespace - including newlines.

@Amronos Amronos requested a review from rhaschke March 17, 2026 15:09
@@ -333,10 +334,10 @@ def _resolve_args(arg_str, context, commands):
resolved = arg_str
for a in _collect_args(arg_str):
splits = [s for s in a.split(' ') if s]
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you please check wheather splitting on any whitespace already resolves the problem too:

Suggested change
splits = [s for s in a.split(' ') if s]
splits = [s for s in a.split() if s]

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! That works. Fixed in d088535.

@Amronos Amronos requested a review from rhaschke March 17, 2026 16:04
@rhaschke rhaschke merged commit 38726b3 into ros:ros2 Mar 17, 2026
10 checks passed
@rhaschke
Copy link
Copy Markdown
Contributor

Thanks for reporting and iterating on this.

@Amronos Amronos deleted the fix-resolve-args branch March 17, 2026 20:18
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.

3 participants