-
Notifications
You must be signed in to change notification settings - Fork 22
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add CMake install commands for Au #265
Conversation
To test, create a new project with two files: `main.cc`: ```cpp int main(int argc, char **argv) { return 0; } ``` `CMakeLists.txt`: ```cmake cmake_minimum_required(VERSION 3.29) project(CppUnitsCompare) include(FetchContent) FetchContent_Declare( Au GIT_REPOSITORY file:///home/chogg/au GIT_TAG "chiphogg/cmake-install#215" ) FetchContent_MakeAvailable(Au) add_executable(CppUnitsCompare main.cpp) target_link_libraries(CppUnitsCompare PUBLIC Au::au) ``` And execute: ```sh cmake -S . -B build cmake --build build ``` This PR is done when the above successfully compiles.
Referencing this Post: #215 (comment)
Fetch content here will basically act like a add_subdirectory, not a cmake install. If you want to test out if your cmake project is installable, you can literally call cmake install, and then have another project find_package(your package) See this: https://cmake.org/cmake/help/latest/guide/tutorial/Installing%20and%20Testing.html
If you did this with my repo for example, on linux you'd find the libraries installed in the standard linux install directories. If you run cmake install in the top level cmake directory and your project is configured correctly, it should work the same. Then you could do something like:
|
Thanks for clarifying. I think it will make sense to get the I've made a little bit of progress. When I change the include line in #include "build/_deps/au-src/au/au.hh" ...then, it does find this file, so we get a little further. However, it still fails to build because it can't find In other words: every header inside of Au assumes that the other Au headers are found relative to So I think we need to find some way to make it so that whenever anyone's CMake project depends on Section 35.5.1 offered a clue: maybe I needed to set set_target_properties(Au::au PROPERTIES
INTERFACE_COMPILE_FEATURES "cxx_std_14"
INTERFACE_INCLUDE_DIRECTORIES "${_IMPORT_PREFIX}/include"
INTERFACE_LINK_LIBRARIES "Au::chrono_interop;Au::constant;Au::math"
)
if(NOT CMAKE_VERSION VERSION_LESS "3.23.0")
target_sources(Au::au
INTERFACE
FILE_SET "HEADERS"
TYPE "HEADERS"
BASE_DIRS "${_IMPORT_PREFIX}/include"
FILES "${_IMPORT_PREFIX}/include/build/_deps/au-src/au/au.hh"
)
else()
set_property(TARGET Au::au
APPEND PROPERTY INTERFACE_INCLUDE_DIRECTORIES
"${_IMPORT_PREFIX}/include"
)
endif() What I think I want is for After playing around, I find:
I'm starting to worry I've bitten off more than I can chew, and we're not actually at all close to landing CMake support, just because the installation step is so hard. |
One thing I've noticed for projects which do somehow make this work, is that their public headers all live somewhere under an This is a very counterintuitive way for me to work, coming from a monorepo world. It seems to motivate keeping the header files in a completely different, parallel directory hierarchy relative to the So is the upshot that I need to bite the bullet and figure out how to move our headers from Were you able to install via |
This appears to work, although I'm not yet sure whether it's a good approach.
OK --- I think I see a way forward! The latest commit is just a proof of concept. I never thought it was production ready, and the surprise build failures reinforce that. But now, for the first time, we're successfully able to incorporate Au into an external project via CMake. And I think it shows me the way forward. I think the righteous approach will be to create a new subdirectory that contains the entire bazel project --- move everything bazel touches one level deeper into that new home. We'll still keep the main I haven't yet figured out what to call the subfolder. I don't want to call it That migration can be one PR, where we can work out all the kinks with the bazel integration. And then we can finish up this present PR once that's working. Meanwhile, comments on the current approach are also welcome. |
Thinking more about it: this could make things really annoying. We would have to invoke everything for bazel from within that subdirectory, instead of the git repo root folder. Maybe I'll try working around these issues, and then think more about what we should do. |
Not sure if the INCLUDEDIR needs to be updated too...
OK, I think the current approach is good enough to land. I'm a little unsure of how the symbolic link would work under windows. But if we find that it's problematic, we could add windows support later. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The changes look good. CMake is rough for packaging. I was able to navigate through the tests in the summary section.
It's funny that cmake --build build
builds all the au unit tests and yet CppUnitsCompare
doesn't depend on them. The concept of make all is foreign to us monrepo users 😄 . --target CppUnitsCompare
does what I expect. -j $(nproc)
will build in parallel, although it won't work on Windows.
In the summary
add_executable(CppUnitsCompare main.cpp)
Should be add_executable(CppUnitsCompare main.cc)
I'm not sure if you want to support find_package
as well in this repo. I tried cd au/cmake/build && cmake --install .
, but i got an error
Here is the long error
sudo cmake --install .
-- Install configuration: ""
-- Up-to-date: /usr/local/include
-- Installing: /usr/local/include/gmock
-- Installing: /usr/local/include/gmock/gmock-actions.h
-- Installing: /usr/local/include/gmock/gmock.h
-- Installing: /usr/local/include/gmock/gmock-nice-strict.h
-- Installing: /usr/local/include/gmock/gmock-cardinalities.h
-- Installing: /usr/local/include/gmock/gmock-spec-builders.h
-- Installing: /usr/local/include/gmock/gmock-matchers.h
-- Installing: /usr/local/include/gmock/gmock-more-actions.h
-- Installing: /usr/local/include/gmock/gmock-function-mocker.h
-- Installing: /usr/local/include/gmock/gmock-more-matchers.h
-- Installing: /usr/local/include/gmock/internal
-- Installing: /usr/local/include/gmock/internal/gmock-port.h
-- Installing: /usr/local/include/gmock/internal/custom
-- Installing: /usr/local/include/gmock/internal/custom/gmock-generated-actions.h
-- Installing: /usr/local/include/gmock/internal/custom/gmock-port.h
-- Installing: /usr/local/include/gmock/internal/custom/gmock-matchers.h
-- Installing: /usr/local/include/gmock/internal/custom/README.md
-- Installing: /usr/local/include/gmock/internal/gmock-internal-utils.h
-- Installing: /usr/local/include/gmock/internal/gmock-pp.h
-- Installing: /usr/local/lib/libgmock.a
-- Installing: /usr/local/lib/libgmock_main.a
-- Installing: /usr/local/lib/pkgconfig/gmock.pc
-- Installing: /usr/local/lib/pkgconfig/gmock_main.pc
-- Installing: /usr/local/lib/cmake/GTest/GTestTargets.cmake
-- Installing: /usr/local/lib/cmake/GTest/GTestTargets-noconfig.cmake
-- Installing: /usr/local/lib/cmake/GTest/GTestConfigVersion.cmake
-- Installing: /usr/local/lib/cmake/GTest/GTestConfig.cmake
-- Up-to-date: /usr/local/include
-- Installing: /usr/local/include/gtest
-- Installing: /usr/local/include/gtest/gtest_pred_impl.h
-- Installing: /usr/local/include/gtest/gtest-test-part.h
-- Installing: /usr/local/include/gtest/gtest.h
-- Installing: /usr/local/include/gtest/gtest-assertion-result.h
-- Installing: /usr/local/include/gtest/gtest-message.h
-- Installing: /usr/local/include/gtest/gtest-matchers.h
-- Installing: /usr/local/include/gtest/gtest-typed-test.h
-- Installing: /usr/local/include/gtest/gtest-printers.h
-- Installing: /usr/local/include/gtest/gtest_prod.h
-- Installing: /usr/local/include/gtest/gtest-param-test.h
-- Installing: /usr/local/include/gtest/gtest-death-test.h
-- Installing: /usr/local/include/gtest/gtest-spi.h
-- Installing: /usr/local/include/gtest/internal
-- Installing: /usr/local/include/gtest/internal/gtest-internal.h
-- Installing: /usr/local/include/gtest/internal/gtest-type-util.h
-- Installing: /usr/local/include/gtest/internal/gtest-death-test-internal.h
-- Installing: /usr/local/include/gtest/internal/gtest-port-arch.h
-- Installing: /usr/local/include/gtest/internal/custom
-- Installing: /usr/local/include/gtest/internal/custom/gtest.h
-- Installing: /usr/local/include/gtest/internal/custom/gtest-printers.h
-- Installing: /usr/local/include/gtest/internal/custom/gtest-port.h
-- Installing: /usr/local/include/gtest/internal/custom/README.md
-- Installing: /usr/local/include/gtest/internal/gtest-port.h
-- Installing: /usr/local/include/gtest/internal/gtest-param-util.h
-- Installing: /usr/local/include/gtest/internal/gtest-string.h
-- Installing: /usr/local/include/gtest/internal/gtest-filepath.h
-- Installing: /usr/local/lib/libgtest.a
CMake Error at _deps/googletest-build/googletest/cmake_install.cmake:84 (file):
file INSTALL cannot find
"/home/gviola/git/au/cmake/build/lib/libgtest_main.a": No such file or
directory.
Call Stack (most recent call first):
_deps/googletest-build/googlemock/cmake_install.cmake:67 (include)
_deps/googletest-build/cmake_install.cmake:47 (include)
cmake_install.cmake:47 (include)
You get the error because an internal fetch content is not really part of the install process, which it is using for google test. which is not necessary for an installed library. @chiphogg Testing should be guarded by a CMake option which would allow you to disable testing and run install, which would remove the dependency on gtest and not have to deal with that. Lines 38 to 50 in 3e25704
Testing should be guarded anyway because there's no way to use it within a package manager or offline in it's current state, and you don't want end users of an installed library to have to build tests, or worse have your version of gtest conflict with theirs. If they want tests they can subdirectory your project or just straight up clone the project. |
Yep --- I tried installing too, and I also got "some kind of error with the googletest deps". In my case, I discovered it by trying to actually use I think it will make the most sense to address this in a separate follow-on PR. But I definitely want to get this working before we close #215. Interestingly, the error is specifically with targets that use googletest, and not the Au tests themselves (i.e., #include "au/au.hh"
#include "au/io.hh"
#include "au/units/feet.hh"
#include "au/units/miles.hh"
#include <iostream>
int main(int argc, char **argv) {
std::cout << au::miles(1).as(au::feet) << " per mile" << std::endl;
} That was pretty cool. Still feels a little sloppy to just "use whatever's on the system", but at least I get the appeal now --- for quick and dirty projects, it's pretty awesome.
Well, it's a little complicated, because we do have one actual target ( We can start thinking now about what the follow-on PR needs to do. @Cazadorro, let me know if this sounds about right to you:
|
(And, to clarify: by "we won't build ... if this is absent", I mean "we won't create the target for ... if this is absent".) |
Just for posterity @Cazadorro: I believe I ended up resolving all of these issues.
If we find more problems later, we can see whether these option based solutions would resolve them. |
We may have gotten the project to build under CMake, but other
projects can't actually use it unless we provide
install
commands.First, we add an
install(TARGETS...)
command (based on theGNUInstallDirs
module) to theheader_only_library()
function. Weinstall it as part of an export set, whose name we set with a global
variable (which must be defined before we load this file).
The tricky bit is that this can't work unless the actual code itself is
in a subdirectory of the repo, not the root folder, for reasons I only
dimly understand. To make this work, we now provide the Au code via a
symlink, which we create inside of a new
cmake/project_symlinks
folder. This lets us refer to this folder in the
BASE_DIRS
variable.In the main CMake file, we add the installation command for the export
set we've been building up. We also write and install the package
config and version files.
The above changes would break bazel support, because there would be
BUILD files reachable underneath
cmake
, both via theau
symlink, andvia
cmake/build
. To fix this, we makecmake/
a fake localrepository, causing bazel to skip it.
To test, create a new project with two files:
main.cc
:CMakeLists.txt
:And execute:
cmake -S . -B build cmake --build build
Helps #215.