ros-infrastructure / catkin_pkg

Standalone Python library for the catkin build system.
https://github.com/ros/catkin
Other
47 stars 89 forks source link

Compare dependencies with all attributes #294

Closed f-fl0 closed 3 years ago

f-fl0 commented 4 years ago

Problem statement

Running catkin_make in a workspace with a package with dependencies like this:

<depend condition="$ROS_DISTRO == kinetic" version_eq="1.9.3-r0">diagnostic_updater</depend>
<depend condition="$ROS_DISTRO == noetic" version_eq="1.9.4-r0">diagnostic_updater</depend>

will result in the following error:

Error(s):
- The generic dependency on 'diagnostic_updater' is redundant with: build_depend, build_export_depend, exec_depend

Proposed solution

Comparing dependencies using their names and attributes (e.g. conditions) instead of just their names.

dirk-thomas commented 4 years ago

While considering more attributes than name is certainly the right thing to do I am concerned about including evaluated_condition (see https://github.com/ros-infrastructure/catkin_pkg/blob/e7a6806b370c9318ce8a792149c8906830ba22e6/src/catkin_pkg/package.py#L324) in this comparison. Both instances could have the same attributes and only that computed attribute could be different, I would think that those should still be considered duplicates. Maybe the right thing to do would be to exclude evaluated_condition from __eq__?

f-fl0 commented 3 years ago

While considering more attributes than name is certainly the right thing to do I am concerned about including evaluated_condition (see

https://github.com/ros-infrastructure/catkin_pkg/blob/e7a6806b370c9318ce8a792149c8906830ba22e6/src/catkin_pkg/package.py#L324 ) in this comparison. Both instances could have the same attributes and only that computed attribute could be different, I would think that those should still be considered duplicates. Maybe the right thing to do would be to exclude evaluated_condition from __eq__?

@dirk-thomas __eq__ now ignores evaluated_condition as you suggested. Let me know if you have other comments.

dirk-thomas commented 3 years ago

Thanks for the patch.