ethz-asl / suitesparse

A catkin wrapper package for Dr. Tim Davis Suitesparse 4.2.1 (with PIC) http://faculty.cse.tamu.edu/davis/suitesparse.html
10 stars 13 forks source link

exported devel/include/suitesparse as include folder because that is whe... #20

Closed HannesSommer closed 10 years ago

HannesSommer commented 10 years ago

I'm not sure whether that is the proper way to do it. But that was for me the quickest way to have pgk-config know about devel/include/suitesparse and not only about src/suitesparse/include, which contains nothing.

simonlynen commented 10 years ago

I don't think this works in general unless you create the folder before running catkin. Catkin checks if the exported folders exist and fails otherwise. Did you verify this? We probably should also double check that the build server behaves correctly w.r.t. that.

furgalep commented 10 years ago

This also might not work if we run catkin build --install. I suspect lots of our hacks won't work in this case.

HannesSommer commented 10 years ago

Ah, ok, so what is then the right way? The current version, exporting "include" makes little sense as it is just empty. How can we fix that? This must have been solved for the other _catkin packages, or am I wrong? What is the point in having pgk-config files exported for the devel folder when they are not intended to work without build --install?

How does it work with catkin client packages? Does somebody know how do they get to know about devel/include/suitesparse, if they depend on suitesparse? Is that automatically without any export on suitesparse side?

simonlynen commented 10 years ago

We have some pre-cmake mkdirs in place. I think suitesparse has that.

On Wed, Jul 16, 2014 at 2:25 AM, HannesSommer notifications@github.com wrote:

Ah, ok, so what is then the right way? The current version, exporting "include" makes little sense as it is just empty. How can we fix that? This must have been solved for the other _catkin packages, or am I wrong? What is the point in having pgk-config files exported for the devel folder when they are not intended to work without build --install?

How does it work with catkin client packages? Does somebody know how do they get to know about devel/include/suitesparse, if they depend on suitesparse? Is that automatically without any export on suitesparse side?

— Reply to this email directly or view it on GitHub https://github.com/ethz-asl/suitesparse/pull/20#issuecomment-49140876.

HannesSommer commented 10 years ago

Simon, you're right. Catkin does currently fail in a fresh workspace with that hack.

Ah, I now understand how it works for catkin client packages : https://github.com/ethz-asl/suitesparse/blob/master/cmake/suitesparse-extras.cmake.in#L1

Hm, that does not seem to be the right way. The other wrapper packages do it differently and more compatible with pkg-config : I checked eige_catkin, ceres_catkin and protobuf_catkin.

I will soon commit corresponding changes for suitesparse for discussion ... They also all use catkin simple. Was there a special reason not to use catkin simple for suitesparse?

simonlynen commented 10 years ago

No I just used catkin for suitesparse and SchweizerMesser before I was at Google last summer. By that time catkin_simple did not exist. It was written last August/September.

HannesSommer commented 10 years ago

Ok, I've just added the file(MAKE_DIRECTORY ... ) and remove the include directory modification from suitesparse-extras.cmake.in. That works for me, including client packages like aslam_backend or sparse_block_matrix. I also cleaned manually a bit the jenkins workspace for suitesparse before I pushed. So the green there is quite meaningful :).