CMake install command does not copy the external directory
Hi,
I cloned the latest master today & built OpenXLSX with CMake. I'm not well versed in CMake but shouldn't it copy the external directory to the target install directory too?
Doğrusu: I have not much of a clue about cmake at all - so I only ever used it to build executables - and the Makefile.GNU that you find in the repository was created by me so I don't have to deal with cmake all the time. Could you tell me what your use case is here?
As far as I understand, the "target" directory is the one where the libOpenXLSX.a binary ends up (on a make install) so that you can include it from other projects? Wouldn't that mean that there are multiple target directories, with another one for the include files to use the library?
So do I correctly understand that what you miss is that the header files from external are not being copied to the destination? ==> I think that might be because those are header-only libraries, and they are used "under the hood" - i.e. nowide, zippy and pugixml functions are not available directly to programs using OpenXLSX.
Is there any functionality that you are missing that is not working for you?
@aral-matrix, hello. I have the same question.
I use the OpenSLX library, which is installed by the "cmake install" command. This command copies headers from the "OpenXLSX/headers" folder, but does not copy headers from the "OpenXLSX/external" folder. However, the "XLXmlParser.hpp" header includes the "external/pugixml/pugixml.hpp" file, and since the "external" folder is not copied, this causes a compilation error (the compiler cannot find such a file).
The line #include <external/pugixml/pugixml.hpp> was added to "XLXmlParser.hpp" file in commit https://github.com/troldal/OpenXLSX/commit/f07956dcfec106d85c8d281b50aa19030b4ff875, so the problem did not exist before that commit.
So I repeat the question asked by @fauder. Shouldn't CMake copy the external directory to the target install directory?
Thank you @adkandrin for the explanation. Now I understood what the issue is. This is my fault, I made the XLXmlParser extend the pugi::xml_node by a lot of functionality, and didn't realize that somehow, this was previously working without an explicit include in the header / I didn't realize the implication when adding that.
I will see if I can fix this so that the pugixml include can be hidden in the CPP file again.
Otherwise & just in case - would you happen to know enough about cmake to tell me how I can include this copy instruction for the external folder headers in the CMakeFile? I'd appreciate a pointer :)
Sorry I could not get back to you on this.
@adkandrin summed it up well 👍
@aral-matrix In the file "OpenXLSX/CMakeLists.txt" there is an instruction that copies headers from the "headers" folder:
file(GLOB OPENXLSX_HEADER_LIST ${CMAKE_CURRENT_LIST_DIR}/headers/*.hpp)
install(
FILES ${OPENXLSX_HEADER_LIST}
DESTINATION ${CMAKE_INSTALL_INCLUDEDIR}/OpenXLSX/headers
)
I think you can similarly add an instruction that copies files from the "external/pugixml" folder:
file(GLOB OPENXLSX_EXTERNAL_PUGIXML_FILE_LIST ${CMAKE_CURRENT_LIST_DIR}/external/pugixml/*)
install(
FILES ${OPENXLSX_EXTERNAL_PUGIXML_FILE_LIST}
DESTINATION ${CMAKE_INSTALL_INCLUDEDIR}/OpenXLSX/external/pugixml
)
All files are copied here, not just headers, because the file "pugihml.hpp" includes the file "pugixml.cpp".
Although it would be better, of course, to find a solution that hides external dependencies in the CPP-file.
Although it would be better, of course, to find a solution that hides external dependencies in the CPP-file.
Sorry for not looking at this issue in a while - I am only now getting down in the backlog to look at older issues again - I will need a few days, if not weeks to think about a solution.
Hi! I also encountered this problem when trying to connect OpenXLSX to my project on Windows as separated library. After two days of suffering, I managed to build the following "crutch".
When using OpenXLSX in a project, you need to do the following:
- If the OpenXLSX library has not yet been built via cmake, make sure that in the files
./OpenXLSX/sources/XLXmlParser.cpp
./OpenXLSX/headers/XLXmlParser.hpp
includes look like this:
#include "../external/pugixml/pugixml.hpp"
#include "XLException.hpp"
and ONLY after that compile the library via cmake
//--- FOR CONVENIENCE --\
- run PowerShell as Administrator
- go to the folder with the sources (OpenXLSX)
- enter the commands:
mkdir build
cd build
cmake ..
cmake --build . --target OpenXLSX --config Release
cmake --install .
In my case library automaticaly installed in C:\Program Files (x86)\OpenXLSX.Library\.
2) Copy the OpenXLSX/external folder to your project's source folder
- In Visual Studio Community 2022:
- open your project
- select Project -> Properties
- in Configuration Properties -> General in "C++ Language Standard" use
ISO C++17 Standard (/std:c++17) - in C/C++ -> General, in "Additional Include Directories", add a line with a link to the external folder and to installed library For example:
C:\Program Files (x86)\OpenXLSX.Library\include
<my project folder>\source\external
- (optional) in C/C++ -> Advanced in "Disable Specific Warnings" enter
4996 - in Linker -> General, in "Additional Library Directories", add lines
C:\Program Files (x86)\OpenXLSX.Library\lib
<my project folder>\source\external
- in Linker -> Input, in "Additional Dependencies", add "OpenXLSX.lib"
- click "Apply", then "OK"
- In your code, include the library as
#include <OpenXLSX/OpenXLSX.hpp>
After these manipulations library began to work correctly.
PS. I didn't check whole functional after this, so you can get another problems.
hi @EvgenyEletskikh - thank you for the additional data points - as I wrote above I will look for a generic solution, that should make your life easier. But I am currently busy on a different project, so my apologies for the wait.
Update! @fauder @EvgenyEletskikh @adkandrin
Although it would be better, of course, to find a solution that hides external dependencies in the CPP-file.
I think today I finally managed to properly hide the whole XLXmlParser module behind the OpenXLSX library interface.
I reworked the library (code review is still pending) to hide the pugixml header behind the wrapper header XLXmlParser.hpp and then only include the XLXmlParser from module files, but not(!) from header files.
This means that I was able to remove all pugixml include statements except for the XLXmlParser wrapper module (header and cpp files): 665310262031d8098069e1a299d9b931ac533a00 and ce2a3c99714976207c3d4ea040523f6dd23d34fe
I would appreciate very much if you could test the latest version on the development-aral branch and give me some feedback here whether this issue can be closed after all this time.
@aral-matrix I'll try it soon (I hope) and get back to you. Thanks for your hard work!
@aral-matrix I'll try it soon (I hope) and get back to you. Thanks for your hard work!
Teşekkür ediyorum :) Birkaç şeye daha bakmam lazım, ama yavaşça (ilk önce) debian için bir "package" hazırlamaya uğraşmaktayım.
Hi, I'm upgrading my OpenXLSX installation and will test it now.
Is your dev branch development-aral safe for production work?
Current master is behind those commits you mentioned.
The current development-aral branch is reasonably safe to use, I have performed a thorough review on the code that is affected by hiding pugixml as a library internal tool. Obviously, I can never guarantee that I did everything bug-free.
The one thing that currently does not work is automatic download & inclusion of libzip from a repository - but if you stick to the default settings, miniz will work just fine.
I had similar problems reported by others in this issue. But branch development-aral fixes it! Thanks.
BTW, I created a Debian package by running:
echo 'include(CPack)' >> CMakeLists.txt
cmake -D CPACK_PACKAGE_CONTACT="email address" -D CPACK_DEBIAN_PACKAGE_MAINTAINER="Jacob Burckhardt <email address>" -D CPACK_PACKAGE_VERSION_MAJOR=2025 -D CPACK_PACKAGE_VERSION_MINOR=10 -D CPACK_PACKAGE_VERSION_PATCH=8-`git rev-parse HEAD` .
cmake --build . --target OpenXLSX
cpack -G DEB
If you want to officially release a Debian package this way, I think you can move the include(CPack) and the -D options into the CMakeLists.txt and change my name to the appropriate maintainer's name.
I had similar problems reported by others in this issue. But branch development-aral fixes it! Thanks.
BTW, I created a Debian package by running: [..] If you want to officially release a Debian package this way, I think you can move the
include(CPack)and the-Doptions into the CMakeLists.txt and change my name to the appropriate maintainer's name.
Thank you so much @Jacob-Burckhardt - it took a bit of fiddling until I got it working, but 7c1a23c400b935651044d5999393daa322ba3b54 implements your suggestions - it's a work in progress for now, because I need to correctly identify the package dependencies (and ideally build the .deb directly from CMake via an option), but you helped me make some good progress today!