Bug 674008 - Review Request: openrave - Open Robotics Automation Virtual Environment
Summary: Review Request: openrave - Open Robotics Automation Virtual Environment
Keywords:
Status: CLOSED DUPLICATE of bug 1338339
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Nobody's working on this, feel free to take it
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On: 672440 676159 689432 689639 814458 1069257
Blocks: 1225692
TreeView+ depends on / blocked
 
Reported: 2011-01-31 10:18 UTC by Tim Niemueller
Modified: 2016-05-21 19:34 UTC (History)
6 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2016-05-21 19:34:49 UTC
Type: ---
Embargoed:


Attachments (Terms of Use)
Patch for upstreaming by Rosen (76.46 KB, patch)
2011-03-21 23:58 UTC, Tim Niemueller
no flags Details | Diff
Patch for upstreaming by Rosen (18.68 KB, patch)
2011-07-23 06:14 UTC, Tim Niemueller
no flags Details | Diff
Patch to 0.5.0 for upstreaming by Rosen (19.03 KB, patch)
2011-12-13 12:36 UTC, Tim Niemueller
no flags Details | Diff

Description Tim Niemueller 2011-01-31 10:18:18 UTC
Spec URL: http://fedorapeople.org/~timn/robotics/openrave.spec
SRPM URL: http://fedorapeople.org/~timn/robotics/openrave-0.2.18-0.1.svn1975.fc14.src.rpm
Description: OpenRAVE is targeted for real-world autonomous robot applications, and includes a seamless integration of 3-D simulation, visualization, planning,
scripting and control.

Koji: https://koji.fedoraproject.org/koji/taskinfo?taskID=2748421

rpmlint:
# rpmlint openrave.spec 
openrave.spec: W: invalid-url Source0: openrave-0.2.18-svn1975.tar.gz
0 packages and 1 specfiles checked; 0 errors, 1 warnings.
- Warning is ok, source must be built from svn

# rpmlint openrave-0.2.18-0.1.svn1975.fc14.x86_64.rpm openrave-devel-0.2.18-0.1.svn1975.fc14.x86_64.rpm openrave-python-0.2.18-0.1.svn1975.fc14.x86_64.rpm openrave-octave-0.2.18-0.1.svn1975.fc14.x86_64.rpm
openrave.x86_64: W: shared-lib-calls-exit /usr/lib64/libopenrave-core.so.0.2.18 exit.5
- Will contact upstream about this, it is in once place and it is of the kind "should never happen".

openrave.x86_64: W: non-conffile-in-etc /etc/bash_completion.d/openrave.bash
- intended

openrave.x86_64: W: wrong-file-end-of-line-encoding /usr/share/doc/openrave-0.2.18/AUTHORS
- minor

openrave.x86_64: W: no-manual-page-for-binary openrave
openrave-devel.x86_64: W: no-manual-page-for-binary openrave-config
openrave-python.x86_64: W: no-documentation
openrave-python.x86_64: W: no-manual-page-for-binary openrave.py
openrave-python.x86_64: W: no-manual-page-for-binary openrave-hash.py
openrave-octave.x86_64: W: no-documentation
- do not exist
openrave-octave.x86_64: W: unstripped-binary-or-object /usr/libexec/octave/packages/openrave-0.2.18/orcreate.mex
openrave-octave.x86_64: W: unstripped-binary-or-object /usr/libexec/octave/packages/openrave-0.2.18/orwrite.mex
openrave-octave.x86_64: W: unstripped-binary-or-object /usr/libexec/octave/packages/openrave-0.2.18/orread.mex
- Octave thing as it seems
4 packages and 0 specfiles checked; 0 errors, 12 warnings.

Comment 1 Rich Mattes 2011-01-31 15:26:33 UTC
It looks like this package is currently using 3 bundled libraries: flann, convexdecomposition, and collada.  Flann is up for review right now in bug 672440.  The collada stuff appears to be "collada-dom" on sourceforge.  And I'm having a hard time tracking down where the convexdecomposition library came from.  It's possible to get around shipping bundled libraries under the right circumstances by filing an exception with FESCo:
http://fedoraproject.org/wiki/Packaging:No_Bundled_Libraries

The end of line encoding is easy to fix with sed or dos2unix.

Comment 2 Tim Niemueller 2011-01-31 16:35:21 UTC
Thanks for the useful comments! I'll use the flann package.

New SRPM: http://fedorapeople.org/~timn/robotics/openrave-0.2.18-0.2.svn1975.fc14.src.rpm
Spec file is changed in-place. Patch has been updated to include a flann system-wide installation check.

The collada-dom in the OpenRAVE tree is only a sub-set of collada-dom, and the CMakeLists.txt explicitly states problems with system-wide installed versions. It only uses the static library, so no shared object is installed that could confuse other apps. I'll look into it for separate packaging.

The convexdecomposition library seems to be the one from https://code.google.com/p/convexdecomposition. I'll look into it if it can be packaged separately. Again, it is linked statically into OpenRAVE and should therefore not cause confusion. OpenRAVE includes a Python wrapper.

Comment 3 Peter Lemenkov 2011-02-02 21:02:30 UTC
I'll review it

Comment 4 Tim Niemueller 2011-02-02 21:30:22 UTC
I have further looked into it and collada-dom is a bloody mess. The package was started for the PS3 and it looks that way. Also, the package does not build out of the box. It starts with wrong file names, obviously intended for a system which is case-insensitive, and goes on with compiler errors. I'd like to use the collada part (it's about a quarter of the collada-dom package) that is in the sub-tree. It is integrated into the OpenRAVE build system and statically linked.

The convexdecomposition library is intended to be integrated into a project and does not come with an own build system config files. Therefore, again, I'd like to keep it the way it is atm. It hasn't been changed since 2009, so the chance is better to have a maintained version in OpenRAVE than to start a new upstream integrating patches that originate from OpenRAVE.

Therefore I'd like to propose the current spec state as the package.

Comment 5 Rosen Diankov 2011-02-03 04:11:15 UTC
hi guys,

Thanks for taking the time to create an official RPM. I'll be as helpful as possible.

A couple of notes about the 3rd party libraries:

1. collada-dom has local changes, so it cannot be replaced with the official one. I have recently become the maintainer of the collada-dom package, so in the future we'll be able to create an official collada-dom rpm.

2. convexdecomposition also has local changes. I've emailed the author about them, but he seems to be busy with other things.

3. You'll notice the openrave includes both ANN and flann. ANN will be phased out. flann is mostly unmodified except some of the CMakeLists.txt files in order to get openrave plugins to link to it correctly.

4. fparser-4.3 is from this project

http://warp.povusers.org/FunctionParser/

Again openrave has many local changes, I've emailed the author about them, but he is not very responsive.

5. crlibm-1.0beta4 is a high-precision math library, it is mostly unmodified except the build system.

minizip, pcre-8.02, zlib, and qhull are unmodified and the openrave build system will try to use the system install versions if possible.

Comment 6 Rosen Diankov 2011-02-03 04:36:17 UTC
By the way, the change log should tell you what svn revisions of openrave are stable:

https://sourceforge.net/apps/trac/openrave/wiki/ChangeLog

Whenever you want to do a new release, check that page.

Comment 7 Tim Niemueller 2011-02-08 23:32:06 UTC
Hi Rosen. Thanks for tuning in and the thorough list of packaged libraries. For packaging it would be much more pleasant if you could make tarball releases. Is that planned or could you think about it?

About the libraries:
collada-dom: keep as-is for now, once collada-dom is available as an updated package make an RPM, propose for inclusion and eventually use it.

convexdecomposition: keep internal version, contains non-upstreamed patches, lib seems to be meant for inclusion (no build system), and is linked statically (i.e. no confusion/errant deps for other software)

ann/flann: using system-wide version with my patch already

fparser: keep the included version because of the local changes. If upstream becomes responsive revisit issue.

crlibm: I have packaged it and it is up for review as bug #676159.

