Bundle libraries for remote execution in run-jsc-benchmarks
Created attachment 432676 [details] Patch
Created attachment 432779 [details] Patch
Created attachment 432792 [details] Patch
<rdar://problem/80316541>
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
Created attachment 433317 [details] Patch
Created attachment 433319 [details] Patch
(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.
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)
(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.
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].
*** Bug 189404 has been marked as a duplicate of this bug. ***