Bug 220418 – [Flatpak SDK] Updates for gst-build and gst-plugins-rs support

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

[Philippe Normand]

- for gst-build support: gst-plugins-rs git master requires Meson 0.56 and cargo-c
- otherwise, it would be nice to have the rsclosedcaption plugin in the SDK, we'll need some of its CEA608 elements soon for the improved text-combiner and playbin3

[Philippe Normand]

Created attachment 417182 [details]
Patch

[Lauro Moura]

Built and run test tests normally.

Just had an issue with gst-build unrelated to this and already present in the current tree.

[Philippe Normand]

Committed r271518: <https://trac.webkit.org/changeset/271518>

[Radar WebKit Bug Importer]

<rdar://problem/73246162>

[Adrian Perez]

Comment on attachment 417182 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=417182&action=review

> Tools/buildstream/elements/sdk/cargo-c.bst:17
> +    cargo --offline build --release

Wouldn't the cargo invocation belong into “build-commands”? Something like:

  config:
    build-commands:
    - |
      cargo --offline build --release
    install-commands:
    - |
      install -D -m a+rx -t "%{install-root}%{bindir}" ./target/release/cargo-cbuild

This allows Buildstream to better handle caching of build artifacts. For example it may
choose to reuse build artifacts in a subsequent run and only run the installation phase
if that was interrupted for some reason (IIRC there were other cases in which install
may re-run without full build as well.)

The same goes for the other .bst files which are invoking cargo in the installation
commands :)

[Philippe Normand]

(In reply to Adrian Perez from comment #5)
> Comment on attachment 417182 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=417182&action=review
> 
> > Tools/buildstream/elements/sdk/cargo-c.bst:17
> > +    cargo --offline build --release
> 
> Wouldn't the cargo invocation belong into “build-commands”? Something like:
> 
>   config:
>     build-commands:
>     - |
>       cargo --offline build --release
>     install-commands:
>     - |
>       install -D -m a+rx -t "%{install-root}%{bindir}"
> ./target/release/cargo-cbuild
> 
> This allows Buildstream to better handle caching of build artifacts. For
> example it may
> choose to reuse build artifacts in a subsequent run and only run the
> installation phase
> if that was interrupted for some reason (IIRC there were other cases in
> which install
> may re-run without full build as well.)
> 
> The same goes for the other .bst files which are invoking cargo in the
> installation
> commands :)

Yes, good point. I'll follow-up as I mistakenly landed this too early. Sorry ;)

[Adrian Perez]

(In reply to Philippe Normand from comment #6)
> (In reply to Adrian Perez from comment #5)
> > Comment on attachment 417182 [details]
> > Patch
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=417182&action=review
> > 
> > > Tools/buildstream/elements/sdk/cargo-c.bst:17
> > > +    cargo --offline build --release
> > 
> > Wouldn't the cargo invocation belong into “build-commands”? Something like:
> > 
> >   config:
> >     build-commands:
> >     - |
> >       cargo --offline build --release
> >     install-commands:
> >     - |
> >       install -D -m a+rx -t "%{install-root}%{bindir}"
> > ./target/release/cargo-cbuild
> > 
> > This allows Buildstream to better handle caching of build artifacts. For
> > example it may
> > choose to reuse build artifacts in a subsequent run and only run the
> > installation phase
> > if that was interrupted for some reason (IIRC there were other cases in
> > which install
> > may re-run without full build as well.)
> > 
> > The same goes for the other .bst files which are invoking cargo in the
> > installation
> > commands :)
> 
> Yes, good point. I'll follow-up as I mistakenly landed this too early. Sorry
> ;)

No problem at all!