fixes overlaid develspace include dir order bug involving symlink

ros-infrastructure / catkin_pkg

Standalone Python library for the catkin build system.

https://github.com/ros/catkin

Other

47 stars 91 forks source link

fixes overlaid develspace include dir order bug involving symlink #260

Closed aurzenligl closed 5 years ago

aurzenligl commented 5 years ago

This bug occurs when setup.bash of overlaid workspace is sourced through symlink path rather than absolute path. This commit resolves symlinks in order_path algorithm, allowing to detect that: ws1/devel/include starts with ws1/devel_symlink if ws1/devel_symlink is a symlink to ws1/devel.

original PR with problem description: https://github.com/ros/catkin/pull/1017

original error reproduction: https://github.com/aurzenligl/study/tree/master/ros-includedirorder

aurzenligl commented 5 years ago

@dirk-thomas, do you prefer review fixes visible in history as separate commits - or just as amends into original commits?

dirk-thomas commented 5 years ago

LGTM. Thanks for addressing this and iterating on the patch.

do you prefer review fixes visible in history as separate commits - or just as amends into original commits?

In general it is easier to review incremental commits since it is quicker to check what is changed in each iteration without re-reviewing the full patch. Luckily this one is fairly small - so no problem.