Bug 227579 – Bundle libraries for remote execution in run-jsc-benchmarks

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Angelos Oikonomopoulos]

Bundle libraries for remote execution in run-jsc-benchmarks

[Angelos Oikonomopoulos]

Created attachment 432676 [details]
Patch

[Angelos Oikonomopoulos]

Created attachment 432779 [details]
Patch

[Angelos Oikonomopoulos]

Created attachment 432792 [details]
Patch

[Radar WebKit Bug Importer]

<rdar://problem/80316541>

[Carlos Alberto Lopez Perez]

Comment on attachment 432792 [details]
Patch

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

The code looks fine, however when I tested it I found unexpected errors.. that is why I'm setting r-

It seems that generating the MiniBrowser bundle with generate-bundle is broken after this patch

$ Tools/Scripts/generate-bundle --platform wpe --release --bundle MiniBrowser 
Traceback (most recent call last):
  File "Tools/Scripts/generate-bundle", line 606, in <module>
    sys.exit(main())
  File "Tools/Scripts/generate-bundle", line 596, in main
    bundle_file_path = bundle_creator.create()
  File "Tools/Scripts/generate-bundle", line 254, in create
    self._create_bundle(bundle_binary)
  File "Tools/Scripts/generate-bundle", line 388, in _create_bundle
    gio_modules = self._get_gio_modules()
  File "Tools/Scripts/generate-bundle", line 327, in _get_gio_modules
    retcode, stdout, stderr = self._run_cmd_and_get_output(['pkg-config', '--variable=giomoduledir', 'gio-2.0'])
AttributeError: 'BundleCreator' object has no attribute '_run_cmd_and_get_output'

> Tools/Scripts/bundle-binary:1
> +#!/usr/bin/env python3

Please add the usual copyright boilerplate at the top of each new code file, is the policy of the project to do that.

> Tools/Scripts/webkitpy/binary_bundling/bundle.py:1
> +import logging

Missing copyright boilerplate also here

> Tools/Scripts/webkitpy/binary_bundling/ldd.py:1
> +import logging

Also missing copyright

[Angelos Oikonomopoulos]

Created attachment 433317 [details]
Patch

[Angelos Oikonomopoulos]

Created attachment 433319 [details]
Patch

[Angelos Oikonomopoulos]

(In reply to Carlos Alberto Lopez Perez from comment #5)
> Comment on attachment 432792 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=432792&action=review
> 
> The code looks fine, however when I tested it I found unexpected errors..
> that is why I'm setting r-
> 
> It seems that generating the MiniBrowser bundle with generate-bundle is
> broken after this patch
> 
> $ Tools/Scripts/generate-bundle --platform wpe --release --bundle
> MiniBrowser 
> Traceback (most recent call last):
>   File "Tools/Scripts/generate-bundle", line 606, in <module>
>     sys.exit(main())
>   File "Tools/Scripts/generate-bundle", line 596, in main
>     bundle_file_path = bundle_creator.create()
>   File "Tools/Scripts/generate-bundle", line 254, in create
>     self._create_bundle(bundle_binary)
>   File "Tools/Scripts/generate-bundle", line 388, in _create_bundle
>     gio_modules = self._get_gio_modules()
>   File "Tools/Scripts/generate-bundle", line 327, in _get_gio_modules
>     retcode, stdout, stderr = self._run_cmd_and_get_output(['pkg-config',
> '--variable=giomoduledir', 'gio-2.0'])
> AttributeError: 'BundleCreator' object has no attribute
> '_run_cmd_and_get_output'

Oops. Did some more testing of the bundling code after fixing this. AFAICT the only difference that diff -Nur finds is the INTERPRETER= line being moved earlier in the generated script. Tested with the flatpak build with/without generate-install-script and with the jhbuild.

> > Tools/Scripts/bundle-binary:1
> > +#!/usr/bin/env python3
> 
> Please add the usual copyright boilerplate at the top of each new code file,
> is the policy of the project to do that.

Thanks, should be there now.

[Carlos Alberto Lopez Perez]

Comment on attachment 433319 [details]
Patch

Patch looks fine! thanks!
BTW, a suggestion to make the bundle-binary script jhbuild/flatpak aware is this diff over your patch: http://sprunge.us/BB3T7i
It adds an optional platform parameter, that when enabled will try to detect jhbuild/flatpak detection on gtk/wpe ports.
So it looks for libraries inside the flatpak container or the jhbuild directories.
If platform is not passed (like when called from the run-jsc-benchmark script then it behaves like now.. doesn't try to detect jhbuild/flatpak which is not used on javascriptonly builds)

[Angelos Oikonomopoulos]

(In reply to Carlos Alberto Lopez Perez from comment #9)
> Comment on attachment 433319 [details]
> Patch
> 
> Patch looks fine! thanks!
> BTW, a suggestion to make the bundle-binary script jhbuild/flatpak aware is
> this diff over your patch: http://sprunge.us/BB3T7i
> It adds an optional platform parameter, that when enabled will try to detect
> jhbuild/flatpak detection on gtk/wpe ports.
> So it looks for libraries inside the flatpak container or the jhbuild
> directories.
> If platform is not passed (like when called from the run-jsc-benchmark
> script then it behaves like now.. doesn't try to detect jhbuild/flatpak
> which is not used on javascriptonly builds)

Thanks for the suggestion! ATM (in this patch), `bundle-binary` is only used non-interactively from run-jsc-stress-tests and run-jsc-benchmarks. IIUC, it will work for e.g. a flatpak setup as long as it's run from within the flatpak container (as for most other scripts). So not sure that explicitly adding flatpak/jhbuild support is needed at this point. But open to adding such if there's some interactive use I'm unaware of.

[EWS]

Committed r279984 (239727@main): <https://commits.webkit.org/239727@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 433319 [details].

[Angelos Oikonomopoulos]

*** Bug 189404 has been marked as a duplicate of this bug. ***