build: clean up and partial synchronisation with upstream #357
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
CMake 3.12 deprecated `FindPythonInterp`, and with CMake 3.27, were obsoleted with CMP0148. Add a version check and switch to the new behaviour to allow building with newer releases. When the minimum CMake version is increased, we can look at dropping the compatibility path. Adjust the `add_test` invocation to clearly indicate the test name and command parameters.
`BUILD_TESTING` is the CMake sanctioned option for the control of tests (c.f. https://cmake.org/cmake/help/latest/module/CTest.html). Use the standard option to control the build of test targets.
Policy settings may impact how `project` functions. They should be set immediately after `cmake_minimum_required` (which implicitly sets policies). Use the `POLICY` check to see if a policy is defined rather than using a version check.
This variable was no longer in use and the recent cleanups had exposed it as being unnecessary.
The `CMARK_STATIC_DEFINE` macro identifies that the library is being linked statically, which is important information for building on Windows. This uses CMake to automatically propagate the necessary definition to the users and the build of the library itself rather than having everyone specify it manually.
Use generator expressions to compute the path rather than hardcoding the layout. Finally, clean up some unnecessary quoting and uniformly spell commands in the tests.
Match the style as recommended by CMake documentation in [1]. This makes it easier to read the command being invoked when running tests. No functional change. [1] https://cmake.org/cmake/help/latest/command/add_test.html
CMake 3.15+ remove `/W3` from the language flags under MSVC with CMP0092. Set the policy to new to avoid the D9025 warning.
compnerd
force-pushed
the
cmark-gfm-build
branch
from
4a4529d
to
fe3902c
Compare
`stdbool.h` is part of C99 however was not provided by Visual Studio 2013 until RTM [1]. Remove the check for the header and inline the include at the usage sites rather than relying on `config.h`. VS2013 was EOL'ed Apr 9, 2019, with extended support ending Apr 9, 2024. `HAVE___ATTRIBUTE__` was unused in the codebase and served no purpose. Remove shims for `snprintf` and `vsnprintf` which were unavailable prior to VS2015. As VS2013 is no longer serviced, this reduces complexity in the project.
Avoid including the utility once, which should avoid some unnecessary CMake checks, and reduces duplication.
man pages are extremely useful, but are not generally available on Windows. This changes the install condition to check for the Windows cross-compile rather than the toolchain in use. It is possible to build for Windows using clang in the GNU driver.
* build: inline a variable * build: use `LINKER_LANGUAGE` property for C++ runtime Rather than explicitly name the C++ runtime, use the `LINKER_LANGUAGE` property to use the driver to spell the C++ runtime appropriately. * build: use CMake to control C standard Rather than use compiler specific flags to control the language standard, indicate to CMake the desired standard. * build: use the correct variable These flags are being applied to the *C* compiler, check the C compiler, not the C++ compiler. * build: loosen the compiler check This loosens the compiler identifier check to enable matching AppleClang which is the identifier for the Xcode compiler. * build: hoist shared flags to top-level CMakeLists This hoists the common shared flags handling to the top-level CMakeLists from sub-layers. This prevents the duplication of the handling. * build: remove duplicated flags This is unnecessary, `/TP` is forced on all MSVC builds, no need to duplicate the flag for older versions. * build: loosen C compiler identifier check Loosen the check to a match rather than equality check, this allows it to match AppleClang which is the identifier for the Apple vended clang compiler part of Xcode. * build: use `add_compile_options` Use `add_compile_options` rather than modify `CMAKE_C_FLAGS`. The latter is meant to be only modified by the user, not the package developer. * build: hoist sanitizer flags to global state This moves the CMAKE_C_FLAGS handling to the top-level and uses `add_compile_options` rather than modifying the user controlled flags. * build: hoist `-fvisibilty` flags to top-level These are global settings, hoist them to the top level. * build: hoist the debug flag handling Use a generator expression and hoist the flag handling for the debug build. * build: hoist the profile flag handling This is a global flag, hoist it to the top level and use `add_compile_options` rather than modify the user controlled flags. * build: remove incorrect variable handling This seemed to be attempting to set the linker not the linker flags for the profile configuration. This variable is not used, do not set it. * build: remove unused CMake includes
Newer versions of CMake attempt to query the system for information about the VS 2017 installation. Unfortunately, this query fails on non-Windows systems when cross-compiling: cmake_host_system_information does not recognize <key> VS_15_DIR CMake will not find these system libraries on non-Windows hosts anyways, and we were silencing the warnings, so simply omit the installation when cross-compiling to Windows.
Remove the use of `CMARK_INLINE` (as is being considered upstream) as this workaround is no longer needed. It was to support VS2013 which has been obsoleted for some time now. Restructure the CMake modules to match expectations for CMake. Avoid defining `likely` and `unlikely` in global scope. While doing so, prefer the modern `__has_builtin` for the check (with fallbacks for pre-GCC 4.10, clang 3.4, and MSVC). This largely shouldn't matter in practice as modern branch predictors do well without the hint. Sink the declaration into source files as the macros are not namespaced. Remove the now unnecessary `config.h`. Clean up the builds to only support a single build type which is dependent on the standard `BUILD_SHARED_LIBS`. Simplify and normalise the install rules to use the `GNUInstallDirs` parameters.
compnerd
force-pushed
the
cmark-gfm-build
branch
from
fe3902c
to
6336e7e
Compare
OKEAMAH
approved these changes
Feb 18, 2024
OKEAMAH
approved these changes
Feb 18, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Pull down a number of upstream changes for the build system. Additionally add some inflight changes. This overhaul simplifies the configuration and uses standard build options now e.g.
BUILD_SHARED_LIBS,BUILD_TESTINGto configure the build as per CMake documentation.