minizip, pcre, zlib, qhull: I have made some changes to ensure system-version is used. I have also added statements to %prep stage to delete these directories in the source code before building.

New SRPM is at http://fedorapeople.org/~timn/robotics/openrave-0.2.18-0.3.svn1975.fc14.src.rpm. Spec file has been changed in place.

Comment 8 Rosen Diankov 2011-02-09 14:00:56 UTC
i agree with you. in fact, an official tar ball is the number one item on every openrave user's list ;0)

we're currently developing a proper testing framework that can track regressions, once this is done we'll immediately start publishing tarballs and releases.

For now, I hope you can live with just the change log. 

and 0.2.19 was just released (r1996). the highlight of this release is ikfast being able to handle arbitrarily complex manipulator kinematics!

Comment 9 Tim Niemueller 2011-02-21 21:05:57 UTC
Blocking bugs/reviews for flann and crlibm have succeeded and are building or have been issued as an update. So I think it's time to continue with OpenRave. 

Peter, do you agree that the remaining, and heavily modified version of collada-dom, convexdecomposition, and fparser, which are only built and linked as static libraries, but not installed as shared libraries, should remain in OpenRave until all patches have been integrated into an active upstream?

Rosen, even without full unit testing, tarballs would already be helpful and only a minimum of work ;-)

I hope we can now proceed quickly and get OpenRave accepted, it's getting a more important component of robotic systems these days, and we're currently preparing Fawkes support for it that we'd like to include in the upcoming release and consequently Fedora.

Comment 10 Rosen Diankov 2011-02-21 23:22:47 UTC
hi tim,

this is great! i'm currently working on unit testing ikfast, i just finished fixing python nose's multiprocess support so that robots taking too long can be immediately terminated without taking valuable testing time. 

what is the deadline on getting this into the next fedora? the past couple of weeks have seen some major improvements in openrave functionality, and it would be great to have those included.

about collada, i committed my changes to the collada-dom trunk. you are willing to create a fedora package directly from trunk, then things should be ok. In that case, you would have to uncomment these lines from openrave's root CMakeLists.txt

line 378: find_package(COLLADA)

lines 410-418 starting at "# check for local collada compilation...."

fawkes is very exciting, i see you are the main dev on it ;0)

Comment 11 Tim Niemueller 2011-02-21 23:36:18 UTC
Rosen, can you make a collada-dom release with your changes? http://sourceforge.net/projects/collada-dom/develop says last commit was on 2011-01-15, is that the latest version you're talking about? Or is that no longer the upstream repository?

Do I always have to build all the bells 'n whistles or is there an easy way to build just the libraries? What is it with the 1.4 and 1.5 in the tree, are both needed? Is there an easy way to get rid of the included external libraries, they're really annoying when packaging. Maybe you have a nicely working CMake config for collada-dom? Of we can figure those things out I'd package collada.

Peter, please still consider doing the review now with the perspective to splitting of collada-dom as soon as we figured it out, but the collada buildsys looks more like meant to be built in some PS3 environment and self-contained, not to play nicely with a Linux distro.

