Install compiled libraries only to 'lib' #195
Conversation
|
There was a problem hiding this comment.
So I'm not at all sure about this, but while looking this over I noticed something. That is the fact that if we remove the lines you are proposing to remove, then there is no reason to pass _build_type into these two macros anymore.
In turn, that leads me to look at where the macros are being called. They are called once with an "empty" (I assume "Release") build type for all platforms. And then, only on WIN32, they are called once for each build type. I don't exactly know what that means.
Looking at the history of this code, it was added in 2016 (via 1d7b4eb before we used PRs) and 2018 (via #10). So this code has been there quite a while.
I think we need to do some more extensive testing on this besides the normal CI. In particular, we should also test this out with Windows Debug to see what happens. If we can get it to work there, then I'm fine with the change, but we should then also remove the build_type argument from both macros (and maybe get rid of the macros completely).
|
Bump. Any updates or plan for this? |
Yep:
By all means, if you're interested in seeing this merged, please test it and provide feedback. |
The current behavior results in duplicate libraries installed to both the Python module in libdir and also into the global lib directory for the prefix. The former seems unnecessary, so we should install the libraries only to lib. Signed-off-by: Scott K Logan <logans@cottsay.net>
Signed-off-by: Chris Lalancette <clalancette@gmail.com>
clalancette
force-pushed
the
cottsay/install-libs-to-lib-only
branch
from
277ef2e
to
c5f3de2
Compare
|
I've updated this to the latest, and also removed what I think are now unnecessary macros. Here's a large set of CI to see what happens:
|
|
All right, the extensive CI is actually really happy with this. So I'm going to go ahead and merge this in, since it seems to be an improvement, and doesn't seem to have downsides. After a few days in source, I'll likely also do a release of this just to be sure the buildfarm is happy with it as well. |
ghost
mentioned this pull request
There seems that some regression might have happened after #195. When removing those 2 lines, we avoid to install the .so files in lib *and* python path. Signed-off-by: Matthias Schoepfer <m.schoepfer@rethinkrobotics.com>
The current behavior results in duplicate libraries installed to both the Python module in libdir and also into the global lib directory for the prefix.
The former seems unnecessary, so we should install the libraries only to lib.