search

ros-infrastructure / catkin_pkg

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

stop using distutils #139

Closed gerkey closed 8 years ago

wjwwood commented 8 years ago

Looks reasonable to me.

gerkey commented 8 years ago

I'm new in to the ros-infrastructure club. Anything that I should test or check before merging?

wjwwood commented 8 years ago

I'd just let @dirk-thomas have a look, since he maintains this one usually.

dirk-thomas commented 8 years ago

LGTM

Just out of curiosity: in which case was the relative path a problem?

wjwwood commented 8 years ago

I don't exactly know. I think it might have had to do with the weird thing we do where ament_tools will symlink the setup.py and some other stuff to the build folder to prevent build artifacts from getting into the source space. I'm not sure exactly under what conditions this becomes a problem, i.e. __file__ is relative.

dirk-thomas commented 8 years ago

If only the setup file is symlinked I don't see how abspath (without realpath) would make a difference in that case.

gerkey commented 8 years ago

I just tried the use case we're working on and, indeed, the use of abspath doesn't make any difference. It was something that we tried along the way and thought was having an effect, but it apparently doesn't. The key change for our use case is to not do a symlink install at all (ticket forthcoming on that issue).

I undid the abspath change and squashed. OK to merge?

dirk-thomas commented 8 years ago

+1