Skip to content

Fix installation process and library path for exported target - #12

Open
KunoVonHagen wants to merge 4 commits into
mainfrom
fix/release-tarball
Open

Fix installation process and library path for exported target#12
KunoVonHagen wants to merge 4 commits into
mainfrom
fix/release-tarball

Conversation

@KunoVonHagen

Copy link
Copy Markdown
Collaborator

This pull request updates the C++ client build and packaging workflow to streamline the release process and improve CMake installation handling. The main changes focus on using CMake's install mechanism for packaging, updating workflow steps, and improving installation path handling.

Build and packaging workflow improvements:

  • The GitHub Actions release workflow now uses cmake --install to install build artifacts into the package directory, replacing manual copy commands, and packages the installed files directly. This simplifies the release process and ensures consistency between local and CI builds.

CMake configuration enhancements:

  • Added include(GNUInstallDirs) at the top of CMakeLists.txt to ensure standard installation directories are used.
  • Updated the target_link_libraries install interface to use $<INSTALL_PREFIX> for the static library path, ensuring correct resolution of the install location during package installation.
  • Removed a redundant include(GNUInstallDirs) from the installation section, as it is now included at the top of the file.

@maldwg maldwg left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looking good :)

@stefanDeveloper stefanDeveloper left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good!

@KunoVonHagen

Copy link
Copy Markdown
Collaborator Author

Even though this pull request fixed downstream issues in the C implementation. C++ consumers still experienced issues. The latest commit fixes that. Please re-check the additional changes.

@stefanDeveloper stefanDeveloper left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good from my side.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants