Add MSVC build support and install rules to CMake build.#510
Open
manuelkNVDA wants to merge 1 commit into
Open
Add MSVC build support and install rules to CMake build.#510manuelkNVDA wants to merge 1 commit into
manuelkNVDA wants to merge 1 commit into
Conversation
Reworks the Visual Studio / MSVC build path while preserving the existing Linux
build. Adds install rules and a CMake package config so downstream projects can
`find_package(abc)` and link against `abc::abc`.
CMakeLists.txt:
- Bumps cmake_minimum_required to 3.25 (for CMAKE_MSVC_DEBUG_INFORMATION_FORMAT
and CMP0141) and moves `project(abc VERSION 1.0.0 ...)` to the top.
- Adds an `if(MSVC) ... else() ...` split: the MSVC branch parses the
per-module `module.make` files to derive the source list (matching
what the upstream Makefile uses), configures CRT macros / warning
suppressions / pthread shim defines, embeds debug info in each .obj
to avoid PDB contention with ~1370 parallel TUs, and adds a
UfarPthStub.c with no-op pthread implementations for the
UfarPth.cpp symbols that are referenced unconditionally on Windows.
- The non-MSVC `else()` branch is byte-identical to the previous
top-level logic, with two minor changes:
* `abc_properties` wraps the source-tree include in
`$<BUILD_INTERFACE:...>` so it doesn't leak into the exported
targets file at install time.
* The redundant inner `project(abc)` is removed so the top-level
project version survives.
- Adds standardized install rules (applied to both branches):
<prefix>/bin/abc[.exe]
<prefix>/lib/lib{abc.a,abc.lib}
<prefix>/include/abc/<full src/ header tree>
<prefix>/lib/cmake/abc/{abcConfig,abcConfigVersion,abcTargets}*.cmake
plus a `cmake/abcConfig.cmake.in` template for find_package(abc).
Source patches required for MSVC compilation:
- src/misc/util/abc_global.h: guard the _CRTDBG_MAP_ALLOC block with
!defined(__cplusplus) so the debug-heap macros don't substitute
`free` inside C++ TUs (breaks CaDiCaL's util.hpp).
- src/opt/eslim/utils.hpp: fix _BitScanForward64 call -- the first
argument must be `unsigned long *`.
- src/opt/eslim/windowMan.tpp: missing semicolon in the no-pthreads
code path.
- src/proof/cec/cecProve.c: move two pthread_t prototypes inside the
existing `#ifdef ABC_USE_PTHREADS` guard.
.gitignore: remove `/cmake` entry because we need the cmake folder for
the added package-config template.
Verified end-to-end on MSVC 16 & 17 (Release + Debug) and Linux/WSL2
(Ubuntu, gcc 13.3)
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Summary
find_package(abc)and link againstabc::abc.note to reviewers: diffing CMakeLists.txt in github's web tool is ugly, but the MSVC branch genuinely leaves the existing code path untouched ; using p4merge with white-space ignored is much more readable.
What's new
MSVC build branch in
CMakeLists.txt(if(MSVC)):module.makefiles, matching the upstream Makefile's canonicalsource list (somewhat future-proof to future file additions, but relies on regex).
WIN32_NO_DLL,WIN32_LEAN_AND_MEAN,HAVE_STRUCT_TIMESPEC,PTW32_STATIC_LIB)configured for a static build with the in-tree pthreads-win32 shim.
CMAKE_MSVC_DEBUG_INFORMATION_FORMAT "Embedded") to avoid PDB writecontention with ~1370 parallel TUs.
UfarPthStub.cprovides no-op pthread implementations so the unconditional pthread references inUfarPth.cpplink without a full pthreads libraryInstall rules and package config (applied to both MSVC and Linux):
<prefix>/bin/abc[.exe]<prefix>/lib/libabc.{a,lib}<prefix>/include/abc/<full src/ header tree><prefix>/lib/cmake/abc/{abcConfig,abcConfigVersion,abcTargets}*.cmakeDownstream usage:
Related issues
Fixes #71 —
cmake files don't contain any install instructions. Adds the missing install rules.Fixes #56 —
Shared library is installed w/out the headers. The full src/ header tree is installed under${CMAKE_INSTALL_INCLUDEDIR}/abc/.(The shared-library / SONAME part of that issue is outside this PR's scope: we still install the static
libabc)Related: #451 —
Building DEB package for libabc-dev. This PR provides the install rules, exported targets, andabcConfig.cmakethat a downstream DEB/CPack packager needs, but does not itself add CPack DEB rules,pkg-configgeneration, or a shared-library build. Those become small follow-ups on top of the infrastructure here.