brechtsanders / xlsxio

XLSX I/O - C library for reading and writing .xlsx files
MIT License
397 stars 113 forks source link

missing CMake package config file #101

Open jakoch opened 2 years ago

jakoch commented 2 years ago

Hey! Please provide a CMake package config file for easier pickup of your library. Resolving this issue would help a lot for picking the library up, because currently one has to write a custom finder.

CMake Warning for this issue:

By not providing "Findxlsxio_read.cmake" in CMAKE_MODULE_PATH this project
  has asked CMake to find a package configuration file provided by
  "xlsxio_read", but CMake did not find one.

  Could not find a package configuration file provided by "xlsxio_read" with
  any of the following names:

    xlsxio_readConfig.cmake
    xlsxio_read-config.cmake

  Add the installation prefix of "xlsxio_read" to CMAKE_PREFIX_PATH or set
  "xlsxio_read_DIR" to a directory containing one of the above files.  If
  "xlsxio_read" provides a separate development package or SDK, be sure it
  has been installed.
playgithub commented 2 years ago

cmake code to find xlsxio

find_package(PkgConfig)
set(ENV{PKG_CONFIG_PATH} /usr/local/lib/pkgconfig)
pkg_check_modules(XLSXIO_READ REQUIRED libxlsxio_read)
message(XLSXIO_READ_INCLUDE_DIRS: ${XLSXIO_READ_INCLUDE_DIRS})
message(XLSXIO_READ_LIBRARIES: ${XLSXIO_READ_LIBRARIES})

Output

XLSXIO_READ_INCLUDE_DIRS:/usr/local/include
XLSXIO_READ_LIBRARIES:libxlsxio_readlibminiziplibzlibexpat

Problem

For XLSXIO_READ_LIBRARIES, names like libxlsxio_read leads error for target_link_libraries. Without the prefix lib, i.e. xlsxio_read, it works.

brechtsanders commented 2 years ago

Why do you set(ENV{PKG_CONFIG_PATH} /usr/local/lib/pkgconfig)? That would override what people already have in the PKG_CONFIG_PATH environment variable...

The name of he library to find with pkg_check_modules depends in the name of the .pc file. Do you have the .pc filesnamed different than libxlsxio_read.pc and libxlsxio_write.pc?

playgithub commented 2 years ago

Why do you set(ENV{PKG_CONFIG_PATH} /usr/local/lib/pkgconfig)? That would override what people already have in the PKG_CONFIG_PATH environment variable...

On manjaro, I've tried without set(ENV{PKG_CONFIG_PATH} /usr/local/lib/pkgconfig), pkg_check_modules can't find the lib.

The name of he library to find with pkg_check_modules depends in the name of the .pc file. Do you have the .pc filesnamed different than libxlsxio_read.pc and libxlsxio_write.pc?

No, nothing modified.

brechtsanders commented 2 years ago

I made a first attempt to generate proper .cmake files and fixed an issue with the .pc files. Can you try if these changes work for you?

playgithub commented 2 years ago

I made a first attempt to generate proper .cmake files and fixed an issue with the .pc files. Can you try if these changes work for you?

For some reason, I have installed windows. I've tried, but the dependency for minizip / zlib needs to be solved. I'm doing it now.

playgithub commented 2 years ago

tested on raspberry pi, it failed, logs as below

$ cat ../CMakeLists.txt
cmake_minimum_required(VERSION 3.16)

project(test_xlsxio
        LANGUAGES CXX
        VERSION 1.0.0.0)

find_package(libxlsxio_read REQUIRED)
$ cmake .. -DCMAKE_BUILD_TYPE=Release
CMake Error at /usr/local/cmake/libxlsxio_readConfig.cmake:11 (MESSAGE):
  cannot find xlsxio_read library in /lib
Call Stack (most recent call first):
  CMakeLists.txt:7 (find_package)

-- Configuring incomplete, errors occurred!
See also "/home/aaa/test/test_xlsxio/out/CMakeFiles/CMakeOutput.log".

@jakoch Could you help test it? @brechtsanders For the custom find cmake scripts for minizip & zlib & expat, the libs can't be found it they are not in conventional paths. In my practice, not convenient for Windows and conan, better if using find_package in a standard way.

brechtsanders commented 2 years ago

Sorry, there was a bug in XLSXIOREAD_ROOT which should now be fixed.

jakoch commented 2 years ago

@playgithub My project uses vcpkg to pull in xlsxio and it's version is fixed to e3acace39 of this repo, see portfile. But i'll try to pull in head for testing the latest changes to package finding.

playgithub commented 2 years ago

@brechtsanders pulled and tested

$ cmake .. -DCMAKE_BUILD_TYPE=Release
CMake Error at /usr/local/cmake/libxlsxio_readConfig.cmake:5 (PKG_CHECK_MODULES):
  Unknown CMake command "PKG_CHECK_MODULES".
Call Stack (most recent call first):
  CMakeLists.txt:7 (find_package)
brechtsanders commented 2 years ago

Can you try again now?

playgithub commented 2 years ago

Succeeded for the code below

cmake_minimum_required(VERSION 3.16)

project(test_xlsxio
        LANGUAGES CXX
        VERSION 1.0.0.0)

find_package(libxlsxio_read REQUIRED)
playgithub commented 2 years ago

find_package(xlsxio REQUIRED) is expected to work when it is installed at /opt/xlsxio, but it doesn't. This is a default behaivor for common libs.

BTW by modern cmake, it is quite easy to implement.

brechtsanders commented 2 years ago

On which platform(s) is this standard behaviour? Homebrew? Should it then look for the libraries in /opt/xlsxio/lib and the includes in in /opt/xlsxio/include?

playgithub commented 2 years ago

On which platform(s) is this standard behaviour? Homebrew?

Linux

find_package's search paths: https://cmake.org/cmake/help/latest/command/find_package.html#search-procedure

On Manjaro, CMAKE_SYSTEM_PREFIX_PATH is

/usr
/
/usr
/usr/local
/usr/X11R6
/usr/pkg
/opt

Should it then look for the libraries in /opt/xlsxio/lib and the includes in in /opt/xlsxio/include?

Yes

BTW, more conveninet if libxlsxio_write and libxlsxio_read as componts of xlsxio, and for auto-generated *Config.cmake, name matters, e.g. find_package(xlsxio REQUIRED) need a file named xlsxioConfig.cmake / xlsxio-config.cmake

brechtsanders commented 2 years ago

Documentation says cmake will look in all paths listed in CMAKE_SYSTEM_PREFIX_PATH, so if you install under /usr/local (or /opt) they way it is normally done, and not under an additional xlsxio folder in there, it should work.

But I added the search in /opt/xlsxio just now so you can try it.

playgithub commented 2 years ago

Documentation says cmake will look in all paths listed in CMAKE_SYSTEM_PREFIX_PATH, so if you install under /usr/local (or /opt) they way it is normally done, and not under an additional xlsxio folder in there, it should work.

Not exactly, please check the search paths in the doc (https://cmake.org/cmake/help/latest/command/find_package.html#search-procedure), some that begins with <prefix>/<name>*/ supports a lib name as an additional prefix.

But I added the search in /opt/xlsxio just now so you can try it.

It is a default behavior and needs no manual works. However for xlsxio, the real libs are xlsxio_write and xlsxio_read, so it needs extra works. If let xlsxio_write and xlsxio_read be component of lib xlsxio, it'll be more convenient. When I have free time, I'd like to refactor the cmake scripts to modern cmake.

brechtsanders commented 2 years ago

From what I read there adding NAMES xlsxio to find_package() might do the trick. Can you try this?

find_package(libxlsxio_read REQUIRED NAMES xlsxio)

Or if that doesn't work maybe:

find_package(libxlsxio_read REQUIRED NAMES libxlsxio_read xlsxio)
playgithub commented 2 years ago
find_package(libxlsxio_read REQUIRED NAMES xlsxio)

Not working

find_package(libxlsxio_read REQUIRED NAMES libxlsxio_read xlsxio)

Not working

playgithub commented 2 years ago

Keep it simple I'm going to use conan for encapsulation