Split up CMakeLists.txt build instructions #4282
Conversation
89e0c9f to
15a7dea
Compare
15a7dea to
16ebecf
Compare
|
It looks like you've dropped the |
|
I know that's currently the state. But in the end, I think it's not required anymore due to us using subdirectories. This will automatically achieve what we've been doing with |
|
Interesting. It would be nice to drop that hack; I'll take it for a spin. |
|
Note though that this isn't ready yet on Windows, it has some problems with the precompiled headers there. I'm currently investigating. |
16ebecf to
f137136
Compare
|
By the way, I guess was wrong regarding IDE_SPLIT_SOURCES. While it's somewhat better now with different subdirectories it is still far from workable. So I'll re-include our hack. |
f137136 to
25542a5
Compare
701cda7 to
9fd15ca
Compare
|
Finally got it building now. Will have to fix up some locations such that e.g. the library and libgit2_clar are generated in the top-level build directory again. Note that this builds on #4288 |
56b60f2 to
bf5040c
Compare
|
I gave it a spin, and it's broken when using the Xcode generator (which is what I'm generally using).
|
|
Thanks for testing! The linking issue is quite easy to fix, I'll just have to add some missing libraries to the LIBGIT2_LIBDIRS variable. I'm a bit surprised the And yeah, I didn't yet re-add the |
a223423 to
bb8290c
Compare
bb8290c to
131969f
Compare
In our CMakeLists.txt, we have to check multiple functions in order to determine if we have to use our own or whether we can use the platform-provided one. For two of these functions, namely `regcomp_l()` and `futimens`, the defined macro is actually used inside of the header file "src/unix/posix.h". As such, these macros are not only required by the library, but also by our test suite, which is makes use of internal headers. To prepare for the CMakeLists.txt split, move these two defines inside of the "features.h" header.
When refering to files and directories inside of the top-level "deps/" directory, we're being inconsistent in using relative or absolute paths. To enable splitting out parts of the top-level CMakeLists.txt into an own file in the "src/" directory, consistently switch over to use absolute paths to avoid errors when converting.
Later on, we will move detection of required libraries, library directories as well as include directories into a separate CMakeLists.txt file inside of the source directory. Obviously, we want to avoid duplication here regarding these parameters. To prepare for the split, put the parameters into three variables LIBGIT2_LIBS, LIBGIT2_LIBDIRS and LIBGIT2_INCLUDES, tracking the required libraries, linking directory as well as include directories. These variables can later be exported into the parent scope from inside of the source build instructions, making them readily available for the other subdirectories.
Previous to keeping track of libraries and linking directories via variables, we had two call sites of the `TARGET_OS_LIBRARIES` function to avoid duplicating knowledge on required operating system library dependencies. But as the libgit2_clar target now re-uses defined variables to link against these libraries, we can simply inline the function.
This makes splitting up the library build instructions later on more obvious and easier to achieve.
This makes splitting up the library build instructions later on more obvious and easier to achieve.
This makes splitting up the library build instructions later on more obvious and easier to achieve.
Extract code required to build the winhttp library into its own CMakeLists.txt, which is included as required.
Extract code required to build the regex library into its own CMakeLists.txt, which is included as required.
Extract code required to build the http-parser library into its own CMakeLists.txt, which is included as required.
Extract code required to build the zlib library into its own CMakeLists.txt, which is included as required.
There are quite some uses of the variables "${CMAKE_CURRENT_SOURCE_DIR}"
and "${CMAKE_CURRENT_BINARY_DIR}" where they are not appropriate.
Convert these sites to instead use the variables "${CMAKE_SOURCE_DIR}"
and "${CMAKE_BINARY_DIR}", which instead point to the project's root
directory.
This will ease splitting up the library build instructions into its own
subdirectory.
To fix leaking build instructions into different targets and to make the build instructions easier to handle, create a new CMakeLists.txt file containing build instructions for the libgit2 target. By now, the split is rather easy to achieve. Due to the preparatory steps, we can now simply move over all related build instructions, only needing to remove the "src/" prefix from some files.
With c26ce78 (Merge branch 'AndreyG/cmake/modernization', 2017-06-28), we have recently introduced a regression in the way we are searching for headers. We have made sure to always include our own headers first, but due to the changes in c26ce78 this is no longer guaranteed. In fact, this already leads the compiler into picking "config.h" from the "deps/regex" dependency, if it is used. Fix the issue by declaring our internal include directories up front, before any of the other search directories is added.
2317fd8 to
8a43161
Compare
|
Thanks @pks-t . So I'm definitely happy with the speed improvements - this shaves about 25% of the original build time off on Windows. That's an insane improvement! One thing I don't love though, is where the build results end up. On Windows, anyway, the test runner ends up in But This seems a bit odd to me, tbqh. Do you happen to know why this happens? Is there a way to make the library land in |
|
Thanks for giving this a whip, @ethomson. The changing output location of the library is definitly a bug and can easily be changed. As to why: when you add a subdirectory, all binaries will by default end up in the same subdirectory inside of the build directory. For the clar I already changed the target output directory, but for the library I obviously forgot to do so. |
|
So I stand corrected. We are already setting the LIBRARY_OUTPUT_DIRECTORY to the project's root and on Linux the library ends up inside the correct directory. Will debug. |
|
@ethomson Should be fixed now. Details are in the commit, TLDR: DLLs and import libraries are not treated as libraries by CMake but instead as runtime and archive targets, respectively. |
As observed by Edward Thomson, the libgit2 DLL built by Windows will not
end up in the top-level build directory but instead inside of the 'src/'
subdirectory. While confusing at first because we are actually setting
the LIBRARY_OUTPUT_DIRECTORY to the project's binary directory, the
manual page of LIBRARY_OUTPUT_DIRECTORY clears this up:
There are three kinds of target files that may be built: archive,
library, and runtime. Executables are always treated as runtime
targets. Static libraries are always treated as archive targets.
Module libraries are always treated as library targets. For non-DLL
platforms shared libraries are treated as library targets. For DLL
platforms the DLL part of a shared library is treated as a runtime
target and the corresponding import library is treated as an archive
target. All Windows-based systems including Cygwin are DLL
platforms.
So in fact, DLLs and import libraries are not treated as libraries at
all by CMake but instead as runtime and archive targets. To fix the
issue, we can thus simply set the variables RUNTIME_OUTPUT_DIRECTORY and
ARCHIVE_OUTPUT_DIRECTORY to the project's root binary directory.
07bcd70 to
a3a3547
Compare
|
Thanks for this @pks-t ! |
|
Yay! :D Thanks @ethomson. I've rebased all my other PRs which also touch the CMakeLists.txt in any way. |
|
@ethomson Right now it looks fine to me. |
While discussing build issues with @ethomson, I've been reminded to how unwieldy our current CMakeLists.txt is. Besides being huge, it also has several problems with the build enviroment leaking between targets. One example are defines for our test suite, which are also set when we build our library code, which is unfortunate.
To fix this, I've started taking a look at splitting up the CMakeLists.txt. It's been quite annoying to do and probably has issues left, but I think it's a good think to do in the long run. Furthermore, I hope it will help the issue with precompiled headers from #3950. While at it, this PR also fixes building our library twice by using an object library, as in #3950.