Bug 233570 – [Flatpak SDK] Update to FDO 21.08.6 release

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Philippe Normand]

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

[Philippe Normand]

Created attachment 445290 [details]
[fast-cq] Patch

[Adrian Perez]

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.

[Philippe Normand]

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.

[Philippe Normand]

(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

[Philippe Normand]

(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

[Adrian Perez]

(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 😅️

[Adrian Perez]

(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.

[EWS]

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].

[Radar WebKit Bug Importer]

<rdar://problem/85859243>