-
Notifications
You must be signed in to change notification settings - Fork 6.6k
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
[steam-audio] Adding port #40942
[steam-audio] Adding port #40942
Conversation
Friendly bump that this is ready for review 😃 |
@Honeybunch, please mark "ready for review" once you have responded to @FrankXie05 / @dg0yt review comments. Thanks! |
I am trying to upstream what parts of this patch I can but the Steam-Audio repo hasn't seen activity for two months so it may take a while |
…arty licenses, and avoid installing 'help' files.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for the delay on the review, looks like this one fell through the cracks. I pushed some nitpick fixes to clean up some meaningless files in the installed package, and make sure the right third party notices were included as they were being installed to $/root/THIRDPARTY.md
rather than being part of the copyright
file.
This looks good to me now. Can you answer my question about -DVCPKG_HOST_TRIPLET=${HOST_TRIPLET}
?
Thanks for your contribution to vcpkg!
ports/steam-audio/portfile.cmake
Outdated
# So the patched port can find the vcpkg host flatc compiler | ||
-DVCPKG_HOST_TRIPLET=${HOST_TRIPLET} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As far as I can see nothing looks for VCPKG_HOST_TRIPLET
or HOST_TRIPLET
, is this leftover from an earlier revision and no longer used?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah I'm pretty sure that's cruft and can be removed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It changes how vcpkg.cmake configures the search path for programs. Cf.
vcpkg/scripts/buildsystems/vcpkg.cmake
Lines 559 to 571 in 561cf50
option(VCPKG_SETUP_CMAKE_PROGRAM_PATH "Enable the setup of CMAKE_PROGRAM_PATH to vcpkg paths" ON) | |
set(VCPKG_CAN_USE_HOST_TOOLS OFF) | |
if(DEFINED VCPKG_HOST_TRIPLET AND NOT VCPKG_HOST_TRIPLET STREQUAL "") | |
set(VCPKG_CAN_USE_HOST_TOOLS ON) | |
endif() | |
cmake_dependent_option(VCPKG_USE_HOST_TOOLS "Setup CMAKE_PROGRAM_PATH to use host tools" ON "VCPKG_CAN_USE_HOST_TOOLS" OFF) | |
unset(VCPKG_CAN_USE_HOST_TOOLS) | |
if(VCPKG_SETUP_CMAKE_PROGRAM_PATH) | |
set(tools_base_path "${VCPKG_INSTALLED_DIR}/${VCPKG_TARGET_TRIPLET}/tools") | |
if(VCPKG_USE_HOST_TOOLS) | |
set(tools_base_path "${VCPKG_INSTALLED_DIR}/${VCPKG_HOST_TRIPLET}/tools") | |
endif() |
Also #25529.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh damn yeah good catch. It's been a hot minute since I've last looked at this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmmm... that seems extremely cursed @dg0yt but not looking to un-curse it right now.
We're only building core
.
Flatbuffers: Explicitly passed in
ISPC: Guarded by STEAMAUDIO_ENABLE_EMBREE
, which is disabled
Sphinx: Guarded by STEAMAUDIO_BUILD_DOCS
, which is disabled
I'll remove it.
FrankXie is out of office and their comments appear to have been addressed.
Steam Audio has a new version (4.6.0). In theory it should be trivial to upgrade (I think?) |
@ethindp Maybe we can wait for PR ValveSoftware/steam-audio#399 to be merged into the next version before updating. :) |
Co-authored-by: Kai Pastor <[email protected]> Co-authored-by: Billy Robert O'Neal III <[email protected]>
Adding this port for those of us who crave spatial audio
find_package
calls are REQUIRED, are satisfied byvcpkg.json
's declared dependencies, or disabled with CMAKE_DISABLE_FIND_PACKAGE_Xxx.vcpkg.json
matches what upstream says.vcpkg.json
matches what upstream says../vcpkg x-add-version --all
and committing the result.