Skip to content

Allow install of static slang library #11458

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
jkiviluoto-nv wants to merge 1 commit into
base: master
Choose a base branch
from
+47 −21
Open

Allow install of static slang library #11458

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion .github/cmake-options-matrix.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,8 @@
},
{
"option": "SLANG_LIB_TYPE",
"value": "STATIC"
"value": "STATIC",
"install_test": "true"
},
{
"option": "SLANG_ENABLE_ASAN",
Expand Down
8 changes: 8 additions & 0 deletions .github/workflows/cmake-options-build-container.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,3 +146,11 @@ jobs:
# don't abort the build when ASAN is enabled. No-op for other options.
ASAN_OPTIONS: detect_leaks=0
run: cmake --build build --config "$cmake_config"

# Exercise `cmake --install` for entries that opt in via install_test.
# The static-lib build (SLANG_LIB_TYPE=STATIC) used to skip the
# install(EXPORT ...) step entirely, so a broken export would not have
# been caught by a plain configure+build. See issue #11359.
- name: Install Slang (${{ matrix.option }})
if: matrix.install_test == 'true' && matrix.windows_only != 'true'
run: cmake --install build --config "$cmake_config" --prefix install
Comment on lines +150 to +156

@coderabbitai coderabbitai Bot Jun 3, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial | 🏗️ Heavy lift

Consider validating find_package(slang) after install, not just that cmake --install succeeds.

Issue #11359 manifested as find_package(slang) failing because slangTargets.cmake wasn't installed — yet the install command itself still succeeded with the old guard in place. The "not in any export set" failure is caught at configure/generate time now, but this step alone would not catch a future regression where the install completes but the exported package config is inconsistent. A minimal consumer find_package(slang CONFIG PATHS install ... NO_DEFAULT_PATH) check (resolving slang::slang) would directly guard the originally-reported breakage.

8 changes: 8 additions & 0 deletions .github/workflows/cmake-options-build.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -177,3 +177,11 @@ jobs:
- name: Build Slang (${{ matrix.option }})
if: (matrix.linux_only != 'true' || inputs.os == 'linux') && (matrix.windows_only != 'true' || inputs.os == 'windows') && matrix.container_only != 'true' && (matrix.skip_linux_aarch64 != 'true' || inputs.os != 'linux' || inputs.platform != 'aarch64')
run: cmake --build build --config "$cmake_config"

# Exercise `cmake --install` for entries that opt in via install_test.
# The static-lib build (SLANG_LIB_TYPE=STATIC) used to skip the
# install(EXPORT ...) step entirely, so a broken export would not have
# been caught by a plain configure+build. See issue #11359.
- name: Install Slang (${{ matrix.option }})
if: matrix.install_test == 'true' && (matrix.linux_only != 'true' || inputs.os == 'linux') && (matrix.windows_only != 'true' || inputs.os == 'windows') && matrix.container_only != 'true' && (matrix.skip_linux_aarch64 != 'true' || inputs.os != 'linux' || inputs.platform != 'aarch64')
run: cmake --install build --config "$cmake_config" --prefix install

@github-actions github-actions Bot Jun 3, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Gap: cmake --install doesn't exercise the consumer-side regression of #11359

The reported failure mode in #11359 is find_package(slang) blowing up on a static install because slangConfig.cmake always include()s a slangTargets.cmake that wasn't installed. The new step only runs cmake --install build --config "$cmake_config" --prefix install. That covers the case where install(EXPORT SlangTargets ...) itself errors at install time, but it doesn't exercise the actual user path that broke.

In particular, if a future change re-introduces a guard around just the install(EXPORT ...) call (the exact shape of the original bug), cmake --install still succeeds — it just silently doesn't ship slangTargets.cmake. The regression resurfaces only when a downstream calls find_package(slang), which CI never does.

Suggested fix: chain a tiny find_package smoke test after the install step. The slang_DIR path differs by OS because SLANG_CMAKE_CONFIG_DIR is cmake on Windows and ${CMAKE_INSTALL_LIBDIR}/cmake/${PROJECT_NAME} elsewhere (CMakeLists.txt around line 652).

- name: Smoke-test find_package(slang) against install prefix
  if: matrix.install_test == 'true' && (matrix.linux_only != 'true' || inputs.os == 'linux') && (matrix.windows_only != 'true' || inputs.os == 'windows') && matrix.container_only != 'true' && (matrix.skip_linux_aarch64 != 'true' || inputs.os != 'linux' || inputs.platform != 'aarch64')
  shell: bash
  run: |
    mkdir -p find_pkg_test && cd find_pkg_test
    cat > CMakeLists.txt <<'EOF'
    cmake_minimum_required(VERSION 3.22)
    project(find_slang_smoke LANGUAGES CXX)
    find_package(slang CONFIG REQUIRED)
    if(NOT TARGET slang::slang)
      message(FATAL_ERROR "slang::slang target missing after find_package")
    endif()
    EOF
    cmake -S . -B build -Dslang_ROOT="$GITHUB_WORKSPACE/install"

Same change applies to .github/workflows/cmake-options-build-container.yml.

49 changes: 29 additions & 20 deletions CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -663,29 +663,38 @@ configure_package_config_file(
# Conditionally handle the case for Emscripten where slang does not create
# linkable targets. In this case do not export the targets. Otherwise, just
# export the slang targets.
#
# A STATIC build used to be excluded here because slang's internal link
# dependencies (core, prelude, compiler-core, slang-capability-*,
# slang-lookup-tables, SPIRV-Headers, ...) leaked onto its public link
# interface and weren't in any export set, which made install(EXPORT ...) fail
# (see issue #5821 / PR #6158). slang_add_target now wraps private link deps in
# $<BUILD_LOCAL_INTERFACE:...> / $<BUILD_INTERFACE:...> (cmake/SlangTarget.cmake),
# so those targets no longer appear in the install/export interface and a static
# install exports cleanly. Exporting also for STATIC keeps SlangConfig.cmake's
# unconditional include() of slangTargets.cmake (cmake/SlangConfig.cmake.in)
# satisfied, so find_package(slang) works for static installs too.
if(NOT CMAKE_SYSTEM_NAME STREQUAL "Emscripten")
if(NOT ${SLANG_LIB_TYPE} STREQUAL "STATIC")
install(
EXPORT SlangTargets
FILE ${PROJECT_NAME}Targets.cmake
NAMESPACE ${PROJECT_NAME}::
DESTINATION ${SLANG_CMAKE_CONFIG_DIR}
)
install(
EXPORT SlangTargets
FILE ${PROJECT_NAME}Targets.cmake
NAMESPACE ${PROJECT_NAME}::
DESTINATION ${SLANG_CMAKE_CONFIG_DIR}
)

if(UNIX)
configure_file(
"${PROJECT_SOURCE_DIR}/extras/pkgconfig/slang-compiler.pc.in"
"${PROJECT_BINARY_DIR}/slang-compiler.pc"
@ONLY
)
if(UNIX)
configure_file(
"${PROJECT_SOURCE_DIR}/extras/pkgconfig/slang-compiler.pc.in"
"${PROJECT_BINARY_DIR}/slang-compiler.pc"
@ONLY
)

# TODO: When build system is changed to respect CMAKE_INSTALL_LIBDIR,
# update this destination.
install(
FILES "${PROJECT_BINARY_DIR}/slang-compiler.pc"
DESTINATION "lib/pkgconfig"
)
endif()
# TODO: When build system is changed to respect CMAKE_INSTALL_LIBDIR,
# update this destination.
install(

@github-actions github-actions Bot Jun 3, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Gap: pkgconfig file installed for STATIC builds, but .pc template explicitly does not support static linking

Removing the STATIC guard makes install(FILES "${PROJECT_BINARY_DIR}/slang-compiler.pc" DESTINATION "lib/pkgconfig") fire for UNIX static builds too. extras/pkgconfig/slang-compiler.pc.in is not equipped for that case:

Libs: -L${libdir} -lslang-compiler
Cflags: -I${includedir}

# Note: pkg-config --static is not currently supported,
# so Libs.private is not populated.

Libs: names only -lslang-compiler and Libs.private is empty. After a STATIC install, pkg-config --libs slang-compiler returns an incomplete link line that omits every transitive internal archive (core, compiler-core, slang-capability-*, slang-lookup-tables, plus bundled vendored libs like miniz/lz4 — see docs/building.md listing the static dependency chain). pkg-config users following the manifest will hit unresolved-symbol errors at link time.

Example: a downstream UNIX project does pkg-config --libs slang-compiler after cmake --install of a SLANG_LIB_TYPE=STATIC Slang. They get -L<prefix>/lib -lslang-compiler and ld reports unresolved references into core::* and compiler-core::*. Before this PR, no .pc was installed so the project would fall back to find_package(slang) (which is the well-supported path with this PR).

Suggested fix: either keep the pkgconfig install gated on shared (the simplest patch and consistent with the .pc.in comment), or populate Libs.private with the full static dependency chain when SLANG_LIB_TYPE is STATIC and document that pkg-config --static --libs slang-compiler is required.

if(UNIX AND NOT ${SLANG_LIB_TYPE} STREQUAL "STATIC")
    configure_file(...)
    install(FILES "${PROJECT_BINARY_DIR}/slang-compiler.pc" DESTINATION "lib/pkgconfig")
endif()

FILES "${PROJECT_BINARY_DIR}/slang-compiler.pc"
DESTINATION "lib/pkgconfig"
)
endif()
endif()

Expand Down
Loading