rust-av / dav1d-rs

libdav1d rust bindings
MIT License
40 stars 20 forks source link

Build failure with SYSTEM_DEPS_DAV1D_BUILD_INTERNAL=always #85

Closed fintelia closed 1 year ago

fintelia commented 1 year ago

I haven't had a chance to investigate much, but since the release of 0.9.5, I've been seeing CI failures for image crate builds that use dav1d. I believe the relevant output is...

  thread 'main' panicked at /home/runner/.cargo/registry/src/index.crates.io-6f17d22bba15001f/dav1d-sys-0.7.2/build.rs:83:10:
  called `Result::unwrap()` on an `Err` value: `PKG_CONFIG_ALLOW_SYSTEM_CFLAGS="1" PKG_CONFIG_ALLOW_SYSTEM_LIBS="1" PKG_CONFIG_PATH="" "pkg-config" "--libs" "--cflags" "dav1d" "dav1d >= 1.0.0" "dav1d <= 1.2.1"` did not exit successfully: exit status: 1
  error: could not find system library 'dav1d' required by the 'dav1d-sys' crate

  --- stderr
  Package dav1d was not found in the pkg-config search path.
  Perhaps you should add the directory containing `dav1d.pc'
  to the PKG_CONFIG_PATH environment variable
  No package 'dav1d' found
  Package dav1d was not found in the pkg-config search path.
  Perhaps you should add the directory containing `dav1d.pc'
  to the PKG_CONFIG_PATH environment variable
  No package 'dav1d' found
  Package dav1d was not found in the pkg-config search path.
  Perhaps you should add the directory containing `dav1d.pc'
  to the PKG_CONFIG_PATH environment variable
  No package 'dav1d' found
sdroege commented 1 year ago

Indeed, that makes sense. Sorry for missing that. The cause is in https://github.com/rust-av/dav1d-rs/commit/3d8c51489049ec44a4c55d498341eea289fa7b05

It's unclear to me how we can handle this correctly as system-deps would decide whether to build the internal version or not. So we would have to be able to pass the upper bound there instead of checking ourselves.

@gdesmott any ideas?

sdroege commented 1 year ago

We'd need https://github.com/gdesmott/system-deps/issues/60 I guess.

gdesmott commented 1 year ago

We'd need gdesmott/system-deps#60 I guess.

Yeah I think so.

sdroege commented 1 year ago

Let's do that then

sdroege commented 1 year ago

https://github.com/gdesmott/system-deps/pull/82 should allow us to fix this.