Skip to content

Add more system dependency options to CMake#7987

Merged
samestep merged 4 commits intoshader-slang:masterfrom
niklaskorz:cmake-system-deps
Aug 20, 2025
Merged

Add more system dependency options to CMake#7987
samestep merged 4 commits intoshader-slang:masterfrom
niklaskorz:cmake-system-deps

Conversation

@niklaskorz
Copy link
Copy Markdown
Contributor

@niklaskorz niklaskorz commented Jul 30, 2025

Allows opt-in for sourcing the following dependencies from the system,
instead of using the vendored ones:

  • miniz
  • lz4
  • vulkan-headers
  • spirv-tools
  • glslang

(some of these already had options that weren't working, as either it expected them to be static libraries or it was expecting to be embedded in another CMakeList that should provide the package, instead of finding the package itself)

This is based on a patch we currently maintain inside nixpkgs, but as it
frequently conflicts with new slang releases, it would be nice to see get
this upstream.

Allows opt-in for sourcing the following dependencies from the system,
instead of using the vendored ones:

- miniz
- lz4
- vulkan-headers
- spirv-tools
- glslang
@niklaskorz niklaskorz requested a review from a team as a code owner July 30, 2025 07:51
@CLAassistant
Copy link
Copy Markdown

CLAassistant commented Jul 30, 2025

CLA assistant check
All committers have signed the CLA.

@samestep
Copy link
Copy Markdown
Contributor

For context, this is a followup on the conversation from NixOS/nixpkgs#424153.

@samestep samestep added the pr: non-breaking PRs without breaking changes label Jul 30, 2025
@samestep
Copy link
Copy Markdown
Contributor

/format

@slangbot
Copy link
Copy Markdown
Contributor

🌈 Formatted, please merge the changes from this PR

Copy link
Copy Markdown
Collaborator

@jkwak-work jkwak-work left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@expipiplus1 , can you review since you are more faimilar with CMake setting?

@jkwak-work
Copy link
Copy Markdown
Collaborator

@niklaskorz , After running /format, you need to manually merge the fix.

@bmillsNV bmillsNV requested a review from expipiplus1 July 30, 2025 22:27
@jkwak-work
Copy link
Copy Markdown
Collaborator

This change seems to conflict or duplicate to another PR that was merged recently

@niklaskorz , can you review the PR and see if this PR is still needed?

@samestep
Copy link
Copy Markdown
Contributor

@jkwak-work I merged master into this branch just now and I don't see any conflicts. Could you elaborate on what you see as conflicts or duplication?

Copy link
Copy Markdown
Collaborator

@jkwak-work jkwak-work left a comment

Choose a reason for hiding this comment

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

I made a mistake on my previous comment.
The keywords "Vulkan" and "SPIRV" looked confusingly similar to me.

It looks good to me.

@samestep samestep added this pull request to the merge queue Aug 20, 2025
Merged via the queue into shader-slang:master with commit 619de90 Aug 20, 2025
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr: non-breaking PRs without breaking changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants