Pcl: FLANN_INCLUDE_DIRS is empty from this change #3202

[UnaNancyOwen]

FLANN_INCLUDE_DIRS is remove in this change #3202.
Therefor, cmake/pcl_all_in_one_installer.cmake#L14 is empty.
It means that the copy files of 3rdParty/FLANN fails during installation.

https://github.com/PointCloudLibrary/pcl/blob/2785b03cae3cff19262e96ebc1b73e37d85af3f3/cmake/pcl_all_in_one_installer.cmake#L14

There are two solutions to this issue.

1. Change to FLANN_INCLUDE_DIR from FLANN_INCLUDE_DIRS

# pcl_all_in_one_installer.cmake#L14
- get_filename_component(FLANN_ROOT "${FLANN_INCLUDE_DIRS}" PATH)
+ get_filename_component(FLANN_ROOT "${FLANN_INCLUDE_DIR}" PATH)

1. Add FLANN_INCLUDE_DIRS in cmake/Modules/FindFLANN.cmake

+ set(FLANN_INCLUDE_DIRS ${FLANN_INCLUDE_DIR})

Which do you think is good?

[UnaNancyOwen]

@taketwo Do you have a reason to removed FLANN_INCLUDE_DIRS in #3202?

[taketwo]

Hi, sorry for the delay. Reasons for removal:

1. New finder script supports both new (config-based) and old (pkgconfig) discovery methods. The new config-based method does not create such variable, so I would have needed to add extra code for that.
2. Generally, this variable is not needed because the consumers of FLANN only need to link to the imported target, includes will be set up automatically by CMake.

What I did not consider is the fact that this variable is needed for packaging. Unfortunately, none of your solutions are sufficient because of item 1. Could you explain why All-in-One installer needs this variable? As far as I can see, in PCLConfig it's hard-coded anyway:

https://github.com/PointCloudLibrary/pcl/blob/4320d936a6706e88da0d1b8fd26fa090eab574a8/PCLConfig.cmake.in#L223

[UnaNancyOwen]

Could you explain why All-in-One installer needs this variable?

It is nessesary to get dependencies root path when generating PCL All-in-one Installer.
It need to collect all files (for inclusion in installer) to one directory for generate installer.

# FLANN_INCLUDE_DIRS is C:/Program Files/flann/include
# FLANN_ROOT is C:/Program Files/flann
get_filename_component(FLANN_ROOT "${FLANN_INCLUDE_DIRS}" PATH)

...

# Copy all files in FLANN_ROOT to PCL_ROOT/3rdParty/FLANN
# Generate installer based file contained in PCL_ROOT

[taketwo]

Thanks. I will write some CMake code to extract FLANN_ROOT from an imported target. How urgent is this issue for you?

[UnaNancyOwen]

It is necessary before the next release.

[UnaNancyOwen]

I will write some CMake code to extract FLANN_ROOT from an imported target.

I think it is not necessary, because the following small changes will work correctory.
It is simple, I will send a pull request soon.

1. Change to FLANN_INCLUDE_DIR from FLANN_INCLUDE_DIRS

# pcl_all_in_one_installer.cmake#L14
- get_filename_component(FLANN_ROOT "${FLANN_INCLUDE_DIRS}" PATH)
+ get_filename_component(FLANN_ROOT "${FLANN_INCLUDE_DIR}" PATH)

[taketwo]

But there is not FLANN_INCLUDE_DIR variable defined if we go this path:

https://github.com/PointCloudLibrary/pcl/blob/2785b03cae3cff19262e96ebc1b73e37d85af3f3/cmake/Modules/FindFLANN.cmake#L36-L49

[UnaNancyOwen]

Oops, that's right.
However, This root was not processed in my environment, because flann_FOUND is NO. Why?

[taketwo]

Do you have FLANN master installed? If so, did you point CMake to this installation by providing flann_DIR variable?

[UnaNancyOwen]

Is it enable in master? (I'm using 1.9.1.)
I will try with master/HEAD instead of 1.9.1. :+1:

[taketwo]

Yes, this is a new feature in their master. We had to adapt to it even before a new FLANN version is released because vcpkg is already using the master.

[UnaNancyOwen]

I confirmed that it went through that route with FLANN master/HEAD.
It is necessary to fix FindFLANN.cmake as you say.

I will write some CMake code to extract FLANN_ROOT from an imported target.

Please write this code. Thanks,