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

libplacebo: fix dependency linkage #153961

Merged
merged 2 commits into from
Nov 15, 2023
Merged

Conversation

rcombs
Copy link
Contributor

@rcombs rcombs commented Nov 11, 2023

  • Have you followed the guidelines for contributing?
  • Have you ensured that your commits follow the commit style guide?
  • Have you checked that there aren't other open pull requests for the same formula update/change?
  • Have you built your formula locally with HOMEBREW_NO_INSTALL_FROM_API=1 brew install --build-from-source <formula>, where <formula> is the name of the formula you're submitting?
  • Is your test running fine brew test <formula>, where <formula> is the name of the formula you're submitting?
  • Does your build pass brew audit --strict <formula> (after doing HOMEBREW_NO_INSTALL_FROM_API=1 brew install --build-from-source <formula>)? If this is a new formula, does it pass brew audit --new <formula>?

All of libplacebo's dependencies are technically optional, but some of them are essential to calling packages (e.g. mpv). In particular, much of the library's functionality depends on linking against either shaderc or glslang, and being compiled against vulkan headers, but the build won't fail by default in the absence of either.

Currently, we're failing to pull in any of these dependencies in CI builds, for a few different reasons, all of which are resolved here:

  • We were missing a build-time dependency on pkg-config; added one
  • We attempted to link against glslang, but meson was unable to find it; fixed by switching to shaderc, which provides a .pc file that meson can find (shaderc is also preferred by libplacebo; it only uses glslang as a wrapper around it)
  • We didn't pass -D[dep]=enabled args, so failures to link deps were ignored; added them
  • We had a dependency on sdl2, but it's been unused for a while, as libplacebo only uses SDL for the plplay test utility, which is only built if depending on ffmpeg, which was removed in libplacebo: remove ffmpeg dependency #132349 (that removal is probably still reasonable, as the ffmpeg package will probably later want to depend on libplacebo for functionality more useful to most people than a test program)

This fixes the usage of libplacebo in mpv (the only current dependent package), which was almost entirely broken previously, at least in CI builds.

@github-actions github-actions bot added the autosquash Automatically squash pull request commits according to Homebrew style. label Nov 11, 2023
@iMichka iMichka mentioned this pull request Nov 15, 2023
6 tasks
@iMichka
Copy link
Member

iMichka commented Nov 15, 2023

Looks great. Would it be possible to squash the 4 commits into one and force push to your branch?

- depend on pkg-config at build-time
- link against shaderc instead of glslang
- ensure we link against the dependencies we expect to
- remove unused dependency on sdl2

The sdl3 dep has been unused since Homebrew#132349 removed the dependency on ffmpeg, which caused the plplay program no longer to be built; plplay was the only component of libplacebo that depended on sdl2.
@rcombs
Copy link
Contributor Author

rcombs commented Nov 15, 2023

@iMichka done.

@github-actions github-actions bot removed the autosquash Automatically squash pull request commits according to Homebrew style. label Nov 15, 2023
Copy link
Contributor

🤖 An automated task has requested bottles to be published to this PR.

@github-actions github-actions bot added the CI-published-bottle-commits The commits for the built bottles have been pushed to the PR branch. label Nov 15, 2023
@BrewTestBot BrewTestBot added this pull request to the merge queue Nov 15, 2023
Merged via the queue into Homebrew:master with commit d80562f Nov 15, 2023
12 checks passed
@github-actions github-actions bot added the outdated PR was locked due to age label Dec 16, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 16, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
CI-published-bottle-commits The commits for the built bottles have been pushed to the PR branch. outdated PR was locked due to age
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants