search

catkin / catkin_tools

Command line tools for working with catkin
Apache License 2.0
163 stars 146 forks source link

Check circular dependencies on catkin build #680

Closed furushchev closed 3 years ago

furushchev commented 3 years ago

catkin_tools reports unfriendly error when the workspace has packages that have circular dependencies like below:

$ catkin build
Traceback (most recent call last):
  File "/usr/bin/catkin", line 11, in <module>
    load_entry_point('catkin-tools==0.5.0', 'console_scripts', 'catkin')()
  File "/usr/lib/python3/dist-packages/catkin_tools/commands/catkin.py", line 272, in main
    catkin_main(sysargs)
  File "/usr/lib/python3/dist-packages/catkin_tools/commands/catkin.py", line 267, in catkin_main
    sys.exit(args.main(args) or 0)
  File "/usr/lib/python3/dist-packages/catkin_tools/verbs/catkin_build/cli.py", line 404, in main
    return build_isolated_workspace(
  File "/usr/lib/python3/dist-packages/catkin_tools/verbs/catkin_build/build.py", line 318, in build_isolated_workspace
    packages_to_be_built, packages_to_be_built_deps, all_packages = determine_packages_to_be_built(
  File "/usr/lib/python3/dist-packages/catkin_tools/verbs/catkin_build/build.py", line 94, in determine_packages_to_be_built
    workspace_package_names = dict([(pkg.name, (path, pkg)) for path, pkg in ordered_packages])
  File "/usr/lib/python3/dist-packages/catkin_tools/verbs/catkin_build/build.py", line 94, in <listcomp>
    workspace_package_names = dict([(pkg.name, (path, pkg)) for path, pkg in ordered_packages])
AttributeError: 'str' object has no attribute 'name'

With this pull request, users will get package names to be resolved like below:

$ catkin build
[build] There is a circular dependencies for '<package name1>', '<package name2>', ... in the workspace which need to be resolved
timonegk commented 3 years ago

There is already a check for circular dependencies just five lines below your changes. When I set up a workspace with a circular dependency, it gets correctly detected. Which version of catkin_pkg do you have installed?

timonegk commented 3 years ago

I am closing this because I cannot reproduce the issue and don't see an advantage of your solution compared to the current solution. If you disagree, feel free to comment or reopen the pull request.