cxxopts icon indicating copy to clipboard operation
cxxopts copied to clipboard

fix compatibility with find package

Open andre-nguyen opened this issue 5 years ago • 8 comments

This preserves behaviour with the add_subdirectory stuff.


This change is Reviewable

andre-nguyen avatar Jun 15 '19 21:06 andre-nguyen

Can you explain the problem that you're fixing here?

jarro2783 avatar Jun 16 '19 22:06 jarro2783

Oh sorry about the lack of details. This PR fixes 2 problems and adds 1 feature.

  1. Problem: the added export(PACKAGE ...) line now actually exports cxxopts to the user package registry (~/.cmake/packages/ on linux) that way it is find_package-able by other projects.
  2. Problem: With a project structure such as:
|-- Project
     -- CmakeLists.txt
            add_subdirectory(cxxopts)
            add_subdirectory(liba)
            add_subdirectory(libb)
    |-- liba
        -- CmakeLists.txt (find_package(cxxopts REQUIRED)
    |-- libb
        -- CmakeLists.txt (find_package(cxxopts REQUIRED)
    |-- cxxopts
        -- CmakeLists.txt

Where two parts of a project can depend on cxxopts and cxxopts is embedded in the library. With the current master commit I couldn't get my libraries to find_package cxxopts. Now it works and it preserves compatibility with the "regular" approach where cxxopts is a subdirectory of the main project rather than a sister directory.

  1. New feature: With the project prefix based on the project version, multiple versions of the library can be installed.

andre-nguyen avatar Jun 17 '19 05:06 andre-nguyen

I don't know how all of this works because I'm not very familiar with cmake, but something doesn't look right here:

jarryd@guepardo ~/current/soft/build/cxxopts $ cmake . -DCMAKE_INSTALL_PREFIX=$HOME/local
-- cxxopts version 2.2.0
-- Configuring done
-- Generating done
-- Build files have been written to: /home/jarryd/current/soft/build/cxxopts
jarryd@guepardo ~/current/soft/build/cxxopts $ ninja install
[0/1] Install the project...
-- Install configuration: "Debug"
-- Installing: /cxxopts-2.2.0/cmake/cxxopts-targets.cmake
-- Installing: /home/jarryd/local/lib/cmake/cxxopts/cxxopts-config.cmake
-- Installing: /home/jarryd/local/lib/cmake/cxxopts/cxxopts-config-version.cmake
-- Installing: /home/jarryd/local/lib/cmake/cxxopts/cxxopts-targets.cmake
-- Installing: /cxxopts-2.2.0/cxxopts/cxxopts.hpp

jarro2783 avatar Jun 17 '19 22:06 jarro2783

Just to confirm, do you mean these two lines? -- Installing: /cxxopts-2.2.0/cmake/cxxopts-targets.cmake -- Installing: /cxxopts-2.2.0/cxxopts/cxxopts.hpp Indeed that path looks incomplete.

andre-nguyen avatar Jun 18 '19 14:06 andre-nguyen

Yes. I don't expect something to be installed in the root directory when giving a prefix in my home directory. In general an arbitrary user probably can't do that anyway.

jarro2783 avatar Jun 18 '19 22:06 jarro2783

I had forgotten the line

include(GNUInstallDirs)

Which defines the CMAKE_INSTALL_LIBDIR variable. It works on my side now, can you confirm?

andre@neptune:~/Documents/personal/cxxopts/build$ cmake .. -DCMAKE_INSTALL_PREFIX=${PWD}/install
-- cxxopts version 2.2.0
-- Configuring done
-- Generating done
-- Build files have been written to: /home/andre/Documents/personal/cxxopts/build
andre@neptune:~/Documents/personal/cxxopts/build$ make install
[ 25%] Built target example
[ 62%] Built target link_test
[100%] Built target options_test
Install the project...
-- Install configuration: ""
-- Installing: /home/andre/Documents/personal/cxxopts/build/install/lib/cxxopts-2.2.0/cmake/cxxopts-targets.cmake
-- Installing: /home/andre/Documents/personal/cxxopts/build/install/lib/cmake/cxxopts/cxxopts-config.cmake
-- Installing: /home/andre/Documents/personal/cxxopts/build/install/lib/cmake/cxxopts/cxxopts-config-version.cmake
-- Installing: /home/andre/Documents/personal/cxxopts/build/install/lib/cmake/cxxopts/cxxopts-targets.cmake
-- Installing: /home/andre/Documents/personal/cxxopts/build/install/include/cxxopts-2.2.0/cxxopts/cxxopts.hpp

andre-nguyen avatar Jun 22 '19 04:06 andre-nguyen

@jarro2783 Anything I can do to help this get merged in?

andre-nguyen avatar Dec 02 '19 01:12 andre-nguyen

Sorry I forgot about this one. Can you rebase first?

jarro2783 avatar Dec 02 '19 06:12 jarro2783