[Bug 674008] Review Request: openrave - Open Robotics Automation Virtual Environment

[bugzilla at redhat.com]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.


https://bugzilla.redhat.com/show_bug.cgi?id=674008

--- Comment #71 from Tim Niemueller <tim at niemueller.de> 2011-12-13 09:01:00 EST ---
(In reply to comment #69)
> hi tim,

Hi Rosen.

> sorry for the confusion, the latest_stable are the release candidates, and are
> in constant flux. the last official release is 0.4.2. should i write something
> on the webpage to avoid this confusion?

Yes, I'm an example of confusion about this ;-)

> Your patches look good except:
> 
> 1. why did you remove the relpath being called? this is necessary on ubuntu
> systems.
> 
> COMMAND ${PYTHON_EXECUTABLE} -c "from distutils.sysconfig import
> get_python_lib; from os.path import relpath; print
> relpath(get_python_lib(1,prefix='${CMAKE_INSTALL_PREFIX}'),'${CMAKE_INSTALL_PREFIX}')"

It didn't work for me this way on Fedora. I'll give it another try, I just
copied that from the older patches and did not re-evaluate.

> 2. openrave.pc is not supposed to link with libopenrave-core.so. Plugin authors
> only need to link with libopenrave.so. I think what you are looking for is
> openrave-core.pc, do you want to output it?

Or the other way around: have openrave-plugin.pc with the basics, and
openrave.pc for anyone who wants to link against OpenRAVE for embedding? Either
way is fine and would be just what I need.

> 3. i would like to rename OPENRAVE_INCLUDE_DIRNAME to OPENRAVE_INCLUDE_DIR.

Sure, any kind of renames are fine. Just let me know so I can adapt the spec
file, please.

> 4. config.h.in, you removed CMAKE_INSTALL_PREFIX. what were your reasons?
> unfortunately this is needed or openrave will not be able to find the default
> plugin/data directories.

That is because I pass in absolute paths, which is required since on Fedora it
must go to /usr/lib or /usr/lib64 based on the architecture. I'd say let's make
the paths in CMakelists.txt absolute paths, so that they can be overridden
nicely by cmake parameters. The fewer places where we compose/combine paths the
better for overriding.
I didn't think enough about this so that I did actually change the default
which I did not intent.

> 5. as for the boost changes, i think we had those fixed on the recent trunk.
> from a quick glance, it looks like the current openrave trunk should compile
> fine for you..

It does on Fedora, it does not on RHEL with an older Boost version (or on
robots which simply have an older Fedora version and are a pain to upgrade
right now).

-- 
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.