no commit

classic Classic list List threaded Threaded
20 messages Options
Reply | Threaded
Open this post in threaded view
|

no commit

Radu B. Rusu
Administrator
Please do not checkout or commit anything to the trunk of PCL until tomorrow morning (PST).

I'm committing a big patch which might break some things, but I am doing it now so that we avoid conflicts. In the
future I'll branch and plan these changes better.

The good news is that Troy's idea worked 100%! All GCC compiles now are < 500MB and blazingly fast! Even more, any
future code that links against PCL won't need to do any templating if it uses the primarily supported point types (as
defined in include/pcl/point_types.h).

Preliminary tests show that pcl_ros now compiles faster than pcl (ha!). Expect some serious tests tomorrow.

Cheers,
Radu.
--
http://pointclouds.org
_______________________________________________
[hidden email] / http://pointclouds.org
https://code.ros.org/mailman/listinfo/pcl-users
Reply | Threaded
Open this post in threaded view
|

Re: no commit

Geoffrey Biggs
So... what was Troy's idea? :)

On 06/12/10 20:18, Radu Bogdan Rusu wrote:

> Please do not checkout or commit anything to the trunk of PCL until tomorrow morning (PST).
>
> I'm committing a big patch which might break some things, but I am doing it now so that we avoid conflicts. In the
> future I'll branch and plan these changes better.
>
> The good news is that Troy's idea worked 100%! All GCC compiles now are < 500MB and blazingly fast! Even more, any
> future code that links against PCL won't need to do any templating if it uses the primarily supported point types (as
> defined in include/pcl/point_types.h).
>
> Preliminary tests show that pcl_ros now compiles faster than pcl (ha!). Expect some serious tests tomorrow.

_______________________________________________
[hidden email] / http://pointclouds.org
https://code.ros.org/mailman/listinfo/pcl-users
Reply | Threaded
Open this post in threaded view
|

Re: no commit

Troy Straszheim
In reply to this post by Radu B. Rusu
On Mon, Dec 6, 2010 at 3:18 AM, Radu Bogdan Rusu <[hidden email]> wrote:
>
> The good news is that Troy's idea worked 100%!

You could have left out the exclamation point, oh ye of little faith...   :)

I think there is more performance to be had.  I seem to remember you
were playing with template specializations while trying to speed
things up... as mentioned, these aren't going to help, as they
increase the amount of code that needs to be compiled and have no
effect on the number of types overall...   so I'm wondering why
pcl/filters/filter.h has a specialization for sensor_msgs::PointCloud2
that (at a glance) doesn't seem to differ from the default...  it
looks like all this does is double the amount of code that needs
parsing.

-t
_______________________________________________
[hidden email] / http://pointclouds.org
https://code.ros.org/mailman/listinfo/pcl-users
Reply | Threaded
Open this post in threaded view
|

Re: no commit

Troy Straszheim
In reply to this post by Geoffrey Biggs
On Mon, Dec 6, 2010 at 8:03 AM, Geoffrey Biggs
<[hidden email]> wrote:
> So... what was Troy's idea? :)

[top-post, tsk tsk]

Manage template instantiations manually by separating interface and
implementation.  Not publication-worthy, but not completely obvious
when dealing with templates.  Note the impl/X.hpp files under pcl/;
compare their contents to corresponding X.h files and to the explicit
instantiations in src/X.cpp.

-t


> On 06/12/10 20:18, Radu Bogdan Rusu wrote:
>> Please do not checkout or commit anything to the trunk of PCL until tomorrow morning (PST).
>>
>> I'm committing a big patch which might break some things, but I am doing it now so that we avoid conflicts. In the
>> future I'll branch and plan these changes better.
>>
>> The good news is that Troy's idea worked 100%! All GCC compiles now are < 500MB and blazingly fast! Even more, any
>> future code that links against PCL won't need to do any templating if it uses the primarily supported point types (as
>> defined in include/pcl/point_types.h).
>>
>> Preliminary tests show that pcl_ros now compiles faster than pcl (ha!). Expect some serious tests tomorrow.
>
> _______________________________________________
> [hidden email] / http://pointclouds.org
> https://code.ros.org/mailman/listinfo/pcl-users
>
_______________________________________________
[hidden email] / http://pointclouds.org
https://code.ros.org/mailman/listinfo/pcl-users
Reply | Threaded
Open this post in threaded view
|

Re: no commit

Troy Straszheim
In reply to this post by Radu B. Rusu
On Mon, Dec 6, 2010 at 3:18 AM, Radu Bogdan Rusu <[hidden email]> wrote:
> Even more, any
> future code that links against PCL won't need to do any templating if it uses the primarily supported point types (as
> defined in include/pcl/point_types.h).

Looking at this:

template void pcl::removeNaNFromPointCloud<pcl::PointXYZ> (const
pcl::PointCloud<pcl::PointXYZ> &, pcl::PointCloud<pcl::PointXYZ> &,
std::vector<int> &);
template void pcl::removeNaNFromPointCloud<pcl::PointXYZI> (const
pcl::PointCloud<pcl::PointXYZI> &, pcl::PointCloud<pcl::PointXYZI> &,
std::vector<int> &);
template void pcl::removeNaNFromPointCloud<pcl::PointXYZRGBA> (const
pcl::PointCloud<pcl::PointXYZRGBA> &,
pcl::PointCloud<pcl::PointXYZRGBA> &, std::vector<int> &);
template void pcl::removeNaNFromPointCloud<pcl::PointXYZRGB> (const
pcl::PointCloud<pcl::PointXYZRGB> &, pcl::PointCloud<pcl::PointXYZRGB>
&, std::vector<int> &);
template void pcl::removeNaNFromPointCloud<pcl::InterestPoint> (const
pcl::PointCloud<pcl::InterestPoint> &,
pcl::PointCloud<pcl::InterestPoint> &, std::vector<int> &);
template void pcl::removeNaNFromPointCloud<pcl::PointNormal> (const
pcl::PointCloud<pcl::PointNormal> &, pcl::PointCloud<pcl::PointNormal>
&, std::vector<int> &);
template void pcl::removeNaNFromPointCloud<pcl::PointXYZRGBNormal>
(const pcl::PointCloud<pcl::PointXYZRGBNormal> &,
pcl::PointCloud<pcl::PointXYZRGBNormal> &, std::vector<int> &);
template void pcl::removeNaNFromPointCloud<pcl::PointXYZINormal>
(const pcl::PointCloud<pcl::PointXYZINormal> &,
pcl::PointCloud<pcl::PointXYZINormal> &, std::vector<int> &);
template void pcl::removeNaNFromPointCloud<pcl::PointWithRange> (const
pcl::PointCloud<pcl::PointWithRange> &,
pcl::PointCloud<pcl::PointWithRange> &, std::vector<int> &);
template void pcl::removeNaNFromPointCloud<pcl::PointWithViewpoint>
(const pcl::PointCloud<pcl::PointWithViewpoint> &,
pcl::PointCloud<pcl::PointWithViewpoint> &, std::vector<int> &);

I suggest we come up with a mechanism to replace that with something like:

#define INSTANTIATE(X) template void
pcl::removeNanFromPointCloud<X>(const pcl::PointCloud<X>&,
pcl::PointCloud<X>&, std::vector<int>&);

#include <pcl/impl/instantiate.hpp>

that is:

1.  the list of point types for which pcl is instantiated should be
maintained centrally:  adding a point type should be a matter of
changing one line of code (probably in a preprocessor list) and
recompiling

2. users should have access to this mechanism;  it should be
straightforward to instantiate everything for some non-pcl point type.

I'll prototype something....

-t
_______________________________________________
[hidden email] / http://pointclouds.org
https://code.ros.org/mailman/listinfo/pcl-users
Reply | Threaded
Open this post in threaded view
|

Re: no commit

Radu B. Rusu
Administrator
Morgen :) (sorry my brain's messed up after sleeping less than 7 hours - espresso time!)


On 12/06/2010 10:12 AM, Troy Straszheim wrote:

> On Mon, Dec 6, 2010 at 3:18 AM, Radu Bogdan Rusu<[hidden email]>  wrote:
>> Even more, any
>> future code that links against PCL won't need to do any templating if it uses the primarily supported point types (as
>> defined in include/pcl/point_types.h).
> I suggest we come up with a mechanism to replace that with something like:
>
> #define INSTANTIATE(X) template void
> pcl::removeNanFromPointCloud<X>(const pcl::PointCloud<X>&,
> pcl::PointCloud<X>&, std::vector<int>&);
>
> #include<pcl/impl/instantiate.hpp>

Troy, excellent suggestion!

Let's add it in today.



Cheers,
Radu.
--
http://pointclouds.org
_______________________________________________
[hidden email] / http://pointclouds.org
https://code.ros.org/mailman/listinfo/pcl-users
Reply | Threaded
Open this post in threaded view
|

Re: no commit

Radu B. Rusu
Administrator
In reply to this post by Troy Straszheim

