build: install() rules + CPack for reproducible packages#91
Conversation
Today the release archives are assembled by hand in release.yml's staging steps. Add CMake install() rules and a CPack config so the same layout can be produced reproducibly with `cmake --install` / `cpack`, and so distro packaging (DEB) has a real target. Layout matches the existing release zip: the engine DLL (openace64) plus a drop-in ACE-named copy (ace64.dll / libace64.so / .dylib), the server + bench CLIs, public headers, per-compiler import libs under lib/, LICENSE/NOTICE/README and the systemd/launchd templates. Windows stays flat (drop ace64.dll next to the .exe); POSIX uses GNUInstallDirs so binaries land under bin/ and lib/. Version is not hard-coded: CPACK_PACKAGE_VERSION / the archive name both derive from OPENADS_VERSION_STR (git describe), so tagging v1.4.1 produces openads-1.4.1-<os>-<arch> automatically. Also stop the vendored, statically-linked deps from leaking their own install rules into our package: HTTPLIB_INSTALL / JSON_Install OFF and mbedtls ENABLE_PROGRAMS/TESTING/MBEDTLS_INSTALL OFF. Verified the resulting ZIP contains only OpenADS artifacts (no httplib.h, no *Config.cmake, no third-party share/doc). The onboarding files (openads.ini.sample, QUICKSTART, openads-studio.*) install OPTIONAL-ly so this PR is self-contained against main and they appear once the sibling PRs land. CPack is not yet wired into release.yml (that touches CI) — left to the maintainer. Verified: `cpack -G ZIP` off main produces a clean, correctly-named archive; normal build unaffected, -Werror clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Code Review
This pull request introduces comprehensive install rules and CPack configuration to CMakeLists.txt to enable structured, cross-platform packaging for OpenADS. Feedback focuses on improving the installation logic: removing redundant CMake variable definitions, ensuring shared libraries and shell scripts are installed with correct executable permissions on POSIX systems, and restricting OS-specific service files and scripts (such as systemd/launchd templates and .bat/.sh scripts) to their respective target platforms.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| set(ENABLE_PROGRAMS OFF CACHE BOOL "" FORCE) | ||
| set(ENABLE_TESTING OFF CACHE BOOL "" FORCE) |
| if(WIN32) | ||
| install(FILES "$<TARGET_FILE:openads_ace>" | ||
| DESTINATION "${OA_INSTALL_BIN}" RENAME "ace${OA_BITS}.dll") | ||
| elseif(APPLE) | ||
| install(FILES "$<TARGET_FILE:openads_ace>" | ||
| DESTINATION "${OA_INSTALL_BIN}" RENAME "libace${OA_BITS}.dylib") | ||
| else() | ||
| install(FILES "$<TARGET_FILE:openads_ace>" | ||
| DESTINATION "${OA_INSTALL_BIN}" RENAME "libace${OA_BITS}.so") | ||
| endif() |
There was a problem hiding this comment.
On POSIX systems (macOS and Linux), using install(FILES) to copy the shared library target will install it with default non-executable permissions (typically 644). Shared libraries should be installed with executable permissions (typically 755) to ensure they can be loaded correctly and to satisfy package manager checks. Using install(PROGRAMS) instead of install(FILES) on non-Windows platforms automatically applies the correct executable permissions.
if(WIN32)
install(FILES "$<TARGET_FILE:openads_ace>"
DESTINATION "${OA_INSTALL_BIN}" RENAME "ace${OA_BITS}.dll")
elseif(APPLE)
install(PROGRAMS "$<TARGET_FILE:openads_ace>"
DESTINATION "${OA_INSTALL_BIN}" RENAME "libace${OA_BITS}.dylib")
else()
install(PROGRAMS "$<TARGET_FILE:openads_ace>"
DESTINATION "${OA_INSTALL_BIN}" RENAME "libace${OA_BITS}.so")
endif()
| install(FILES "${CMAKE_SOURCE_DIR}/tools/scripts/systemd/openads-serverd.service" | ||
| "${CMAKE_SOURCE_DIR}/tools/scripts/launchd/com.openads.serverd.plist" | ||
| DESTINATION "${OA_INSTALL_DOC}/service" OPTIONAL) |
There was a problem hiding this comment.
Installing systemd and launchd service files on Windows is unnecessary and pollutes the installation directory. These should be wrapped in a if(NOT WIN32) block, and ideally, the systemd service should only be installed on Linux and the launchd plist only on macOS.
if(NOT WIN32)
if(APPLE)
install(FILES "${CMAKE_SOURCE_DIR}/tools/scripts/launchd/com.openads.serverd.plist"
DESTINATION "${OA_INSTALL_DOC}/service" OPTIONAL)
else()
install(FILES "${CMAKE_SOURCE_DIR}/tools/scripts/systemd/openads-serverd.service"
DESTINATION "${OA_INSTALL_DOC}/service" OPTIONAL)
endif()
endif()
| install(FILES "${CMAKE_SOURCE_DIR}/openads.ini.sample" | ||
| "${CMAKE_SOURCE_DIR}/QUICKSTART.md" | ||
| "${CMAKE_SOURCE_DIR}/openads-studio.sh" | ||
| "${CMAKE_SOURCE_DIR}/openads-studio.bat" | ||
| DESTINATION "${OA_INSTALL_BIN}" OPTIONAL) |
There was a problem hiding this comment.
Installing openads-studio.bat on POSIX systems or openads-studio.sh on Windows is unnecessary. Additionally, openads-studio.sh should be installed with executable permissions using install(PROGRAMS) so that users do not have to manually run chmod +x after installation.
install(FILES "${CMAKE_SOURCE_DIR}/openads.ini.sample"
"${CMAKE_SOURCE_DIR}/QUICKSTART.md"
DESTINATION "${OA_INSTALL_BIN}" OPTIONAL)
if(WIN32)
install(FILES "${CMAKE_SOURCE_DIR}/openads-studio.bat"
DESTINATION "${OA_INSTALL_BIN}" OPTIONAL)
else()
install(PROGRAMS "${CMAKE_SOURCE_DIR}/openads-studio.sh"
DESTINATION "${OA_INSTALL_BIN}" OPTIONAL)
endif()
What & why
The release archives are currently assembled by hand in the
release.ymlstaging steps. This adds CMakeinstall()rules and aCPackconfig so the same layout is reproducible withcmake --install/cpack, and so distro packaging (DEB) has a real target.The layout matches the existing release zip:
openace64) + a drop-in ACE-named copy (ace64.dll/libace64.so/.dylib)openads_serverd+openads_benchlib/LICENSE/NOTICE/README+ the systemd/launchd templates underservice/Windows stays flat (drop
ace64.dllnext to the.exe); POSIX usesGNUInstallDirsso binaries land underbin//lib/(right for the systemd + DEB story).Version is automatic
CPACK_PACKAGE_VERSIONand the archive name both derive fromOPENADS_VERSION_STR(git describe), so taggingv1.4.1yieldsopenads-1.4.1-<os>-<arch>with no edits.Clean package
The vendored, statically-linked deps were leaking their own install rules into the package. Disabled:
HTTPLIB_INSTALL/JSON_InstallOFF, and mbedtlsENABLE_PROGRAMS/ENABLE_TESTING/MBEDTLS_INSTALLOFF. Verified the ZIP then contains only OpenADS artifacts — nohttplib.h, no*Config.cmake, no third-partyshare/doc.Validation
cpack -G ZIPoffmain(MSVC x64) produces a clean, correctly-named archive; the normal build is unaffected and-Werrorstays clean. ZIP contents:Notes
openads.ini.sample,QUICKSTART.md,openads-studio.*from feat(serverd): read settings from an openads.ini via --config #88/docs: client onboarding kit — Studio launcher, quick start, migration guide #90) installOPTIONAL-ly, so this PR is self-contained againstmainand they appear automatically once those land.release.ymlhere (that touches CI) — happy to add thecpackstep if you'd prefer, or you can.