Bug 220418 - [Flatpak SDK] Updates for gst-build and gst-plugins-rs support
Summary: [Flatpak SDK] Updates for gst-build and gst-plugins-rs support
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Philippe Normand
URL:
Keywords: InRadar
Depends on: 220237
Blocks: 220654
  Show dependency treegraph
 
Reported: 2021-01-07 08:47 PST by Philippe Normand
Modified: 2021-01-15 07:19 PST (History)
3 users (show)

See Also:


Attachments
Patch (55.94 KB, patch)
2021-01-07 08:52 PST, Philippe Normand
ews-feeder: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Philippe Normand 2021-01-07 08:47:15 PST
- 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
Comment 1 Philippe Normand 2021-01-07 08:52:25 PST
Created attachment 417182 [details]
Patch
Comment 2 Lauro Moura 2021-01-08 09:45:09 PST
Built and run test tests normally.

Just had an issue with gst-build unrelated to this and already present in the current tree.
Comment 3 Philippe Normand 2021-01-15 06:03:51 PST
Committed r271518: <https://trac.webkit.org/changeset/271518>
Comment 4 Radar WebKit Bug Importer 2021-01-15 06:04:13 PST
<rdar://problem/73246162>
Comment 5 Adrian Perez 2021-01-15 06:10:49 PST
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 :)
Comment 6 Philippe Normand 2021-01-15 06:14:01 PST
(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 ;)
Comment 7 Adrian Perez 2021-01-15 07:19:37 PST
(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!