On 12/06/2010 09:50 AM, Troy Straszheim wrote:
> On Mon, Dec 6, 2010 at 3:18 AM, Radu Bogdan Rusu<[hidden email]>  wrote:
>>
>> The good news is that Troy's idea worked 100%!
>
> You could have left out the exclamation point, oh ye of little faith...   :)

:) And the crowds bowed to him, as he was their true compilation savior... :)

> I think there is more performance to be had.  I seem to remember you
> were playing with template specializations while trying to speed
> things up... as mentioned, these aren't going to help, as they
> increase the amount of code that needs to be compiled and have no
> effect on the number of types overall...   so I'm wondering why
> pcl/filters/filter.h has a specialization for sensor_msgs::PointCloud2
> that (at a glance) doesn't seem to differ from the default...  it
> looks like all this does is double the amount of code that needs
> parsing.

Okay, that is not so much _just_ a specialization, as the code inside is completely different. While all the <PointT>
instantiations deal with a native PCL point type -- which means they can access .x, .y, .z directly, the
<sensor_msgs::PointCloud2> specialization deals with the binary blob directly and performs all those nasty memcpys inside.

The reason for all Filters to behave like this, is that they are generally useful for getting an arbitrary PointCloud2
message with any fields inside, doing something (e.g. removing some noise), and returning a PointCloud2 message back
with the same fields. So basically, we're "agnostic" to the data type inside of the cloud, as long as it has some
minimal set of fields (most require x/y/z to operate).

The Filter<PointT> already assumes that the data is coming in as PointT, which means that someone in the chain needs to
convert from PointCloud2->PointT a priori, thus dropping and losing the other fields (unless by some miracle, all fields
in PointCloud2 match the PointT type exactly).


Cheers,
Radu.
--
http://pointclouds.org
_______________________________________________
[hidden email] / http://pointclouds.org
https://code.ros.org/mailman/listinfo/pcl-users
Reply | Threaded
Open this post in threaded view
|

Re: no commit

Troy Straszheim
On Mon, Dec 6, 2010 at 10:37 AM, Radu Bogdan Rusu <[hidden email]> wrote:
>
> Okay, that is not so much _just_ a specialization, as the code inside is
> completely different.

whoops, sorry for the noise...

-t
_______________________________________________
[hidden email] / http://pointclouds.org
https://code.ros.org/mailman/listinfo/pcl-users
Reply | Threaded
Open this post in threaded view
|

Re: no commit

Radu B. Rusu
Administrator
In reply to this post by Radu B. Rusu
Thank you for your patience. If you do checkout trunk, you'll notice some big changes -- but at least we've woke up here
on the west coast so we can assist any compilation errors or reply any "wtf?" questions =)

As Troy already mentioned, we've changed the structure of the .HPP files a bit. Initially they were placed in the src/*
directory, as we thought "well, they're implementations, so all implementations should be placed there". To get that
working however we had to -Isrc/ which is really nasty. Besides, they're templated implementations.

The new code structure has them placed in impl/*, close to the header files.


The big compilation speedups (we're not done refactoring everything yet, so you'll notice more changes today/tomorrow)
come from:

  * the user (e.g., pcl_ros, pcl_visualization, <your code goes here>) was including the PCL header files, which were
including the template implementations;

  * the template implementations now are completely hidden and if guarded, and included only from the CPP src files...

  * ... where we instantiate concrete T types for all types in point_types.h.

  * the user code now will only look at a forward declaration when including the PCL header files (no more HPP
inclusion), and all the magic is transfered to linker.

This means that we only need to compile and create the object pcl::Foo<pcl::Bar> _once_, at the PCL compile time, and
then we're done forever! =)

Preliminary tests yesterday night (though we shouldn't trust them, lack of sleep is known to produce illusions and
dementia ;) ) showed that including standard ROS code (message_filters, tf, dynamic_reconfigure, etc) takes now an order
of magnitude more to compile than any PCL code. We'll verify this carefully today and if true, it's truly a great
compilation success story. PCL vs GCC : 1 - 1?

Cheers,
Radu.
--
http://pointclouds.org

On 12/06/2010 03:18 AM, Radu Bogdan Rusu wrote:

> Please do not checkout or commit anything to the trunk of PCL until tomorrow morning (PST).
>
> I'm committing a big patch which might break some things, but I am doing it now so that we avoid conflicts. In the
> future I'll branch and plan these changes better.
>
> The good news is that Troy's idea worked 100%! All GCC compiles now are < 500MB and blazingly fast! Even more, any
> future code that links against PCL won't need to do any templating if it uses the primarily supported point types (as
> defined in include/pcl/point_types.h).
>
> Preliminary tests show that pcl_ros now compiles faster than pcl (ha!). Expect some serious tests tomorrow.
>
> Cheers,
> Radu.
> --
> http://pointclouds.org
_______________________________________________
[hidden email] / http://pointclouds.org
https://code.ros.org/mailman/listinfo/pcl-users
Reply | Threaded
Open this post in threaded view
|

Re: no commit

Troy Straszheim
In reply to this post by Radu B. Rusu
On Mon, Dec 6, 2010 at 10:31 AM, Radu Bogdan Rusu <[hidden email]> wrote:
>> I suggest we come up with a mechanism to replace that with something like:
>>
>> #define INSTANTIATE(X) template void
>> pcl::removeNanFromPointCloud<X>(const pcl::PointCloud<X>&,
>> pcl::PointCloud<X>&, std::vector<int>&);
>>
>> #include<pcl/impl/instantiate.hpp>
>

I pushed something to https://github.com/straszheim/tmp

It works like this.  Comments invited.

In each pcl template class U's impl/U.hpp, the class defines a macro
PCL_INSTANTIATE_U(POINT_TYPE), e.g.

  https://github.com/straszheim/tmp/blob/master/include/pcl/impl/Type.hpp

and in the src/U.cpp file, pcl/impl/instantiate.hpp is included:

  https://github.com/straszheim/tmp/blob/master/include/pcl/impl/instantiate.hpp

This file provides four things to the user:

1.  a preprocessor sequence of PCL templates that should be
instantiated with all point types, called PCL_TEMPLATES

2.  a preprocessor sequence of all 'standard' pcl point classes,
called PCL_POINT_TYPES

3.  a macro PCL_INSTANTIATE(TEMPLATE, TYPES) which instantiates
TEMPLATE with all types POINT_TYPES  by calling
PCL_INSTANTIATE_TEMPLATE(T)  for each T in TYPES.  This is for use by
classes internal to the library.  Example:

   https://github.com/straszheim/tmp/blob/master/src/Type.cpp

4.  a macro PCL_INSTANTIATE_ALL(POINT_TYPE, TEMPLATE_TYPES) which
instantiates each T in TEMPLATE_TYPES with POINT_TYPE via
PCL_INSTANTIATE_T(POINT_TYPE).  This could be used for point types
external to PCL, for instance:

   https://github.com/straszheim/tmp/blob/master/src/User.cpp

So:

To modify the list of 'builtin' pcl point types:  modify PCL_POINT_TYPES.

To add your point type MyPoint to your library that uses pcl, create a
.cpp file.  Therein:

1.  include all PCL impl headers (there should be a header that does
this for you, say <pcl/all.hpp>)
2.  include your MyPoint.hpp
3.  include impl/instantiate.hpp
4.  PCL_INSTANTIATE_ALL(MyPoint, PCL_TEMPLATES);

as in User.cpp

That's just a first cut at it.   Maybe it will be too much to
instantiate all of PCL for MyPoint in one translation unit...

-t
_______________________________________________
[hidden email] / http://pointclouds.org
https://code.ros.org/mailman/listinfo/pcl-users
Reply | Threaded
Open this post in threaded view
|

Re: no commit

Adam Leeper
I'm trying to run a node on the robot that runs fine on my desktop. It compiles, but fails on launch.

I have trunk of perception_pcl checked out and built on the robot. Is this error saying it can't find something built by the cminpack package in perception_pcl? Or another library? I tried doing sudo apt-get install libminpack1 on the robot, but no dice.

/u/aleeper/ros/overlays/sandbox/pr2_gripper_grasp_adjust/bin/grasp_adjust_server: error while loading shared libraries: libminpack.so: cannot open shared object file: No such file or directory

Thanks,
Adam




Adam Leeper
Stanford University
[hidden email]
719.358.3804


On Mon, Dec 6, 2010 at 1:32 PM, Troy Straszheim <[hidden email]> wrote:
On Mon, Dec 6, 2010 at 10:31 AM, Radu Bogdan Rusu <[hidden email]> wrote:
>> I suggest we come up with a mechanism to replace that with something like:
>>
>> #define INSTANTIATE(X) template void
>> pcl::removeNanFromPointCloud<X>(const pcl::PointCloud<X>&,
>> pcl::PointCloud<X>&, std::vector<int>&);
>>
>> #include<pcl/impl/instantiate.hpp>
>

I pushed something to https://github.com/straszheim/tmp

It works like this.  Comments invited.

In each pcl template class U's impl/U.hpp, the class defines a macro
PCL_INSTANTIATE_U(POINT_TYPE), e.g.

 https://github.com/straszheim/tmp/blob/master/include/pcl/impl/Type.hpp

and in the src/U.cpp file, pcl/impl/instantiate.hpp is included:

 https://github.com/straszheim/tmp/blob/master/include/pcl/impl/instantiate.hpp

This file provides four things to the user:

1.  a preprocessor sequence of PCL templates that should be
instantiated with all point types, called PCL_TEMPLATES

2.  a preprocessor sequence of all 'standard' pcl point classes,
called PCL_POINT_TYPES

3.  a macro PCL_INSTANTIATE(TEMPLATE, TYPES) which instantiates
TEMPLATE with all types POINT_TYPES  by calling
PCL_INSTANTIATE_TEMPLATE(T)  for each T in TYPES.  This is for use by
classes internal to the library.  Example:

  https://github.com/straszheim/tmp/blob/master/src/Type.cpp

4.  a macro PCL_INSTANTIATE_ALL(POINT_TYPE, TEMPLATE_TYPES) which
instantiates each T in TEMPLATE_TYPES with POINT_TYPE via
PCL_INSTANTIATE_T(POINT_TYPE).  This could be used for point types
external to PCL, for instance:

  https://github.com/straszheim/tmp/blob/master/src/User.cpp

So:

To modify the list of 'builtin' pcl point types:  modify PCL_POINT_TYPES.

To add your point type MyPoint to your library that uses pcl, create a
.cpp file.  Therein:

1.  include all PCL impl headers (there should be a header that does
this for you, say <pcl/all.hpp>)
2.  include your MyPoint.hpp
3.  include impl/instantiate.hpp
4.  PCL_INSTANTIATE_ALL(MyPoint, PCL_TEMPLATES);

as in User.cpp

That's just a first cut at it.   Maybe it will be too much to
instantiate all of PCL for MyPoint in one translation unit...

-t


_______________________________________________
[hidden email] / http://pointclouds.org
https://code.ros.org/mailman/listinfo/pcl-users
Reply | Threaded
Open this post in threaded view
|

Re: no commit

Radu B. Rusu
Administrator
Adam, you need to rosmake your package or go into (roscd) cminpack and build the library.

Cheers,
Radu.
--
http://pointclouds.org

On 12/06/2010 08:25 PM, Adam Leeper wrote:

> I'm trying to run a node on the robot that runs fine on my desktop. It compiles, but fails on launch.
>
> I have trunk of perception_pcl checked out and built on the robot. Is this error saying it can't find something built by
> the cminpack package in perception_pcl? Or another library? I tried doing sudo apt-get install libminpack1 on the robot,
> but no dice.
>
> /u/aleeper/ros/overlays/sandbox/pr2_gripper_grasp_adjust/bin/grasp_adjust_server: error while loading shared libraries:
> libminpack.so: cannot open shared object file: No such file or directory
>
> Thanks,
> Adam
>
>
>
>
> Adam Leeper
> Stanford University
> [hidden email] <mailto:[hidden email]>
> 719.358.3804
>
>
> On Mon, Dec 6, 2010 at 1:32 PM, Troy Straszheim <[hidden email] <mailto:[hidden email]>> wrote:
>
>     On Mon, Dec 6, 2010 at 10:31 AM, Radu Bogdan Rusu <[hidden email] <mailto:[hidden email]>> wrote:
>      >> I suggest we come up with a mechanism to replace that with something like:
>      >>
>      >> #define INSTANTIATE(X) template void
>      >> pcl::removeNanFromPointCloud<X>(const pcl::PointCloud<X>&,
>      >> pcl::PointCloud<X>&, std::vector<int>&);
>      >>
>      >> #include<pcl/impl/instantiate.hpp>
>      >
>
>     I pushed something to https://github.com/straszheim/tmp
>
>     It works like this.  Comments invited.
>
>     In each pcl template class U's impl/U.hpp, the class defines a macro
>     PCL_INSTANTIATE_U(POINT_TYPE), e.g.
>
>     https://github.com/straszheim/tmp/blob/master/include/pcl/impl/Type.hpp
>
>     and in the src/U.cpp file, pcl/impl/instantiate.hpp is included:
>
>     https://github.com/straszheim/tmp/blob/master/include/pcl/impl/instantiate.hpp
>
>     This file provides four things to the user:
>
>     1.  a preprocessor sequence of PCL templates that should be
>     instantiated with all point types, called PCL_TEMPLATES
>
>     2.  a preprocessor sequence of all 'standard' pcl point classes,
>     called PCL_POINT_TYPES
>
>     3.  a macro PCL_INSTANTIATE(TEMPLATE, TYPES) which instantiates
>     TEMPLATE with all types POINT_TYPES  by calling
>     PCL_INSTANTIATE_TEMPLATE(T)  for each T in TYPES.  This is for use by
>     classes internal to the library.  Example:
>
>     https://github.com/straszheim/tmp/blob/master/src/Type.cpp
>
>     4.  a macro PCL_INSTANTIATE_ALL(POINT_TYPE, TEMPLATE_TYPES) which
>     instantiates each T in TEMPLATE_TYPES with POINT_TYPE via
>     PCL_INSTANTIATE_T(POINT_TYPE).  This could be used for point types
>     external to PCL, for instance:
>
>     https://github.com/straszheim/tmp/blob/master/src/User.cpp
>
>     So:
>
>     To modify the list of 'builtin' pcl point types:  modify PCL_POINT_TYPES.
>
>     To add your point type MyPoint to your library that uses pcl, create a
>     .cpp file.  Therein:
>
>     1.  include all PCL impl headers (there should be a header that does
>     this for you, say <pcl/all.hpp>)
>     2.  include your MyPoint.hpp
>     3.  include impl/instantiate.hpp
>     4.  PCL_INSTANTIATE_ALL(MyPoint, PCL_TEMPLATES);
>
>     as in User.cpp
>
>     That's just a first cut at it.   Maybe it will be too much to
>     instantiate all of PCL for MyPoint in one translation unit...
>
>     -t
>     _______________________________________________
>     [hidden email] <mailto:[hidden email]> / http://pointclouds.org
>     https://code.ros.org/mailman/listinfo/pcl-users
>
>
_______________________________________________
[hidden email] / http://pointclouds.org
https://code.ros.org/mailman/listinfo/pcl-users
Reply | Threaded
Open this post in threaded view
|

Re: no commit

Troy Straszheim
On Mon, Dec 6, 2010 at 9:56 PM, Radu Bogdan Rusu <[hidden email]> wrote:
> Adam, you need to rosmake your package or go into (roscd) cminpack and build the library.
>

This kind of thing should be filed as a buildsystem bug.  Not that it
is ever actually going to get fixed, but at least we'd have a list of
gotchas and annoyances.  [looks at trac]  do we even have a rosbuild
category/component to fie these under?

-t
_______________________________________________
[hidden email] / http://pointclouds.org
https://code.ros.org/mailman/listinfo/pcl-users
Reply | Threaded
Open this post in threaded view
|

Re: no commit

Brian Gerkey
On Tue, Dec 7, 2010 at 9:04 AM, Troy Straszheim
<[hidden email]> wrote:
> On Mon, Dec 6, 2010 at 9:56 PM, Radu Bogdan Rusu <[hidden email]> wrote:
>> Adam, you need to rosmake your package or go into (roscd) cminpack and build the library.
>>
>
> This kind of thing should be filed as a buildsystem bug.  Not that it
> is ever actually going to get fixed, but at least we'd have a list of
> gotchas and annoyances.  [looks at trac]  do we even have a rosbuild
> category/component to fie these under?

Yep, you can file tickets against rosbuild:
  https://code.ros.org/trac/ros/newticket?component=rosbuild
But usually this kind of problem is actually a bug in the package's
Makefile, not rosbuild.

        brian.
_______________________________________________
[hidden email] / http://pointclouds.org
https://code.ros.org/mailman/listinfo/pcl-users
Reply | Threaded
Open this post in threaded view
|

Re: no commit

Radu B. Rusu
Administrator

On 12/07/2010 09:11 AM, Brian Gerkey wrote:

> On Tue, Dec 7, 2010 at 9:04 AM, Troy Straszheim
> <[hidden email]>  wrote:
>> On Mon, Dec 6, 2010 at 9:56 PM, Radu Bogdan Rusu<[hidden email]>  wrote:
>>> Adam, you need to rosmake your package or go into (roscd) cminpack and build the library.
>>>
>>
>> This kind of thing should be filed as a buildsystem bug.  Not that it
>> is ever actually going to get fixed, but at least we'd have a list of
>> gotchas and annoyances.  [looks at trac]  do we even have a rosbuild
>> category/component to fie these under?
>
> Yep, you can file tickets against rosbuild:
>    https://code.ros.org/trac/ros/newticket?component=rosbuild
> But usually this kind of problem is actually a bug in the package's
> Makefile, not rosbuild.

Let's hear it from Adam if rosmaking his package worked. It might only be system related if he had an old tree, did an
svn-up, called rosmake but without pre-clean and that didn't rebuild some of these 3rd party dependencies.

I think the 3rd party dependencies get built only if package/installed doesn't exist - so the Makefile is not really
checked. Am I right Brian?

Cheers,
Radu.
--
http://pointclouds.org
_______________________________________________
[hidden email] / http://pointclouds.org
https://code.ros.org/mailman/listinfo/pcl-users
Reply | Threaded
Open this post in threaded view
|

Re: no commit

Troy Straszheim
In reply to this post by Brian Gerkey
On Tue, Dec 7, 2010 at 9:11 AM, Brian Gerkey <[hidden email]> wrote:

> On Tue, Dec 7, 2010 at 9:04 AM, Troy Straszheim
> <[hidden email]> wrote:
>> On Mon, Dec 6, 2010 at 9:56 PM, Radu Bogdan Rusu <[hidden email]> wrote:
>>> Adam, you need to rosmake your package or go into (roscd) cminpack and build the library.
>>>
>>
>> This kind of thing should be filed as a buildsystem bug.  Not that it
>> is ever actually going to get fixed, but at least we'd have a list of
>> gotchas and annoyances.  [looks at trac]  do we even have a rosbuild
>> category/component to fie these under?
>
> Yep, you can file tickets against rosbuild:
>  https://code.ros.org/trac/ros/newticket?component=rosbuild
> But usually this kind of problem is actually a bug in the package's
> Makefile, not rosbuild.

I guess I'm missing something here.  Typically, I specify to the build
system that my libX is dependent on libY.  If libY is neither
something that I can find on the system nor something that I know how
to build, there is a hard error: can't continue.  If there is no
error, then when asked to build libX, the buildsystem should link
against libY, first verifying that libY is up-to-date if it is a
target.   Maybe the 'bug' that I think I'm seeing is that the makefile
error is not caught earlier or at least reported as "you didn't run
rosmake".

Is the problem that the user ran 'make' instead of 'rosmake'?

-t
_______________________________________________
[hidden email] / http://pointclouds.org
https://code.ros.org/mailman/listinfo/pcl-users
Reply | Threaded
Open this post in threaded view
|

Re: no commit

Radu B. Rusu
Administrator

On 12/07/2010 10:46 AM, Troy Straszheim wrote:

> On Tue, Dec 7, 2010 at 9:11 AM, Brian Gerkey<[hidden email]>  wrote:
>> On Tue, Dec 7, 2010 at 9:04 AM, Troy Straszheim
>> <[hidden email]>  wrote:
>>> On Mon, Dec 6, 2010 at 9:56 PM, Radu Bogdan Rusu<[hidden email]>  wrote:
>>>> Adam, you need to rosmake your package or go into (roscd) cminpack and build the library.
>>>>
>>>
>>> This kind of thing should be filed as a buildsystem bug.  Not that it
>>> is ever actually going to get fixed, but at least we'd have a list of
>>> gotchas and annoyances.  [looks at trac]  do we even have a rosbuild
>>> category/component to fie these under?
>>
>> Yep, you can file tickets against rosbuild:
>>   https://code.ros.org/trac/ros/newticket?component=rosbuild
>> But usually this kind of problem is actually a bug in the package's
>> Makefile, not rosbuild.
>
> I guess I'm missing something here.  Typically, I specify to the build
> system that my libX is dependent on libY.  If libY is neither
> something that I can find on the system nor something that I know how
> to build, there is a hard error: can't continue.  If there is no
> error, then when asked to build libX, the buildsystem should link
> against libY, first verifying that libY is up-to-date if it is a
> target.   Maybe the 'bug' that I think I'm seeing is that the makefile
> error is not caught earlier or at least reported as "you didn't run
> rosmake".
>
> Is the problem that the user ran 'make' instead of 'rosmake'?


I think Brian is right, the problem in this case (though I'm not sure if Adam fixed it yet, and if so how - or what the
error really was) is not necessarily rosbuild but the 3rd party Makefile.

Basically we have very nice CMake files for all our packages, and a ton of different variations of regular Makefiles for
3rd party dependencies. Hard to clean these up in one unified way (I think), as they all require different things, so
all we can do is make sure that each individual such metapackage has a decent Makefile, patched, verified, etc.

Cheers,
Radu.
--
http://pointclouds.org
_______________________________________________
[hidden email] / http://pointclouds.org
https://code.ros.org/mailman/listinfo/pcl-users
Reply | Threaded
Open this post in threaded view
|

Re: no commit

Brian Gerkey
In reply to this post by Troy Straszheim
On Tue, Dec 7, 2010 at 10:46 AM, Troy Straszheim
<[hidden email]> wrote:

> I guess I'm missing something here.  Typically, I specify to the build
> system that my libX is dependent on libY.  If libY is neither
> something that I can find on the system nor something that I know how
> to build, there is a hard error: can't continue.  If there is no
> error, then when asked to build libX, the buildsystem should link
> against libY, first verifying that libY is up-to-date if it is a
> target.   Maybe the 'bug' that I think I'm seeing is that the makefile
> error is not caught earlier or at least reported as "you didn't run
> rosmake".
>
> Is the problem that the user ran 'make' instead of 'rosmake'?

That's very possible.  As it stands (i.e., using rosbuild v1), 'make'
in a package will never try to build anything outside that package.
It will track dependencies, in the sense that if an archive lib is
new, then executables using it in the present package will be
relinked.   But it won't go into that other package and rebuild
anything.  That's what rosmake is for.

        brian.
_______________________________________________
[hidden email] / http://pointclouds.org
https://code.ros.org/mailman/listinfo/pcl-users
Reply | Threaded
Open this post in threaded view
|

Re: no commit

Troy Straszheim
On Tue, Dec 7, 2010 at 11:19 AM, Brian Gerkey <[hidden email]> wrote:

>>
>> Is the problem that the user ran 'make' instead of 'rosmake'?
>
> That's very possible.  As it stands (i.e., using rosbuild v1), 'make'
> in a package will never try to build anything outside that package.
> It will track dependencies, in the sense that if an archive lib is
> new, then executables using it in the present package will be
> relinked.   But it won't go into that other package and rebuild
> anything.  That's what rosmake is for.
>

I should have said filed as a "please fix in rosbuild 2" rather than
"bug" if rosbuild is doing what it is designed to do, sorry for the
noise.   Better yet, I'll just add it to the rosbuild2 wiki page...

-t
_______________________________________________
[hidden email] / http://pointclouds.org
https://code.ros.org/mailman/listinfo/pcl-users
Reply | Threaded
Open this post in threaded view
|

Re: no commit

Radu B. Rusu
Administrator
In reply to this post by Radu B. Rusu

On 12/07/2010 11:12 AM, Radu Bogdan Rusu wrote:

>
> On 12/07/2010 10:46 AM, Troy Straszheim wrote:
>> On Tue, Dec 7, 2010 at 9:11 AM, Brian Gerkey<[hidden email]> wrote:
>>> On Tue, Dec 7, 2010 at 9:04 AM, Troy Straszheim
>>> <[hidden email]> wrote:
>>>> On Mon, Dec 6, 2010 at 9:56 PM, Radu Bogdan Rusu<[hidden email]> wrote:
>>>>> Adam, you need to rosmake your package or go into (roscd) cminpack and build the library.
>>>>>
>>>>
>>>> This kind of thing should be filed as a buildsystem bug. Not that it
>>>> is ever actually going to get fixed, but at least we'd have a list of
>>>> gotchas and annoyances. [looks at trac] do we even have a rosbuild
>>>> category/component to fie these under?

Okay, here's what happened:

  * there is a [cminpack] _package_ part of the [point_cloud_perception] _stack_, installed on everyone's C-Turtle in
/opt/ros/.../

  * Adam overlayed a [tabletop_object_perception] package from source that uses [cminpack], and compiled it. ldd on
[tabletop_object_perception]/foo.so was hooked up to [cminpack]/libminpack.so

  * another overlay was added on top with the new [perception_pcl] stack, which contains a newer [cminpack], where the
authors changed the name of the library from libminpack.so to libcminpack.so

  * when compiling another project that depends on [tabletop_object_perception], foo.so was complaining about:
/usr/bin/ld: warning: libminpack.so, needed by /.../foo.so, not found (try using -rpath or -rpath-link)

That makes sense, as rospack was now finding the new overlayed [cminpack] package that had a libcminpack.so exported and
not a libminpack.so one.

The solution was simple: recompile [tabletop_object_perception].

Cheers,
Radu.
--
http://pointclouds.org
_______________________________________________
[hidden email] / http://pointclouds.org
https://code.ros.org/mailman/listinfo/pcl-users