conda-forge / gstreamer-feedstock

A conda-smithy repository for gstreamer.
BSD 3-Clause "New" or "Revised" License
9 stars 28 forks source link

Bump to 1.18.0 and add Windows support #43

Closed tschoonj closed 3 years ago

tschoonj commented 3 years ago

Closes #41

conda-forge-linter commented 3 years ago

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe) and found it was in an excellent condition.

I do have some suggestions for making it better though...

For recipe:

Documentation on acceptable licenses can be found here.

conda-forge-linter commented 3 years ago

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe) and found it was in an excellent condition.

tschoonj commented 3 years ago

By the way, I recommend that gst-python, which currently has its own feedstock, is also made part of this recipe, given that all gstreamer packages are released simultaneously.

tschoonj commented 3 years ago

@msarahan @ccordoba12 @mingwandroid

Could someone review this PR please? Thanks!

tschoonj commented 3 years ago

@isuruf @pkgw @xhochy @scopatz

Could you comment on this PR please?

Many thanks in advance!

tschoonj commented 3 years ago

Nice work! This looks very thorough.

Thanks!

I'm not familiar with the details of this package. I noticed two things:

First, adding the "build" prefix to the pkg-config search path is a bit of a red flag, which might indicate future problems cross-compiling. Related are these messages during the build:

Warning: rpath {BLAH}/_build_env/lib is outside prefix {PREFIX} (removing it)

But if the rpath is being removed and the tests are passing, it might all be OK.

This is due to gobject-introspection and meson not playing nice together if they are not together in build. I have done something similar for other GNOME projects as well.

Second, I see these sorts of messages being printed during the test runs:

+ gst-typefind-1.0 --version
gst-typefind-1.0 version 1.18.1
GStreamer 1.18.1
Unknown package origin

Does this mean that GStreamer has some kind of prefix-detection code that isn't working right when conda-ified?

This looks normal to me. I get the exact same output when using GStreamer from Homebrew.

pkgw commented 3 years ago

This is due to gobject-introspection and meson not playing nice together if they are not together in build. I have done something similar for other GNOME projects as well.

Ah, OK. "Someone" really ought to split up the gobject-introspection package — but unfortunately this isn't a task that I can realistically take on any time soon :-(

pkgw commented 3 years ago

Anyone else have any feedback? I will merge in a few days if not.

tschoonj commented 3 years ago

Any thoughts on me adding gst-python to the recipe?

pkgw commented 3 years ago

@tschoonj Thanks for this PR! I would support adding gst-python, but (evidently) in a separate PR to keep the review scope in bounds.