Also including a few changes I had in the back-burner for several weeks: - update to GStreamer 1.18.5 - update internal pipenv used by bst to pygobject 3.42 (needed for f35 host) - minor drive-by fixes in gst recipes correcting docker image export
Created attachment 445290 [details] [fast-cq] Patch
Comment on attachment 445290 [details] [fast-cq] Patch Patch LGTM, I wrote a couple of minor comments, though. View in context: https://bugs.webkit.org/attachment.cgi?id=445290&action=review > Tools/buildstream/elements/sdk/gst-libav.bst:26 > + PAGER=this-is-not-a-pager gst-inspect-1.0 libav I suppose using PAGER='' to leave it empty may not work if gst-inspect-1.0 uses some hardcoded default like trying to search for “less“… have you tried that? Another option could be PAGER=cat but honestly the chance of something called “this-is-not-a-pager“ being installed and executable is virtually zero so let's just leave it as-is 😜️ > Tools/buildstream/elements/sdk/gst-plugins-base.bst:-45 > - Is there any reason to remove this empty line? Maybe we can leave it to avoid bringing noise into the diff—and also I find the YAML more legible keeping this line. > Tools/buildstream/elements/sdk/gstreamer.bst:-9 > - Same for this empty line here. > Tools/buildstream/elements/sdk/gstreamer.bst:-34 > - …and this one here.
Comment on attachment 445290 [details] [fast-cq] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=445290&action=review >> Tools/buildstream/elements/sdk/gst-plugins-base.bst:-45 >> - > > Is there any reason to remove this empty line? Maybe we can leave it to avoid > bringing noise into the diff—and also I find the YAML more legible keeping this > line. that's `bst track` fault.
(In reply to Adrian Perez from comment #2) > Comment on attachment 445290 [details] > Patch > > Patch LGTM, I wrote a couple of minor comments, though. > > View in context: > https://bugs.webkit.org/attachment.cgi?id=445290&action=review > > > Tools/buildstream/elements/sdk/gst-libav.bst:26 > > + PAGER=this-is-not-a-pager gst-inspect-1.0 libav > > I suppose using PAGER='' to leave it empty may not work if gst-inspect-1.0 > uses > some hardcoded default like trying to search for “less“… have you tried that? PAGER="" won't work I'm afraid, if I understand this code anyway: https://gitlab.freedesktop.org/gstreamer/gstreamer/-/blob/main/subprojects/gstreamer/tools/gst-inspect.c#L1959
(In reply to Philippe Normand from comment #4) > (In reply to Adrian Perez from comment #2) > > Comment on attachment 445290 [details] > > Patch > > > > Patch LGTM, I wrote a couple of minor comments, though. > > > > View in context: > > https://bugs.webkit.org/attachment.cgi?id=445290&action=review > > > > > Tools/buildstream/elements/sdk/gst-libav.bst:26 > > > + PAGER=this-is-not-a-pager gst-inspect-1.0 libav > > > > I suppose using PAGER='' to leave it empty may not work if gst-inspect-1.0 > > uses > > some hardcoded default like trying to search for “less“… have you tried that? > > PAGER="" won't work I'm afraid, if I understand this code anyway: > https://gitlab.freedesktop.org/gstreamer/gstreamer/-/blob/main/subprojects/ > gstreamer/tools/gst-inspect.c#L1959 And that's a GStreamer bug actually, PAGER= gst-inspect-1.0 crashes
(In reply to Philippe Normand from comment #3) > Comment on attachment 445290 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=445290&action=review > > >> Tools/buildstream/elements/sdk/gst-plugins-base.bst:-45 > >> - > > > > Is there any reason to remove this empty line? Maybe we can leave it to avoid > > bringing noise into the diff—and also I find the YAML more legible keeping this > > line. > > that's `bst track` fault. Ah, one of those little annoyances of automated tools. Probably the reason why I prefer to update refs by hand 😅️
(In reply to Philippe Normand from comment #5) > (In reply to Philippe Normand from comment #4) > > (In reply to Adrian Perez from comment #2) > > > Comment on attachment 445290 [details] > > > Patch > > > > > > Patch LGTM, I wrote a couple of minor comments, though. > > > > > > View in context: > > > https://bugs.webkit.org/attachment.cgi?id=445290&action=review > > > > > > > Tools/buildstream/elements/sdk/gst-libav.bst:26 > > > > + PAGER=this-is-not-a-pager gst-inspect-1.0 libav > > > > > > I suppose using PAGER='' to leave it empty may not work if gst-inspect-1.0 > > > uses > > > some hardcoded default like trying to search for “less“… have you tried that? > > > > PAGER="" won't work I'm afraid, if I understand this code anyway: > > https://gitlab.freedesktop.org/gstreamer/gstreamer/-/blob/main/subprojects/ > > gstreamer/tools/gst-inspect.c#L1959 > > And that's a GStreamer bug actually, PAGER= gst-inspect-1.0 crashes Yeah, I imagined setting it to empty probably wasn't working, hence the r+ anyway — I was just wondering. wondering.
Committed r286290 (244649@main): <https://commits.webkit.org/244649@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 445290 [details].
<rdar://problem/85859243>