Skip to content
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.

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

Provide SlangConfig.cmake in releases #5649

Open
eliemichel opened this issue Nov 23, 2024 · 6 comments · May be fixed by #5674
Open

Provide SlangConfig.cmake in releases #5649

eliemichel opened this issue Nov 23, 2024 · 6 comments · May be fixed by #5674
Labels
goal:quality & productivity Quality issues and issues that impact our productivity coding day to day inside slang

Comments

@eliemichel
Copy link
Contributor

This is more of a nice-to-have than a must-have, but it would be really helpful that releases provide such a file (generated by CMake's install process) to ease the import of precompiled binaries in CMake setups!

@ov-l
Copy link

ov-l commented Nov 25, 2024

I can make a pull-request for this since I already have this setup in my local branch.

@ov-l ov-l linked a pull request Nov 25, 2024 that will close this issue
@bmillsNV
Copy link
Collaborator

Thanks! ov-l. Assigned the issue to you :)

@bmillsNV bmillsNV added this to the Q1 2025 (Winter) milestone Nov 26, 2024
@bmillsNV bmillsNV added the goal:quality & productivity Quality issues and issues that impact our productivity coding day to day inside slang label Nov 26, 2024
@eliemichel
Copy link
Contributor Author

eliemichel commented Nov 26, 2024

Side note: you might have to be careful as of what is imported in case of cross-compilation (e.g., to WebAssembly), though I'm not used to configuring CMake installs so it might work out of the box.

For my use case I've had to manually deal with it so that slangc executable is setup for the native system while slang library is setup for the target system: FetchSlang.cmake

@ov-l
Copy link

ov-l commented Nov 26, 2024

I added slang as the only target because I just needed that for my use case. If you intend to link with slang, that should work out of the box. slangc on the other hand would be a different target which I have not included in the export target list, and the executable is already installed by default by the existing cmake script (if you build the executable).
With modern CMake, if you just link with slang::slang, it should be all good on all platforms normally, unless something is wrong in the original CMake script with target setup. I personally tested on linux and windows, both work fine.

@ov-l ov-l removed their assignment Nov 26, 2024
@jkwak-work
Copy link
Collaborator

I am having a hard time figuring out what is requested initially.
It says "import of precompiled binaries", and I am not sure where it is supposed to be imported to.
And what is the use case for this.

@eliemichel , can you clarify my questions so that we can provide a proper support?

@eliemichel
Copy link
Contributor Author

eliemichel commented Nov 26, 2024

@eliemichel , can you clarify my questions so that we can provide a proper support?

Sure, I meant "import" as in CMake's vocable of "imported targets" that can be manually created with e.g., add_library(foo STATIC IMPORTED). The typical way to equip a binary release of a library (be it static or shared) so that it can be properly imported by CMake's find_package mechanism (in so-called "Config" mode) is to provide a SlangConfig.cmake file (in lib/cmake IIRC).

In our case, that would mean if download and unzip a binary release to /path/to/slang, then configure a project with cmake -DSlang_DIR=/path/to/slang (or add this path to CMAKE_PREFIX_PATH): after that simply calling find_package(Slang REQUIRED) in my CMakeLists.txt would make available the Slang targets (static, shared, executable targets -- whether the release provides).

Aside from manual download, this is also very compatible with FetchContent-based approaches to fetch the precompiled binaries and internally (i.e., inside some FetchSlang.cmake's dynamics) automatically set Slang_DIR to the place where it did download the release.

I believe the approach proposed in #5674 is sound, although I wouldn't consider myself an expert in CMake's import/export mechanism. But let me know if there is actually another recommended way of doing all this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
goal:quality & productivity Quality issues and issues that impact our productivity coding day to day inside slang
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants