Skip to content

Allow installing static lib - #169

Merged
hobu merged 8 commits into
hobuinc:masterfrom
m-kuhn:build_static
Jan 2, 2026
Merged

Allow installing static lib#169
hobu merged 8 commits into
hobuinc:masterfrom
m-kuhn:build_static

Conversation

@m-kuhn

@m-kuhn m-kuhn commented Nov 3, 2025

Copy link
Copy Markdown
Contributor

This builds and installs either the static or shared lib.
The behavior is controlled through BUILD_SHARED_LIBS.

cmake -DBUILD_SHARED_LIBS=OFF will build as static library.

The default is a dynamic library to replicate the original behavior of installing a shared library if no options are provided.

Aligns the target namespace with the package name (lowercase lazperf).
Internally uses the namespaced target lazperf::lazperf for linking.

Implements #168

@dg0yt dg0yt 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.

Only a cursory review, coming from the vcpkg PR.

Comment thread cpp/CMakeLists.txt Outdated
Comment thread cpp/CMakeLists.txt Outdated
lazperf_add_library(${LAZPERF_SHARED_LIB} SHARED ${SRCS})
endif()
lazperf_add_library(${LAZPERF_STATIC_LIB} STATIC ${SRCS})
lazperf_add_library(${LAZPERF_LIB} ${SRCS})

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

If you want the lib target to stand out where used here and downstream, do the next step and add a namespaced alias.

Suggested change
lazperf_add_library(${LAZPERF_LIB} ${SRCS})
lazperf_add_library(lazperf ${SRCS})
add_library(lazperf::lazperf ALIAS lazperf)

(IDK if it should be here or in lazperf_add_library.)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

There's a namespace defined in install.cmake, would that need to be dropped instead?

NAMESPACE
LAZPERF::

@dg0yt dg0yt Nov 3, 2025

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

If the namespace is already defined, it should be used.
Nothing needs to be dropped from installation. The proposed alias is for sub-project usage.
So it wouldn't matter downstream if the project is used as a sub-project or from an installation.

(LAZPERF::lazperf? TBH I prefer when package name, namespace and main target do not use different spellings. There could be LAZPERF::LAZPERF or lazperf::lazperf.)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Proposed lazperf::lazperf now since package and target already are lowercase, so namespace is now aligned too. I am open to reverting that if this is not desired.

Comment thread cpp/emscripten/CMakeLists.txt Outdated
@m-kuhn

m-kuhn commented Nov 3, 2025

Copy link
Copy Markdown
Contributor Author

Thanks @dg0yt

@hobu

hobu commented Nov 11, 2025

Copy link
Copy Markdown
Collaborator

Windows and wasm builds not healthy with these changes.

LAZPERF_EXPORT is only redefined to dllimport on installation, this isn't functional inside the source code
@hobu

hobu commented Nov 12, 2025

Copy link
Copy Markdown
Collaborator

wasm build still failing with

CMake Error at cmake/functions.cmake:4 (lazperf_install_library):
Unknown CMake command "lazperf_install_library".

Comment thread cmake/functions.cmake
Comment thread cpp/CMakeLists.txt Outdated
@hobu
hobu merged commit 00ed6d6 into hobuinc:master Jan 2, 2026
4 checks passed
@m-kuhn
m-kuhn deleted the build_static branch January 2, 2026 12:32
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.

3 participants