Also applies to *_BINARY_DIR.
This effectively reverts 84083dcc8b,
which broke all users of libgit2 that use it as a CMake subdirectory
(via `add_subdirectory()`). This is because CMAKE_SOURCE_DIR refers
to the root-most CMake directory, which in the case of
`add_subdirectory()` is a parent project to libgit2 and thus the paths
don't make any sense to the configuration files. Corollary,
CMAKE_SOURCE_DIR only makes sense if the CMake project is always the
root project - which can rarely be guaranteed.
In all honesty, CMake should deprecate and eventually remove
CMAKE_SOURCE_DIR and CMAKE_BINARY_DIR. It's been the source of headaches
and confusion for years, they're rarely useful over
CMAKE_CURRENT_(SOURCE|BINARY)_DIR or PROJECT_(SOURCE|BINARY)_DIR,
and they cause a lot of confusing configuration and source
code layouts to boot.
Any time they are used, they break `add_subdirectory()` almost 100% of
the time, cause confusing error messages, and hide subtle bugs.
Ensure that we `enable_testing()` at the top-level CMakeLists.txt or
else we'll need to navigate within the build directory to the correct
place in the hierarchy to run `ctest`. Now we can `ctest` at the
top-level again.
Older versions of gcc do not believe that we've adequately declared our
test functions. Include `clar_suite.h` conditionally for those old
versions. Do not do this on newer compilers to avoid unnecessary
recompilation of the entire suite when we add or remove a test function.
This change makes it possible to prefer a python (version 3) interpreter
on systems where python2 and python3 coexist (where python2 is found as
`/usr/bin/python`).
Download poxygit, a debugging git server, and clone from it using NTLM,
both IIS-style (with connection affinity) and Apache-style ("broken",
requiring constant reauthentication).
Back in commit cf9f34521 (cmake: bump minimum version to 2.8.11,
2017-09-06), we have bumped the minimum CMake version to require at
least v2.8.11. The main hold-backs back then were distributions like
RHEL/CentOS as well as Ubuntu Trusty, which caused us to not target a
more modern version. Nowadays, Ubuntu Trusty has been EOL'd and CentOS 6
has CMake v3.6.1 available via the EPEL6 repository, and thus it seems
fair to upgrade to a more recent version.
Going through repology [1], one can see that all supported mainstream
distributions do in fact have CMake 3 available. Going through the list,
the minimum version that is supported by all mainstream distros is in
fact v3.5.1:
- CentOS 6 via EPEL6: 3.6.1
- Debian Oldstable: 3.7.2
- Fedora 26: 3.8.2
- OpenMandriva 3.x: 3.5.1
- Slackware 14.2: 3.5.2
- Ubuntu 16.04: 3.5.1
Consequentally, let's upgrade CMake to the minimum version of 3.5.1 and
remove all the version CMake checks that aren't required anymore.
[1]: https://repology.org/project/cmake/versions
Right now, we have an awful hack in our test CI setup that extracts the
test command from CTest's output and then prepends the leak checker.
This is dependent on non-machine-parseable output from CMake and also
breaks on various ocassions, like for example when we have spaces in the
current path or when the path contains backslashes. Both conditions may
easily be triggered on Win32 systems, and in fact they do break our
Azure Pipelines builds.
Remove the awful hack in favour of a new CMake build option
"USE_LEAK_CHECKER". If specifying e.g. "-DUSE_LEAK_CHECKER=valgrind",
then we will set up all tests to be run under valgrind. Like this, we
can again simply execute ctest without needing to rely on evil sourcery.
Although the error functions were deprecated, we did not properly mark
them as deprecated. We need to include the `deprecated.h` file in order
to ensure that the functions get their export attributes.
Similarly, do not define `GIT_DEPRECATE_HARD` within the library, or
those functions will also not get their export attributes. Define that
only on the tests and examples.
Introduce a CLAR_XML option, to run the `ctest` commands with the new
`-r` flag to clar. Permitted values are `OFF`, `ON` and a directory to
write the XML test results to.
Simplify the names for the tests, removing the unnecessary
"libgit2-clar" prefix. Make "all" the new default test run, and include
the online tests by default (since HTTPS should always be enabled).
For the CI tests, create an offline-only test, then the various online
tests.
While the aim of libgit2 was to conform to C90 code, we never instructed
the compiler to enforce C90 compliance. Thus, quite a few violations
were able to get into our code base, which have been removed with the
previous commits. As we are now able to build libgit2 with C90 enforced,
we can set the C_STANDARD property for our own build targets.
Note that we explicitly avoid setting the C standard for our third-party
dependencies. At least the zlib target does not build with C90 enforced,
and we do not want to fix them by deviating from upstream. Thus we
simply enforce no standard for them.
While we want to enforce strict C90 mode, this may cause issues with
system provided header files which are themselves not strictly
conforming. E.g. if a system header has C++ style comments, a compiler
in strict C90 mode would produce an error and abort the build. As the
user most likely doesn't want to change the system header, this would
completely break the build on such systems. One example of this is
mbedtls, which provides such header files.
The problem can be worked around by distinguishing between
system-provided and project-provided include directories. When adding
include directories via "-isystem" instead of "-I", the compiler will
skip certain checks and print out less warnings. To use system includes,
we can simply add the "SYSTEM" flag to CMake's `INCLUDE_DIRECTORIES` and
`TARGET_INCLUDE_DIRECTORIES` functions. Note that we have to split the
include directories into two variables because of this, as we definitely
still want to check for all warnings produced by our own header files.
With the recent change of always resolving pkg-config libraries to their
full path, we do not have to manage the LIBGIT2_LIBDIRS variable
anymore. The only other remaining user of LIBGIT2_LIBDIRS is winhttp,
which is a CMake-style library target and can thus be resolved by CMake
automatically.
Remove the variable to simplify our build system a bit.
Some tests shall be run against our own SSH server we spin up in Travis.
As those need to be run separate from our previous tests which run
against git-daemon, we have to do this in a separate step. Instead of
bundling all that knowledge in the CI script, move it into the test
build instructions by creating a new test target.
Right now, we test our credential callback code twice, once via SSH on
localhost and once via a non-existent GitHub repository. While the first
URL makes sense to be configurable, it does not make sense to hard-code
the non-existing repository, which requires us to call tests multiple
times. Instead, we can just inline the URL into another set of tests.
We support two types of passing credentials to the proxy, either via the
URL or explicitly by specifying user and password. We test these types
by modifying the proxy URL and executing the tests twice, which is
in fact unnecessary and requires us to maintain the list of environment
variables and test executions across multiple CI infrastructures.
To fix the situation, we can just always pass the host, port, user and
password to the tests. The tests can then assemble the complete URL
either with or without included credentials, allowing us to test both
cases in-process.
Our performance tests (or to be more concrete, our single performance
test) are not built by default, as they are always #ifdef'd out. While
it is true that we don't want to run performance tests by default, not
compiling them at all may cause code rot and is thus an unfavorable
approach to handle this.
We can easily improve this situation: this commit removes the #ifdef,
causing the code to always be compiled. Furthermore, we add `-xperf` to
the default command line parameters of `generate.py`, thus causing the
tests to be excluded by default.
Due to this approach, we are now able to execute the performance tests
by passing `-sperf` to `libgit2_clar`. Unfortunately, we cannot execute
the performance tests on Travis or AppVeyor as they rely on history
being available for the libgit2 repository. As both do a shallow clone
only, though, this is not given.
It defaults to ON, e.g. "pick whatever default is appropriate for the platform".
It accepts one of SecureTransport, OpenSSL, WinHTTP, or OFF.
It errors if the backend library couldn't be found.
Due to our split of CMake files into multiple modules, we had to replace
some uses of the `${CMAKE_CURRENT_SOURCE_DIR}` and
`${CMAKE_CURRENT_BINARY_DIR}` variables and replace them with
`${CMAKE_SOURCE_DIR}` and `${CMAKE_BINARY_DIR}`. This enabled us to
still be able to refer to top-level files when defining build
instructions inside of a subdirectory.
When replacing all variables, it was assumed that the absolute set of
variables is always relative to the current project. But in fact, this
is not the case, as these variables always point to the source and
binary directory as given by the top-levl project. So the change
actually broke the ability to include libgit2 directly as a subproject,
as source files cannot be found anymore.
Fix this by instead using project-specific source and binary directories
with `${libgit2_SOURCE_DIR}` and `${libgit2_BINARY_DIR}`.
There are two locations where we check whether CMake supports
`TARGET_INCLUDE_DIRECTORIES`. While the first one uses `VERSION_LESS
2.8.12`, the second one uses `VERSION_GREATER 2.8.11`, which are
obviously equivalent to each other. It'd still be easier to grep for
specific CMake versions being required for some features if both used
the same conditional mentioning the actual target version required. So
this commit refactors these conditions to make them equal.
Distinguish variables keeping track of our internal libgit2 sources and
the final objects which shall be linked into the library. This will ease
the transition to use object libraries for our bundled dependencies
instead of linking them in.
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.
In a future commit, we will split out the build instructions for our
library directory and move them into a subdirectory. One of the benefits
is fixing scoping issues, where e.g. defines do not leak to build
targets where they do not belong to. But unfortunately, this does also
pose the problem of how to propagate some defines which are required by
both the library and the test suite.
One way would be to create another variable keeping track of all added
defines and declare it inside of the parent scope. While this is the
most obvious and simplest way of going ahead, it is kind of unfortunate.
The main reason to not use this is that these defines become implicit
dependencies between the build targets. By simply observing a define
inside of the CMakeLists.txt file, one cannot reason whether this define
is only required by the current target or whether it is required by
different targets, as well.
Another approach would be to use an internal header file keeping track
of all defines shared between targets. While configuring the library, we
will set various variables and let CMake configure the file, adding or
removing defines based on what has been configured. Like this, one can
easily keep track of the current environment by simply inspecting the
header file. Furthermore, these dependencies are becoming clear inside
the CMakeLists.txt, as instead of simply adding a define, we now call
e.g. `SET(GIT_THREADSAFE 1)`.
Having this header file though requires us to make sure it is always
included before any "#ifdef"-preprocessor checks are executed. As we
have already refactored code to always include the "common.h" header
file before any statement inside of a file, this becomes easy: just make
sure "common.h" includes the new "features.h" header file first.
Our CMakeLists.txt is very unwieldy in its current size, spanning more
than 700 lines of code. Furthermore, it has several issues regarding
scoping, where for example some defines, includes, etc. from our test
suite are also applied to our normal library code.
To fix this, we can separate out build instructions for our tests and
move them into their own CMakeLists.txt in the "tests" directory. This
reduced complexity of the root CMakeLists.txt file and fixes the issues
regarding leaking build context from tests into the library.