About Fawkes: thanks! A student is currently working on adding OpenRave support (topic branch on http://git.fawkesrobotics.org/fawkes.git) for use with our Katana arm. Afaik you two emailed frequently recently :-)

Comment 12 Rosen Diankov 2011-02-21 23:46:46 UTC
hi tim,

yes those are the changes. i can have a cmake build for collada-dom committed within the week if you are willing to wait. what is the fedora deadline on package releases?

there's several people from germany using the katana and openrave, what's your student's name? ;0)

I'm not sure if you are subscribed to the openrave-users list, but recently they've been two major additions that will help planning with low-dof arms like the katana: 5DOF IK and move hand straight with translation IK:

http://openrave.programmingvision.com/ordocs/en/openravepy-html/openravepy.examples.tutorial_ik5d-module.html

http://openrave.programmingvision.com/ordocs/en/openravepy-html/openravepy.examples.movehandstraight2-module.html

Comment 13 Tim Niemueller 2011-02-22 00:02:51 UTC
(In reply to comment #12)
> hi tim,

Hi Rosen.

> yes those are the changes. i can have a cmake build for collada-dom committed
> within the week if you are willing to wait. what is the fedora deadline on
> package releases?

There is no deadline, a package is added when it's ready. Packages can be added to the current release and future ones, so there is no need to wait half a year for the next release. It's just a matter of package maintainer patience...
So yes, I'll happily wait a week to get that CMake build system if it makes packaging easier and avoids the integrated external libraries.

Again, I want to get OpenRave in rather sooner than later in the current stage, which can be upgraded at any time to later known stable versions. So I request again that we go forward with the status quo and improve from there (http://www.jwz.org/doc/worse-is-better.html). Peter?

Comment 14 Rosen Diankov 2011-02-22 04:01:07 UTC
hi tim,

i just committed the cmake build in collada-dom and bumped up its version to 2.3. I also added a *.pc file so you can use it with pkg-config. Basically you would use collada15dom as the name.

In order to compile it use:

mkdir build; cd build; cmake -DCMAKE_INSTALL_PREFIX=xxx ..; make install

openrave r2044 now automatically recognizes the 2.3 installation.

for now check out collada-dom direclty from trunk. if you see now problems, i can commit their fixes to trunk right away... maybe we can eventually do a tar ball release of collada-dom 2.3.

Comment 15 Peter Lemenkov 2011-02-26 09:58:52 UTC
Hello All!
Sorry for the radio silence - I'm still here, and I'm still plan to review it. Will provide more lengthy reply later.

Comment 16 Rosen Diankov 2011-03-01 15:07:27 UTC
Hi all,

openrave r2131 just added support for the Open Asset Import Library:

http://assimp.sourceforge.net/

This allows users to specify robot geometry in many more formats than was previously possible:

http://assimp.sourceforge.net/main_features_formats.html

There is already a discussion in including it into fedora here:

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

perhaps usage with openrave can speed things up...

Comment 17 Tim Niemueller 2011-03-01 16:37:40 UTC
Looks like progress on assimp might take a while. Does that eliminate the need for collada? Sorry, I haven't found a chance to look at your collada changes, yet. Will do asap.

Comment 18 Rosen Diankov 2011-03-01 17:31:17 UTC
collada stays.

assimp is used just for extracting geometry from non-collada files, which is then combined with the openrave custom XML for kinematics.

Comment 19 Rosen Diankov 2011-03-06 07:48:11 UTC
hi guys,

just curious what the timeframe is on this release.

Comment 20 Tim Niemueller 2011-03-06 10:20:38 UTC
Reviews are done when they're done. OpenRAVE is particularly problematic because it uses (and includes) so many non-standard libraries, especially the ones which not available in Fedora, yet. Instead of just packaging OpenRAVE, I'll end up packaging (and maintaining!) more like half a dozen packages in the end...

I hope I'll find some time for collada soonish, but a paper deadline is ahead requiring my full attention atm.

Comment 21 Rosen Diankov 2011-03-06 10:39:02 UTC
..i'm sorry, i think the only problem here was collada. of all the libraries we talked about, this is the only necessary ones. you can ignore the rest for now.

i saw the 'worse is better' article you cited, that's why i thought it would be prudent to hurry up and at least get something out ;0)

Comment 22 Tim Niemueller 2011-03-21 15:51:50 UTC
Peter: Sorry, saw your ping in crlibm request only today. Builds have been done and updates issued. I have packaged an external collada-dom. I'll update the OpenRAVE package at a later point to use this external library. Can you give your opinion about the remaining integrated libraries?

Rosen: Please see the collada-dom review at bug #689432. It contains a small patch, which uses the system-wide installed minizip if available, and which adds ${LIB_SUFFIX} to install to /usr/lib64 on x86_64. I think it would be safe to include it in the main package. Would you please tag the 2.3 release or better, upload tarballs? Thanks!

Comment 23 Rosen Diankov 2011-03-21 23:42:50 UTC
hi tim,

I just patched up collada-dom with your patches (r849), thanks! I'll see if i can create a tarball within the day.

OpenRAVE should already detect the COLLADA external library.

Also, I just patched it OpenRAVE to also use LIB_SUFFIX (r2185). was there any other patches you created for openrave? i couldn't download the sprm.

Comment 24 Tim Niemueller 2011-03-21 23:57:25 UTC
Thanks for applying. Maybe you can make it a 2.3.1 or something. I'm currently looking into assimp. Please let that be the last dependency for a while ;-)
Yes, there are a few more things for OpenRAVE. I'll attach the patch to this ticket and happily drop it once you integrated it. I think there are no controversial things in there.

Comment 25 Tim Niemueller 2011-03-21 23:58:10 UTC
Created attachment 486706 [details]
Patch for upstreaming by Rosen

Comment 26 Rosen Diankov 2011-03-22 01:13:14 UTC
I have a couple of questions about your patches:

1. why are you doing

if(NOT OPENRAVE_OCTAVE_INSTALL_DIR)

if(NOT OPENRAVE_PLUGINS_INSTALL_DIR)

2. What happens to local installs when force-installing openravepy in the python site-packages?

3. Would it be better to do a pkg-config thing for crlibm instead of searching for the include files?

4. sympy has local changes which i'm still waiting for them to be integrated in the official releases (i've opened up issues), so don't use the default sympy over the local one in openrave.

Comment 27 Rosen Diankov 2011-03-22 01:36:36 UTC
for the *_INSTALL_DIR stuff, perhaps we can append it with "CACHE PATH". For example:


set(OPENRAVE_OCTAVE_INSTALL_DIR "${CMAKE_INSTALL_PREFIX}/share/openrave/octave" CACHE PATH)

i think this would still enable you to set new paths from the command line.

Comment 28 Rosen Diankov 2011-03-22 07:34:18 UTC
just committed r2187 that has most of Tim Niemueller's patches. The crlibm installation search is not included since I would like to see pkg-config used.

Also, the installation directories now use CACHE PATH, so they should be settable through the command line.

Comment 29 Tim Niemueller 2011-03-22 09:31:59 UTC
Hi Rosen, thanks for integrating the patches. I'll update the package to the new version. So I think Question 1 is set, whatever works to set it from the outside is fine with me.
To 2.: I don't really know and I hope you know a solution, maybe an option that I can set from the outside? For the package I just need that functionality.
To 3: crlibm does not come with pkg-config files. Even if we added those to Fedora packages, they wouldn't exist in other distros. Hence it is the best to check for header, libraries, and function. I'd like to request that you do add my changes from the patch for the time being.
To 4: Is there an upstream bug for the integration process, what is holding them back? Since sympy is included in Fedora there is no way I can use the internal version, it must be fixed upstream. What fixes did you make? I they are required consider filing a bug to get them added to the sympy (Fedora) package, but be prepared to document by mailing list conversation archives, forum or bug tracker entries that the patch is about to be accepted upstream and that the inclusion of the patch in the Fedora package is just a fast track solution for this review.

Comment 30 Rosen Diankov 2011-03-23 05:19:21 UTC
hi tim,

Ok, i added the crlibm check back in (r2188).

You should also be able to set the path openravepy installs in by setting OPENRAVEPY_INSTALL_DIR via the command line.


As for sympy, the biggest problem I reported is Issue 2081:

http://code.google.com/p/sympy/issues/detail?can=2&q=2081&colspec=ID%20Type%20Status%20Priority%20Milestone%20Owner%20Summary%20Stars&id=2081

Basically, it is possible for sympy to give you wrong equations, which would make the openrave ikfast solver return wrong solutions.  They are still wondering how to fix it so that everyone's happy.

We're looking at the problem from two different angles here: I need to make openrave 100% reliable, you need to reduce unnecessary dependencies so management is easier.

I committed a temporary compromise (r2189):

If you limit openrave to only sympy 0.6.7, I might have come up with a workaround where you don't have to install a local sympy (ie executing ikfast will replace the wrong function in memory).

We can probably support more sympy versions, but they will require IK to be re-tested. There'a an openrave testing server now, so this might be possible in the future:

http://www.openrave.org/testing/job/openrave/

If you ever decide to update the sympy version in fedora core, please email me.

Comment 31 Tim Niemueller 2011-03-23 10:42:37 UTC
Rosen, thanks for you in-depth reply! It's good to see the check re-added. I'll try another build later today, hopefully without any additional patch!

The sympy solution looks good. I will set an explicit dependency in the openrave package on the particular version of sympy. Although I'm not the maintainer of the package, I'd get notice of sympy updates. I'll also request watchcommits permissions to stay on it and contact you if we need to accomodate another sympy version. I'm watching the sympy issue now, it seems stalled with no reply within two months. Maybe you could ping them to move things forward.

The assimp library was already work-in-progress and I just missed it. It was stalled for legal reasons, but that seems fixed. Now there is another issue that is being figured out between the assimp and irrlicht developers. Something to be learned from this odyssey: upstream early and often any changes you make, do not integrate 3rd party libs in your own source trees and stay compatible.

Peter: not having assimp at the beginning means having an integrated converter library. I'd like to keep this (small) piece in there until we have assimp and can drop it.

Comment 32 Rosen Diankov 2011-03-24 16:07:28 UTC
just added a tarball release of collada 2.3:

https://sourceforge.net/projects/collada-dom/files/Collada%20DOM/Collada%20DOM%202.3/

Comment 33 Rosen Diankov 2011-03-25 06:56:16 UTC
openrave r2195 adds a more intelligent check for sympy in case this bug is fixed in future versions. If this bug isn't fixed and using 0.6.7, it will still use the system install. If none of these conditions apply, it will install the local sympy version.

Comment 34 Rosen Diankov 2011-03-28 09:54:31 UTC
Just released openrave 0.2.20 (r2199). You can see change log at:


https://sourceforge.net/apps/trac/openrave/wiki/ChangeLog

Comment 35 Rosen Diankov 2011-04-06 07:09:42 UTC
hi guys, anyone know what's happening with the release? any problems left that i can help in resolving?

Comment 36 Tim Niemueller 2011-04-06 10:34:40 UTC
Hi Rosen. We've used OpenRAVE successfully during the RoboCup German Open 2011 last week. It has been integrated with Fawkes to drive a Katana arm. We're preparing a demo video highlighting some of our work areas, I'll post a link once it's ready.

I have to talk to my student, I think we've used openrave-0.2.20-0.2.svn2227.fc14.x86_64 in the end. It still required a little patch, due to bug #667294 (Boost was missing deprecated iostreams ctors). This should have been resolved by now and I'm going to test it later today. Please consider upgrading to non-deprecated calls in a future version.

Rosen, I have currently set to use single-precision floats to speed up OpenRave a little. Do you think that has overly negative impact, or is that a good default choice for a distro package? When compiling OpenRave at around 50% it compiles openravepy_int.o, which takes more than 1.5GB of memory and is pretty heavy on the system. Any chance of splitting this up in a future release?

We've been seeing some weird slowness on 32-bit systems with slow database generation, while it ran fine with the identical (besides bitness) 64-bit version. I'll discuss this with my team members tomorrow if this was an intermediate problem and if so how it was resolved.

From the packaging site I need to push updates for collada-dom (which is basically ready and built, I just didn't have the time). I have to disable assimp atm, until review bug #635511 is through. Once that is done, we get rid of the ivcon integrated library, right?

After that, it leaves two libraries in there. We're clear that convexdecomposition has no standalone upstream and it got patches, therefore should stay. fparser has critical patches. Rosen, how is upstreaming going on this? Maybe you can try again and tell them that'll bring fparser to the Fedora universe as an incentive?

Peter, please have a look again at this review now, I think we've reached a good state to continue, with the only things to resolve being assimp acceptance and fparser patch upstreaming and eventually ripping it out of OpenRave. What do you think?

SPEC has been changed in-place, the SRPM is at http://fedorapeople.org/~timn/robotics/openrave-0.2.20-0.2.svn2227.fc14.src.rpm. It contains the patch to work with old boost. It can build with or without assimp, for the latter just install the review package. Once it has been accpted I'll add it to the BRs.

Comment 37 Rosen Diankov 2011-04-07 07:38:19 UTC
Hi Tim,

congratulations on a successful robocup! integration with Fawkes sounds very exciting.

thanks for the heads up on the iostreams bug, it is fixed in r2278.

i don't recommend single precision, it is just not worth the speedup. bugs due to single-precision inaccuracies take a long time to find. If you really need it, then just have a openrave-single rpm package along the regular one.

the openravepy_int.cpp problem is known and we're trying to separate into multiple files, but it's not a high priority thing for us.

if you can track down the 32bit db problem slowness, please open a ticket at the openrave trac:

https://sourceforge.net/apps/trac/openrave/newticket

i pinged the fparser author and he agrees it would be cool to have a fedora core rpm. we're also coming to a compromise on what to do with the local changes. however, this is not an issue worth delaying the rpm.

the assimp library is used when opening model files that are not IV or VRML. for example STL, 3DS, etc.

FYI, we've started a new tag called latest_stable:

https://openrave.svn.sourceforge.net/svnroot/openrave/tags/latest_stable

this tag is automatically updated every night if the builds/tests succeed. tar balls of the source (and eventually installers) are also updated:

https://sourceforge.net/projects/openrave/files/latest_stable/

the trunk revision used is written in the commit log. whenever you do releases, please take from this tag.

Comment 38 Rosen Diankov 2011-04-11 10:06:50 UTC
We've started getting all release/package related scripts inside the 'release' folder:

https://openrave.svn.sourceforge.net/svnroot/openrave/trunk/release

currently there is a windows installer and publishing to sourceforge. i'm not sure how the fedora core stuff works, but i would be more than happy to put a script that generates the fedora core spec, rpm files, and publishes it to fedora core. i can give you commit permissions so you can manage this.

Comment 39 Rosen Diankov 2011-04-21 16:34:22 UTC
In the past week a *lot* of updates happened on the install-end of OpenRAVE. THe latest trunk version (0.3.1). Where the include files, share files, and python files are stored were completely changed:

http://openrave-users-list.185357.n3.nabble.com/OpenRAVE-0-3-Official-Release-td2833265.html

http://openrave-users-list.185357.n3.nabble.com/Finding-OpenRAVE-0-3-with-CMake-td2833288.html

The python files are now installed in lib/python2.6/site-packages (or lib/python2.6/dist-packages) accordingly. 

We also generate debian source packages from cmake, which are compiled into debian binaries at:

https://launchpad.net/~openrave/+archive/release

The package tree looks something like:

http://openrave.programmingvision.com/en/main/devel/releases.html#ubuntu-releases

I think it would be possible to merge Tim's changes into the openrave cmake file to produce the RPM spec files. what do you guys think?

Comment 40 Tim Niemueller 2011-04-21 16:49:39 UTC
I hope all patches are included. With changes you mean stuff from the spec file? You are welcome to add it, but the Fedora infrastructure forbids to use this to generate the packages. The spec file will be maintained and updated in the Fedora package git repository.

I'll upgrade to OpenRAVE 0.3 soonish. Peter, can you have a look and do a first review?

Rosen, what's the status on the fparser changes. Have they been upstreamed and released?

Comment 41 Rosen Diankov 2011-04-21 17:02:28 UTC
Hi Tim,

we'll be releasing 0.3.1 in the next 24 hours.

once openrave can output the RPM spec files, you can always run cmake on your computer, generate the spec file, and upload it to the fedora git repository (even as a nightly cron script).

no matter how you look at it, the source of the information is coming from the openrave svn repo and whenever openrave adds a new library dependency, we will simultaneously edit all the dependencies of all distros it supports.

I've emailed the fparser author and we're coming to a compromise, i'll let you know when it is ok to make an rpm package of it.

Comment 42 Rosen Diankov 2011-04-24 10:18:54 UTC
we just released 0.3.1:

https://sourceforge.net/mailarchive/forum.php?thread_name=BANLkTi%3Dibi8J9SXsuSi99R%3DWEiZd8pDx7w%40mail.gmail.com&forum_name=openrave-users

we also started a page on how to install openrave on different platforms:

http://openrave.programmingvision.com/en/main/install.html

it would be great to add Fedora on that list.

what do you think about the source rpm proposal?

Comment 43 Rosen Diankov 2011-07-03 08:30:31 UTC
hi guys,

i'm not sure what happened with fedora rpm packages. Just letting you know that openrave 0.4.0 was officially released yesterday.

is there a chance we can get the source rpm file also put inside openrave subversion repository?

thanks,
rosen,

Comment 44 Tim Niemueller 2011-07-03 10:16:14 UTC
Rosen, what's the state of the fparser changes, have they been upstreamed, yet? You can of course put the SRPM file in your subversion (though my take is that binary package stuff generally does not belong in a subversion repository), but it will not be the source from which Fedora builds its packages, we want tarballs that we put on a Fedora mirror. Likewise the authoritative spec file will reside in Fedora's git repo, but you are obviously free to include a copy in your repository. I hope I find some time to package 0.4.0 next week, but it's busy atm.

Peter, can you start a first review cycle so that we can straighten out rough edges? Thanks!

Comment 45 Rosen Diankov 2011-07-03 10:23:02 UTC
hi tim,

the fparser author has been very cooperative and he released a new version that should allow openrave usage without any internal modifications. i'll email this thread once the changes are done, so hopefully we can have everything resolved in openrave 0.4.1

Comment 46 Rosen Diankov 2011-07-03 10:23:29 UTC
also, where the SRPM link?

thanks,

Comment 47 Peter Lemenkov 2011-07-03 10:50:31 UTC
Hello folks. Sorry for the hiatus - I was busy with other stuff.

Tim, where is the latest src.rpm to start review with?

Comment 48 Rosen Diankov 2011-07-03 11:10:51 UTC
by the way, the fparser version that is ok to package is 4.4.3

Comment 49 Tim Niemueller 2011-07-23 06:13:57 UTC
I have updated the package to the latest release 0.4.1.

Rosen, I had to make some changes to make OpenRAVE compatible again. Most importantly I had to remove the extra library suffix before the .so, that's pretty uncommon and confusing. I changed pkgconfig and cmake files along the way. The include directory now contains the full version instead of just major and minor, I do not see a good reason to shorten it. I have also made a couple of fixes for the new Boost version. I'll attach the patch to this ticket for your review and hopefully upstream application.
If you have a minute, would you mind to write a cmake-based buildsys for fparser? It comes as bare code with nothing but C++ files. Ideally, it installed pkg-config and cmake files to find the library, so that we can then use this in OpenRAVE to detect if its installed system-wide.

Peter, please check the new SRPM and let me know what you think.

SRPM is at http://fedorapeople.org/~timn/robotics/openrave-0.4.1-1.fc15.src.rpm. It requires a new collada build, which I have just uploaded. If you want to rebuild please check Koji for collada-dom and download the latest build.

Comment 50 Tim Niemueller 2011-07-23 06:14:57 UTC
Created attachment 514810 [details]
Patch for upstreaming by Rosen

Comment 51 Rosen Diankov 2011-07-23 08:30:06 UTC
Hi Tim,

The version number suffixes allow multiple versions of openrave to be installed at the same (0.4.1 was still experimental, but the newest 0.4.2 does a much better job of this). We will eventually have symlinks installed through update-alternatives that change the openrave version used.

We worked very hard to make this possible, so it would be great if the fedora package is consistent with the ubuntu packages. Users shouldn't have to deal with fedora-only quirks.

We still have to integrate the new fparser into OpenRAVE. We're about to release 0.4.2, so it'll be a great timing to have a cmake-package for fparser for it.

Please send the boost fixes asap so we can integrate them in 0.4.2.

Are you talking about the recent release of collada-dom 2.3?

thanks,

Comment 52 Rosen Diankov 2011-07-23 08:31:22 UTC
I just saw the patch for the boost filesystem fixes, thanks

Comment 53 Rosen Diankov 2011-07-23 08:49:03 UTC
Hi Tim,

We just committed r2633 that fixes the boost problems (just removing the ::native right?), and we fixed the openrave.pc.in file

Also, after looking at your patches it seems you replaced the MAJOR.MINOR string with OPENRAVE_VERSION. This is not a good idea. The openrave patch versions preserve ABI compatibility and the shared objects of the same major.minor version are always interchangeable.

Here's a small explanation of how we're managing verisons:

http://openrave.org/en/main/devel/releases.html#versioning

Comment 54 Rosen Diankov 2011-07-23 12:20:00 UTC
> Also, after looking at your patches it seems you replaced the MAJOR.MINOR
> string with OPENRAVE_VERSION. This is not a good idea. The openrave patch
> versions preserve ABI compatibility and the shared objects of the same
> major.minor version are always interchangeable.


Therefore, we only want the user to specifically pick in the programs an openrave major.minor version, and not the full major.minor.patch version.

Comment 55 Tim Niemueller 2011-07-23 17:59:46 UTC
(In reply to comment #51)
> Hi Tim,
> 
> The version number suffixes allow multiple versions of openrave to be installed
> at the same (0.4.1 was still experimental, but the newest 0.4.2 does a much
> better job of this). We will eventually have symlinks installed through
> update-alternatives that change the openrave version used.

This is a no-go. There is one version installed via the package manager, that's it. I only kept it in the include dir because there is another openrave dir inside it. And you still can have multiple versions, as the SOVERSION is attached to the file name. Keeping up2date with OpenRAVE is hard enough, having more than one version would make things pretty bad.

> We worked very hard to make this possible, so it would be great if the fedora
> package is consistent with the ubuntu packages. Users shouldn't have to deal
> with fedora-only quirks.

Nope, that's a Debian/Ubuntu thing, but cannot happen on Fedora besides very specific exceptions (core libraries like glib/gtk for example).

> We still have to integrate the new fparser into OpenRAVE. We're about to
> release 0.4.2, so it'll be a great timing to have a cmake-package for fparser
> for it.

Great!

> Please send the boost fixes asap so we can integrate them in 0.4.2.
> 
> Are you talking about the recent release of collada-dom 2.3?

It's 2.3.1, I needed it to have the cmake files. Update is pending, cf. Bodhi and Koji.

Comment 56 Tim Niemueller 2011-07-23 18:04:07 UTC
(In reply to comment #54)
> 
> Therefore, we only want the user to specifically pick in the programs an
> openrave major.minor version, and not the full major.minor.patch version.

You're trying to solve problems particularly prevalent on source installs, here the distro will keep up with OpenRAVE and that will be one specific sequence of versions. So actually I'm probably going to remove all version numbers, so even the one from the exclude file. If you have a long-term stable 1.0 release at some point, and then a couple of years later a 2.0, then having a multi-install could be an option, like for Eigen. But not just for minor versions because the API is still a moving target. The solution is to stabilize the API, not increase the number of required (and obviously incompatible) versions on a system.

So I'd like to have some way to optionalize your version suffixes. So if there was an OPT flag we could simply turn it off on Fedora. Developers should use pkg-config or cmake to get the appropriate include and link flags so there are no fundamental differences between Ubuntu and Fedora afterwards.

Comment 57 Rosen Diankov 2011-07-24 04:18:04 UTC
hi tim,

thank you for the feedback, it was really helpful. I'm worried that removing suffixes would make it difficult for openrave-aware programs to run cross platform.

Whether fedora allows multiple openrave versions or only one is up to you, so even if there are suffixes on the files, users will only be interacting with one version.

How about we add these symlinks to give the fedora core users the impression that openrave is installed without version numbers:

/usr/include/openrave -> /usr/include/openrave-0.4
/usr/share/openrave -> /usr/share/openrave-0.4

Are there any other files necessary?

Please don't misunderstand my hesitation for adding a cmake option to remove the suffixes. I agree with you that the solution is to stabilize the API, but let's be serious here, all library developers want to stablize their APIs once and never touch it again. And how many times do APIs evolve into something that the users just don't like and want to go back to the old one easily?

It is because we don't know how motion planning technologies will develop that makes it difficult to solidify an API. And even though OpenRAVE undergoes changes, we take very careful precautions to preserve backward compatibility. If anything breaks, it is very minor and rare (like a function returning bool now returns void because it throws exceptions on errors).

As for the package names, if you name the packages openrave0.3, openrave0.4, openrave0.5, then you should be able to install multiple openrave packages. I'm not sure how python packages work on fedora, but I would be surprised if you are locked into only one python version per fedora distribution.

In OpenRAVE, you can set -DOPT_BUILD_PACKAGE_DEFAULT=OFF to avoid installing the symlinks of a particular package.

Comment 58 Rosen Diankov 2011-07-24 04:43:23 UTC
Sorry it should be:

/usr/include/openrave -> /usr/include/openrave-0.4/openrave

> attached to the file name. Keeping up2date with OpenRAVE is hard enough, having
> more than one version would make things pretty bad.

Please tell us what is so difficult to keep up with OpenRAVE updates so we can attempt to resolve the problem.

Comment 59 Jason Tibbitts 2011-07-30 04:17:09 UTC
What is being requested of the SCM admins here?  Someone just set the fedora-cvs flag but I don't see any request to process.

Comment 60 Tim Niemueller 2011-07-30 04:22:19 UTC
I think Peter meant to set the review flag.

Comment 61 Peter Lemenkov 2011-07-30 04:23:42 UTC
(In reply to comment #59)
> What is being requested of the SCM admins here?  Someone just set the
> fedora-cvs flag but I don't see any request to process.

Sorry, Jason - that's my fault. I wanted to raise fedora-review actually.

Comment 62 Jason Tibbitts 2011-07-30 05:28:36 UTC
It should be assigned to someone if it's being reviewed.  Currently it's assigned to our good friend "nobody".

Comment 63 Gwyn Ciesla 2011-07-30 14:37:50 UTC
Clearing flag.

Comment 64 Peter Lemenkov 2011-07-31 19:13:17 UTC
REVIEW:

Legend: + = PASSED, - = FAILED, 0 = Not Applicable

+ rpmlint is not completely silent:

sulaco ~/rpmbuild/SPECS: rpmlint ~/Desktop/openrave-*
openrave.x86_64: W: shared-lib-calls-exit /usr/lib64/libopenrave-core.so.0.4.1 exit.5

^^^ This should be reported upstream  (Rosen, ping!) as it could be a possible defect. Not a blocker anyway.

openrave.x86_64: W: non-conffile-in-etc /etc/bash_completion.d/openrave.bash

^^^ That's ok. 
openrave.x86_64: W: wrong-file-end-of-line-encoding /usr/share/doc/openrave-0.4.1/AUTHORS

^^^ This should be fixed with dos2unix or sed. Please, take care of this message.

openrave.x86_64: W: no-manual-page-for-binary openrave
openrave-devel.x86_64: W: no-manual-page-for-binary openrave-config
openrave-devel.x86_64: W: no-manual-page-for-binary openrave-createplugin.py

^^^ That's ok for now. Hopefully they will be written someday.

openrave-octave.x86_64: W: unstripped-binary-or-object /usr/libexec/octave/packages/openrave-0.4.1/orcreate.mex
openrave-octave.x86_64: W: unstripped-binary-or-object /usr/libexec/octave/packages/openrave-0.4.1/orwrite.mex
openrave-octave.x86_64: W: unstripped-binary-or-object /usr/libexec/octave/packages/openrave-0.4.1/orread.mex

^^^ that should be fixed. Just chmod them to 0755 at the end of  %install section and rpmbuild will do the rest.

openrave-octave.x86_64: W: no-documentation
openrave-python.x86_64: W: no-documentation

^^^ That's ok.

openrave-python.x86_64: E: script-without-shebang /usr/lib64/python2.7/site-packages/openravepy/databases/linkstatistics.py
openrave-python.x86_64: E: non-executable-script /usr/lib64/python2.7/site-packages/openravepy/interfaces/__init__.py 0644L /usr/bin/env
openrave-python.x86_64: E: non-executable-script /usr/lib64/python2.7/site-packages/openravepy/__init__.py 0644L /usr/bin/env

^^^ That should be fixed if possible. I'm not sure about purposes of these scrips so I can't say for sure what to do with them - I guess that you should  remove shebang and mark them as 0644.

openrave-python.x86_64: W: no-manual-page-for-binary openrave.py
openrave-python.x86_64: W: no-manual-page-for-binary openrave-robot.py

^^^ No man-pages for now. It's ok for packaging, although it's not that good for project - adding good old man-pages is always a good idea  (Ping again, Rosen).

5 packages and 0 specfiles checked; 3 errors, 13 warnings.
sulaco ~/rpmbuild/SPECS: 

+ The package is named according to the  Package Naming Guidelines.
+ The spec file name matches the base package %{name}, in the format %{name}.spec.
+ The package meets the Packaging Guidelines. Well, this package is quite big so I could overlook something, but it does look ok at first glance.
+ The package is licensed with a Fedora approved license and meets the Licensing Guidelines.
+ The License field in the package spec file matches the actual license.
+ The file, containing the text of the license(s) for the package, is included in %doc.
+ The spec file is written in American English.
+ The spec file for the package is legible.
+ The package successfully compiles and builds into binary rpms on at least one primary architecture:

http://koji.fedoraproject.org/koji/taskinfo?taskID=3241903

+ All build dependencies are listed in BuildRequires.
0 No need to handle locales.
+ The package stores shared library files in some of the dynamic linker's default paths, and it calls ldconfig in %post and %postun.
+ The package does NOT bundle copies of system libraries.
0 The package is not designed to be relocatable.
+ The package owns all directories that it creates.
+ The package does not list a file more than once in the spec file's %files listings.
+ Permissions on files are set properly.
+ The package has a %clean section, which contains rm -rf %{buildroot} (or $RPM_BUILD_ROOT).
+ The package consistently uses macros.
+ The package contains code, or permissible content.
0 No extremely large documentation files.
+ Anything, the package includes as %doc, does not affect the runtime of the application.
+ Header files are stored in a -devel package.
0 No static libraries.
+ The pkgconfig(.pc) files are stored in a -devel package and necessary runtime requirement added.
+ The library file(s) that end in .so (without suffix) is(are) stored in a -devel package.
+ The -devel package requires the base package using a fully versioned dependency: Requires: %{name} = %{version}-%{release}
+ The package does NOT contain any .la libtool archives.
0 The package does not contain *.desktop file. This isn't a blocker but I strongly advice you to convince upstream (Rosen, ping again!) to add it.

- The package mustn't own files or directories already owned by other packages. Please, add cmake as a requires to the *-devel sub-package because you installed fines into %{_libdir}/cmake.

+ At the beginning of %install, the package runs rm -rf %{buildroot} (or $RPM_BUILD_ROOT).
+ All filenames in rpm packages are valid UTF-8.

Ok, almost finished. Tim, please, fix/comment issues mentioned above and I'll continue.

Comment 65 Tim Niemueller 2011-12-13 11:54:53 UTC
Peter, thanks for the review. Finally I got around to package OpenRAVE 0.5.0 and fix the problems you mentioned:
- dos2unix on some files
- no more zero-length octave files (fixed upstream)
- did not fix the non-executable-script warnings, they should be fixed upstream
- devel sub-package depends on cmake

I have also updated the patch, Rosen please re-check if this is suitable to include upstream, it does:
- provide compatibility with older Boost versions (think of Enterprise Linux)
- Make include directory name a variable that can be overridden (for example to use only a major version suffix at some point)
- Allow overriding of share directory (not using version suffix on Fedora, but default to use it
- Allow overriding of library suffix (no version suffix on Fedora)
- Allow overriding of CMake directory name (default to the original value, set on Fedora from cmake call)
- Allow overriding of CPack package install directory
- pkg-config file uses the new variables

The new SRPM is at http://fedorapeople.org/~timn/robotics/openrave-0.5.0-1.fc15.src.rpm, spec file changed in place.

Comment 66 Rosen Diankov 2011-12-13 12:23:22 UTC
hi tim,

sorry i've been procrastinating on this issue recently, i can take a look at your changes. currently the patch attached to this thread is openrave-0.4.1... where can we get the latest patch?

good news is there's still a couple of weeks before we officially release/freeze 0.5.0, so we can include these changes. we're aiming for end of the year. in terms of API changes, there's 3 tickets left to resolve: #158, #182, #228

Comment 67 Tim Niemueller 2011-12-13 12:36:52 UTC
Created attachment 546188 [details]
Patch to 0.5.0 for upstreaming by Rosen

Comment 68 Tim Niemueller 2011-12-13 12:40:08 UTC
I have added the patch, it's in the SRPM as well. 0.5.0 is not released? I thought the version at the top of the OpenRAVE page would state the latest release, no? Really looking forward to this version and having it in Fedora finally.

Peter, if you're happy to accept the current state I won't release a package before 0.5.0 final comes out, hence I won't change the package version to a pre-release one for the review right now unless you insist.

Comment 69 Rosen Diankov 2011-12-13 13:38:54 UTC
hi tim,

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?

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}')"


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?

3. i would like to rename OPENRAVE_INCLUDE_DIRNAME to OPENRAVE_INCLUDE_DIR.

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.

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..

Comment 70 Rosen Diankov 2011-12-13 13:40:20 UTC
for the boost filesystem, i see you added "boost::filesystem::native", was there a reason for this?

Comment 71 Tim Niemueller 2011-12-13 14:01:00 UTC
(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).

Comment 72 Tim Niemueller 2011-12-13 14:02:49 UTC
(In reply to comment #70)
> for the boost filesystem, i see you added "boost::filesystem::native", was
> there a reason for this?

Compatibility with older Boost versions as stated in the previous comment. This is also taken from my older patch and though I haven't tried it again on my CentOS box I'd assume it still breaks. And since it is an easy fix I wanted to keep it just in case.

Comment 73 Rosen Diankov 2011-12-13 14:23:59 UTC
> 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.

that makes sense, unfortunately i think there's a debian thing where we need relative paths. given that you are not using CMAKE_INSTALL_PREFIX couldn't you add something like:

cmake -DCMAKE_INSTALL_PREFIX=""

?

i'll re-examine the boost patches.


> 
> > 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).

Comment 74 Rosen Diankov 2011-12-14 00:31:54 UTC
hi tim,

i committed the changes to r2909. here's my notes:

new variables:

- OPENRAVE_CMAKE_INSTALL_DIR, replaces OPENRAVE_CMAKE_DIRNAME
- OPENRAVE_INCLUDE_INSTALL_DIR - replaces OPENRAVE_INCLUDE_DIRNAME

pkg-config file now uses:

openrave${OPENRAVE_BIN_SUFFIX}.pc
openrave${OPENRAVE_BIN_SUFFIX}-core.pc

We stuck with openrave-core since that's what the library name is.


you had some typos on boost::filesystem::system_complete, it would be great if you test on RHEL again.

Comment 75 Rosen Diankov 2011-12-14 13:21:17 UTC
make that r2911

Comment 76 Rosen Diankov 2012-01-07 03:22:35 UTC
hi guys, any feedback on the last openrave build before we officially release 0.5.0?

Comment 77 Tim Niemueller 2012-01-07 11:53:08 UTC
I'll build the new version today or tomorrow as time permits.

Comment 78 Tim Niemueller 2012-01-11 13:16:18 UTC
I tried to build, my patch is down to two things (will create a new bug attachment):
- Add @LIB_SUFFIX@ in pkg-config files (probably just an oversight)
- Full path requirement in config.h.in

But the F-15 sympy version is 0.7.1 and it assumes that it needs to build the local sympy version, which is not acceptable according to the Fedora guidelines. Rosen, is there an OpenRAVE version that works with this newer sympy version?

The first part of the patch is simpe. For the second part I have added three new variables which contain the absolute path (including the CMAKE_INSTALL_PREFIX). I can now easily override those from the spec file and the original behavior for the config.h.in stays the same while having only one substitution variable there.
Please have a look and let me know if you can apply the patches

Comment 79 Rosen Diankov 2012-01-12 00:53:06 UTC
hi tim,

ok, the two features you requested are now in r2949

#define OPENRAVE_PLUGINS_INSTALL_DIR "@OPENRAVE_PLUGINS_INSTALL_ABSOLUTE_DIR@"
#define OPENRAVE_DATA_INSTALL_DIR "@OPENRAVE_DATA_INSTALL_ABSOLUTE_DIR@"
#define OPENRAVE_PYTHON_INSTALL_DIR "@OPENRAVE_PYTHON_INSTALL_ABSOLUTE_DIR@"



As for sympy, i think you'll have an easier time including a sympy 0.6.x version into fc15 rather than getting ikfast to work with 0.7.1. of course, we'd be more than happy to get patches for it, as long as 0.6.x compat is preserved.

Comment 80 Tim Niemueller 2012-01-12 09:55:46 UTC
Ok, I'll try the new version, but with sympy we got a problem on our hands. Since F-15 a newer version is part of the distro and we cannot just duplicate that with major effort. It might make more sense to spend that time on getting OpenRAVE fit for Sympy 0.7.1, it could be a show stopper otherwise.

Since I have no experience with Sympy, what's the big deal with the new version, what needs to be changed, how much effort would it be?

Comment 81 Rosen Diankov 2012-01-12 12:33:27 UTC
hi tim,

i understand your concerns. i don't know what changed in 0.7.1, feel free to explore this on your free time.

Comment 82 Tim Niemueller 2012-01-12 12:45:56 UTC
Hmm, that sounds like it would never happen? It's out there for more than half a year and it'll eventually show up in other distros as well. Have you tried it and spotted errors or is it merely a "0.6.7 works so let's don't touch it" kind of thing? Any Python/OpenRAVE guru on your side who could take a look? I don't have the time for a while...

Comment 83 Rosen Diankov 2012-01-12 16:16:55 UTC
you are right, eventually we'll have to add support. i plan to get around to it in 4 months

Comment 84 Rosen Diankov 2012-01-20 13:38:19 UTC
openrave 0.5.0 was just officially released. several bugs were fixed in the last 48 hours, so i recommend you create new rpms with the following code:

https://openrave.svn.sourceforge.net/svnroot/openrave/tags/0.5.0

Comment 85 Rosen Diankov 2012-02-14 05:33:55 UTC
we're now at openrave 0.6.3 (r3104) with several exciting changes to the fparser library. As of fparser v4.4.3, openrave can now use the library without any local changes at all, meaning that it is now possible to create an official RPM of the package!

http://www.programmingvision.com/fparser4.4.3_with_cmake.tgz

if making a diff against the official 4.4.3 that can be found here:

http://warp.povusers.org/FunctionParser/

please note the extra files that are needed:

1. CMakeLists.txt
2. modules-cmake directory
3. fpconfig.hh.in
4. fparser-config.cmake.in
5. fparser-config-version.cmake.in

Comment 86 Tim Niemueller 2012-04-19 22:27:43 UTC
Added fparser library based on cmake stuff in OpenRAVE (missed your last post somehow). Review has been posted and waits for approval (see depending bug).

Have you seen version 4.5 and that it removed the eval function? I had packaged that first before realizing that the current OpenRAVE is incompatible.

I have added a tiny patch allowing it to build against system sympy 0.7.1. I have yet to run tests to check if that actually works. Rosen, do you have made any tests by now?

New SRPM is at http://fedorapeople.org/~timn/robotics/openrave-0.6.4-1.fc15.src.rpm and spec has been changed in place.

Once fparser has been approved and imported I can do new scratch builds and hopefully finish this review.

Comment 87 Rosen Diankov 2012-04-19 23:41:28 UTC
hi tim,

unfortunately compatibility for sympy 0.7.1 is not that easy. like promised i'll be looking into compatibility for it shortly and within the week it should be in trunk. but keep in mind it will be officially released in openrave 0.8.0.
for now, just package sympy 0.6.x, i'm sure fedora core supports offering rpms with different versions of libraries as long as they are named differently. it's not worth breaking openrave functionality.

also, just did a hot fix for openrave 0.6.4 for "openrave-core.pc.in", i highly suggest you repackage the srpm with r3204 of

https://openrave.svn.sourceforge.net/svnroot/openrave/tags/0.6.4

as for libfparser, the priority on it is really low. by the time openrave 0.8.0 releases, most likely fparser will have reached a different version ;0)

Comment 88 Rosen Diankov 2012-04-20 03:44:34 UTC
just committed another hotfix for 0.6.4, make that r3206 of

https://openrave.svn.sourceforge.net/svnroot/openrave/tags/0.6.4

Comment 89 Rosen Diankov 2012-04-23 05:25:15 UTC
hi tim,

i had a question on the comment you made here:

http://git.fawkesrobotics.org/fawkes.git/commitdiff/eb2deb5bd5757b16c1b68a56cc7bc815319646ae


"Also, the OpenRAVE tools gave invalid flags for x86_64 (included 32bit lib dir)."

can you give us the current output and expected output? by openrave tools, do you mean the openrave-config program?

Comment 90 Rosen Diankov 2012-04-23 05:31:33 UTC
while i am at it, i saw this post:

http://git.fawkesrobotics.org/fawkes.git/commitdiff/7488ceba5729fb35b7faab86779434acdc76cf3f

you should be able to do:

const Vector& trans;
__env->plot3(&trans[0], ...)

Comment 91 Tim Niemueller 2012-05-15 11:08:37 UTC
(In reply to comment #87)
> hi tim,
> 
> unfortunately compatibility for sympy 0.7.1 is not that easy. like promised
> i'll be looking into compatibility for it shortly and within the week it should
> be in trunk. but keep in mind it will be officially released in openrave 0.8.0.
> for now, just package sympy 0.6.x, i'm sure fedora core supports offering rpms
> with different versions of libraries as long as they are named differently.
> it's not worth breaking openrave functionality.

Nope, it ain't that easy and it would mean I need to file another review request. I have already done almost half a dozen for OpenRAVE, so I'd rather wait for sympy 0.7 compatibility.

> also, just did a hot fix for openrave 0.6.4 for "openrave-core.pc.in", i highly
> suggest you repackage the srpm with r3204 of
> 
> https://openrave.svn.sourceforge.net/svnroot/openrave/tags/0.6.4

I'll have a look.

> as for libfparser, the priority on it is really low. by the time openrave 0.8.0
> releases, most likely fparser will have reached a different version ;0)

Well, for now I'm happy with the old fparser version. Let me know when I can upgrade.

Comment 92 Tim Niemueller 2012-05-15 11:12:14 UTC
> can you give us the current output and expected output? by openrave tools, do
> you mean the openrave-config program?

The output always includes "-L/usr/lib" which is wrong on x86_64. Also, historically *-config had been used to know which flags were used to compile that particular tool and not what flags should be used to link against it. That has arguably change over the recent years, but I still prefer pkg-config whenever available.

(In reply to comment #90)
> you should be able to do:
> 
> const Vector& trans;
> __env->plot3(&trans[0], ...)

Right, that'll be faster. I'll change it. Thanks for catching this. Is there a specific reason why there is no convenience (overridden) method to directly pass the transform?

Comment 93 Rosen Diankov 2012-05-16 02:47:48 UTC
hi tim,

still working on sympy 0.7.x compat and should be done soon.

If openrave-config is including "-L/usr/lib", it is because some library openrave links to is including /usr/lib in its linked directories. The stringis the following:

-L@CMAKE_INSTALL_PREFIX@/lib -lopenrave@OPENRAVE_LIBRARY_SUFFIX@ @OPENRAVE_BOOST_LIB_DIRS@ @Boost_THREAD_LIBRARY_RELEASE@

If you can find out which library (boost most likely?) then you can find out why it is giving wrong info.


The openrave latest_stable branch now includes fparser 4.5 since it allows us to compile with LLVM/Clang

As for the Vector casts, we removed the operator (float*) cast because several compiles failed because it was clashing with operator[]. For example, which function should v[0] call? I believe it was encouraging unsafe C-type array practices.

Comment 94 Tim Niemueller 2012-05-16 13:54:34 UTC
(In reply to comment #93)
> still working on sympy 0.7.x compat and should be done soon.

Excellent, looking forward to it!

> If openrave-config is including "-L/usr/lib", it is because some library
> openrave links to is including /usr/lib in its linked directories. The stringis
> the following:
> 
> -L@CMAKE_INSTALL_PREFIX@/lib -lopenrave@OPENRAVE_LIBRARY_SUFFIX@
> @OPENRAVE_BOOST_LIB_DIRS@ @Boost_THREAD_LIBRARY_RELEASE@

I assume CMAKE_INSTALL_PREFIX is /usr resulting in -L/usr/lib.

> The openrave latest_stable branch now includes fparser 4.5 since it allows us
> to compile with LLVM/Clang

Ok, so I'll try the most recent OpenRAVE version with a system-wide installed fparser 4.5 then.

> As for the Vector casts, we removed the operator (float*) cast because several
> compiles failed because it was clashing with operator[]. For example, which
> function should v[0] call? I believe it was encouraging unsafe C-type array
> practices.

I was more wondering why there is no plot3(OpenRAVE::Vector &trans, ...).

Comment 95 Rosen Diankov 2012-05-16 14:46:48 UTC
added @LIB_SUFFIX@ to openrave-config.in in r3285

do you think turning this code:

plot3(&v[0],..)

to

plot3(v,...)

justifies a new function and API change?

Comment 96 Tim Niemueller 2012-05-16 15:14:58 UTC
Not, not an API change. I was wondering about a convenience method in addition to the existing one just by overloading. But it's absolutely minor, don't bother.

r3285 should fix the issue. pkg-config is still my favorite though ;-)

I'll prepare a new version as soon as I find the time.

Comment 97 Rosen Diankov 2012-05-27 09:32:55 UTC
great news!

ikfast now works with sympy 0.7.x (trunk r3320). You can download the two necessary files from here and patch up the openrave 0.6.4 files:

https://openrave.svn.sourceforge.net/svnroot/openrave/trunk/python/ikfast.py

https://openrave.svn.sourceforge.net/svnroot/openrave/trunk/python/ikfast_generator_cpp.py

In openrave 0.7.0 we've added inverse dynamics computation and many other cool new features, but it will be several (or more) weeks before we officially release it as openrave 0.8.0

Comment 98 Rosen Diankov 2012-05-27 11:23:53 UTC
i forgot to say that if you are going to just replace the ikfast* files, you'll need to get rid of the sympy 0.6.x check in CMakeLists.txt

Comment 99 Rosen Diankov 2012-07-11 08:46:02 UTC
hi guys, 

great news, we released openrave 0.6.6 last week that has the latest and greatest dependencies (including sympy 0.7.x support and collada 2.4.x support)

please tell me if there's anything else needed for an official fedora core rpm

Comment 100 Michael Schwendt 2014-03-19 20:13:14 UTC
Has anyone ever before asked the FPC about a bundling exception for fparser?
https://fedoraproject.org/wiki/Packaging:No_Bundled_Libraries#Copylibs

Alternatively, instead of building a shared libfparser (which may be problematic due to the nature of the code, e.g. the various defines in fpconfig.hh and optional features), it would be possible to build a fparser-source binary rpm, which packages like openrave may build-require and *copy* from (since fparser is a copylib).

Comment 101 Michael Schwendt 2014-03-19 20:31:37 UTC
> http://fedorapeople.org/~timn/robotics/openrave-0.6.4-1.fc15.src.rpm 

That's a damaged file:

$ rpm -Kv openrave-0.6.4-1.fc15.src.rpm 
openrave-0.6.4-1.fc15.src.rpm:
    Header SHA1 digest: OK (d2a244f989f76bc5dd02d95386a3d86b4473666e)
    MD5 digest: BAD Expected(7fcab7a1c091b5384736f7d98e6132a8) != (e948197b48aa8bf3705ecbd247b100f3)

Comment 102 Rosen Diankov 2014-03-19 21:17:25 UTC
FYI, i'm planning to make a github repo for fparser and put all my cmake scripts on it so it installs easily on any linux/windows system. this will happen within a month.

Comment 103 Till Hofmann 2015-07-01 13:45:21 UTC
Hi everyone,

I'm picking up Tim's work from here (with Tim's consent). I'm not sure whether I should submit a new review request, so I'll add my comments here first.

SPEC: https://thofmann.fedorapeople.org/openrave.spec
SRPM: https://thofmann.fedorapeople.org/openrave-0.9.0-4.20150624gitb32979b.fc20.src.rpm
patch0: https://thofmann.fedorapeople.org/openrave.pkgconfig.patch
patch1: https://thofmann.fedorapeople.org/openrave.pyscript-permissions.patch

The new version contains the following changes:
- update to a newer upstream version: Since the last release (0.9.0 from Oct 2012) a lot of fixes have been included, so it makes more sense to use the current git version instead of the last release
- move documentantion into a separate doc package
- fix AutoProvides and AutoRequires, which included openrave plugins
- rename python package to python2-openrave, following the Python Packaging Guidelines (unfortunately, we cannot yet build a python3 package, because there is no python3 version of flann)
- exclude arm from the supported architectures (upstream bug: https://github.com/rdiankov/openrave/issues/354)
- clean up BR
- split models into a separate noarch package

Since there is now a separate package for fparser, this package now uses that external fparser, which ships all headers needed to build openrave. Since fparser does not currently provide a cmake module, I've added a patch to find fparser using pkg-config. I've also sent the patch upstream: https://github.com/rdiankov/openrave/pull/352

The second patch contains a simple fix to make all installed python scripts executable. Also sent upstream: https://github.com/rdiankov/openrave/pull/353

We still use two 3rdparty libs: convexdecomposition and ivcon. 
ivcon is so small (two files) that I think it does not make sense to package it separately. I'm not sure how to proceed with convexdecomposition. 


Koji scratch build: http://koji.fedoraproject.org/koji/taskinfo?taskID=10259352

rpmlint still gives some warnings:

openrave.x86_64: W: shared-lib-calls-exit /usr/lib64/libopenrave-core.so.0.9.0 exit.5
--> reported upstream: https://github.com/tpaviot/oce/issues/161

openrave.x86_64: W: no-documentation
openrave-octave.x86_64: W: no-documentation
openrave-models.noarch: W: no-documentation
openrave-devel.x86_64: W: no-documentation
--> separate doc package

openrave-devel.x86_64: W: only-non-binary-in-usr-lib
--> symlink

openrave.x86_64: W: no-manual-page-for-binary openrave
openrave-devel.x86_64: W: no-manual-page-for-binary openrave-config
openrave-devel.x86_64: W: no-manual-page-for-binary openrave-createplugin.py
--> no manpages provided by upstream.

openrave.x86_64: E: non-executable-script /usr/share/openrave/openrave.bash 0644L /bin/bash
--> I *think* this script is only used to set up the environment, and should always be sourced. But this may actually need to be fixed.


Please let me know if I should submit a new review request.

Comment 104 Till Hofmann 2015-08-13 09:08:51 UTC
I've created a COPR for openrave:

https://copr.fedoraproject.org/coprs/thofmann/openrave/

Comment 105 Till Hofmann 2016-05-21 19:34:49 UTC
I submitted a new review request with some additional changes. If anyone wants to review it, I'd be very thankful!

*** This bug has been marked as a duplicate of bug 1338339 ***


Note You need to log in before you can comment on or make changes to this bug.