Skip to content

build: install() rules + CPack for reproducible packages#91

Merged
Admnwk merged 1 commit into
FiveTechSoft:mainfrom
Admnwk:feat/cmake-install-cpack
Jun 26, 2026
Merged

build: install() rules + CPack for reproducible packages#91
Admnwk merged 1 commit into
FiveTechSoft:mainfrom
Admnwk:feat/cmake-install-cpack

Conversation

@Admnwk

@Admnwk Admnwk commented Jun 26, 2026

Copy link
Copy Markdown
Collaborator

What & why

The release archives are currently assembled by hand in the release.yml staging steps. This adds CMake install() rules and a CPack config so the same layout is reproducible with cmake --install / cpack, and so distro packaging (DEB) has a real target.

The layout matches the existing release zip:

  • engine DLL (openace64) + a drop-in ACE-named copy (ace64.dll / libace64.so / .dylib)
  • openads_serverd + openads_bench
  • public headers, per-compiler import libs under lib/
  • LICENSE/NOTICE/README + the systemd/launchd templates under service/

Windows stays flat (drop ace64.dll next to the .exe); POSIX uses GNUInstallDirs so binaries land under bin//lib/ (right for the systemd + DEB story).

Version is automatic

CPACK_PACKAGE_VERSION and the archive name both derive from OPENADS_VERSION_STR (git describe), so tagging v1.4.1 yields openads-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_Install OFF, and mbedtls ENABLE_PROGRAMS/ENABLE_TESTING/MBEDTLS_INSTALL OFF. Verified the ZIP then contains only OpenADS artifacts — no httplib.h, no *Config.cmake, no third-party share/doc.

Validation

cpack -G ZIP off main (MSVC x64) produces a clean, correctly-named archive; the normal build is unaffected and -Werror stays clean. ZIP contents:

ace64.dll  openace64.dll  openads_serverd.exe  openads_bench.exe
include/openads/*.h   lib/{msvc,mingw,borland}/*  lib/openace64.lib
LICENSE  NOTICE  README.md   service/{openads-serverd.service,com.openads.serverd.plist}

Notes

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>

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread CMakeLists.txt
Comment on lines +119 to +120
set(ENABLE_PROGRAMS OFF CACHE BOOL "" FORCE)
set(ENABLE_TESTING OFF CACHE BOOL "" FORCE)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The variables ENABLE_PROGRAMS and ENABLE_TESTING are already set to OFF on lines 104 and 105 within the same if(OPENADS_WITH_TLS) block. Redundantly setting them again here is unnecessary.

Comment thread CMakeLists.txt
Comment on lines +389 to +398
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()

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

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()

Comment thread CMakeLists.txt
Comment on lines +421 to +423
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)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

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()

Comment thread CMakeLists.txt
Comment on lines +428 to +432
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)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

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()

@Admnwk Admnwk merged commit 98b6b4a into FiveTechSoft:main Jun 26, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant