Bug 212077 - [Flatpak SDK] Add bubblewrap ... wrapper
Summary: [Flatpak SDK] Add bubblewrap ... wrapper
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: WebKit Local Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Philippe Normand
URL:
Keywords: InRadar
Depends on:
Blocks: 212326
  Show dependency treegraph
 
Reported: 2020-05-19 08:59 PDT by Philippe Normand
Modified: 2020-05-24 05:53 PDT (History)
3 users (show)

See Also:


Attachments
Patch (9.18 KB, patch)
2020-05-19 09:02 PDT, Philippe Normand
no flags Details | Formatted Diff | Diff
Patch (9.23 KB, patch)
2020-05-21 07:23 PDT, Philippe Normand
no flags Details | Formatted Diff | Diff
Patch (9.59 KB, patch)
2020-05-22 02:04 PDT, Philippe Normand
no flags Details | Formatted Diff | Diff
Patch (9.57 KB, patch)
2020-05-22 07:14 PDT, Philippe Normand
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Philippe Normand 2020-05-19 08:59:19 PDT
Allowing us to use flatpak run now.
Comment 1 Philippe Normand 2020-05-19 09:02:58 PDT
Created attachment 399740 [details]
Patch
Comment 2 Carlos Alberto Lopez Perez 2020-05-19 09:26:22 PDT
Comment on attachment 399740 [details]
Patch

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

> Tools/ChangeLog:8
> +        The bind-mounts are now handled through webkit-bwrap.

It looks to me some binds mounts are gone with this new way of doing things.

> Tools/flatpak/flatpakutils.py:-661
> -                           "--bind-mount=/run/shm=/dev/shm",

I don't see this now bind-mounted with the new setup. Why is that?

> Tools/flatpak/flatpakutils.py:-668
> -                           "--bind-mount=/run/systemd/journal=/run/systemd/journal",

neither this, isn't it needed?

> Tools/flatpak/flatpakutils.py:-715
> -                    flatpak_command.append("--bind-mount={uid_doc_path}={uid_doc_path}".format(uid_doc_path=uid_doc_path))

I also don't see this mounted
Comment 3 Philippe Normand 2020-05-19 09:37:57 PDT
That's correct, flatpak run sets those bind-mounts already, while flatpak build doesn't.
Comment 4 Philippe Normand 2020-05-20 01:29:57 PDT
Comment on attachment 399740 [details]
Patch

I couldn't reproduce the api-gtk failures here.
Comment 5 EWS 2020-05-20 01:37:17 PDT
Committed r261909: <https://trac.webkit.org/changeset/261909>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 399740 [details].
Comment 6 Radar WebKit Bug Importer 2020-05-20 01:38:14 PDT
<rdar://problem/63437366>
Comment 7 Diego Pino 2020-05-20 07:32:20 PDT
Reverted r261909 for reason:

execution of LayoutTests returns syntax error in the bots (GTK, WPE)

Committed r261917: <https://trac.webkit.org/changeset/261917>
Comment 8 Philippe Normand 2020-05-20 08:05:31 PDT
python ./Tools/Scripts/run-webkit-tests --no-build --no-show-results --no-new-test-results --clobber-old-results --builder-name 'GTK Linux 64-bit Release (Tests)' --build-number 13781 --buildbot-worker gtk-linux-slave-6 --master-name webkit.org --buildbot-master build.webkit.org --report https://results.webkit.org --exit-after-n-crashes-or-timeouts 50 --exit-after-n-failures 500 --release --layout-tests-directory ./Tools/BuildSlaveSupport/build.webkit.org-config/public_html/dashboard/Scripts/tests --gtk --results-directory layout-test-results/dashboard-layout-test-results --debug-rwt-logging
 in dir /home/slave/webkitgtk/gtk-linux-64-release-tests/build (timeout 1200 secs)
 watching logfiles {}
 argv: ['python', './Tools/Scripts/run-webkit-tests', '--no-build', '--no-show-results', '--no-new-test-results', '--clobber-old-results', '--builder-name', 'GTK Linux 64-bit Release (Tests)', '--build-number', '13781', '--buildbot-worker', 'gtk-linux-slave-6', '--master-name', 'webkit.org', '--buildbot-master', 'build.webkit.org', '--report', 'https://results.webkit.org', '--exit-after-n-crashes-or-timeouts', '50', '--exit-after-n-failures', '500', '--release', '--layout-tests-directory', './Tools/BuildSlaveSupport/build.webkit.org-config/public_html/dashboard/Scripts/tests', '--gtk', '--results-directory', 'layout-test-results/dashboard-layout-test-results', '--debug-rwt-logging']
 using PTY: True
sh: 1: Syntax error: "(" unexpected
program finished with exit code 0
elapsedTime=0.892059
Comment 9 Philippe Normand 2020-05-21 07:23:45 PDT
Created attachment 399954 [details]
Patch
Comment 10 Carlos Alberto Lopez Perez 2020-05-21 07:26:13 PDT
Comment on attachment 399954 [details]
Patch

what is the change done compared with r261909?
Comment 11 Philippe Normand 2020-05-21 07:30:31 PDT
No change :) I'd like to reland as-is, making sure the exec bit is set on webkit-bwrap this time.
Comment 12 Carlos Alberto Lopez Perez 2020-05-21 07:36:33 PDT
(In reply to Philippe Normand from comment #11)
> No change :) I'd like to reland as-is, making sure the exec bit is set on
> webkit-bwrap this time.

I'm confused. Your previous patch already set it as executable.
Comment 13 Philippe Normand 2020-05-21 07:42:22 PDT
How do you know?
Comment 14 Philippe Normand 2020-05-21 07:52:24 PDT
I've rebased the patch mostly for Nikolas though. I'll need to test it again before landing in any case.
Comment 15 Carlos Alberto Lopez Perez 2020-05-21 08:10:26 PDT
(In reply to Philippe Normand from comment #13)
> How do you know?

I just checked out r261909 locally and checked that the file was executable.
Comment 16 Philippe Normand 2020-05-21 09:13:22 PDT
With svn or git/git-svn?
Comment 17 Carlos Alberto Lopez Perez 2020-05-21 09:20:33 PDT
(In reply to Philippe Normand from comment #16)
> With svn or git/git-svn?

With git. There should be no difference.

You can also check  https://trac.webkit.org/browser/webkit/trunk/Tools/flatpak/webkit-bwrap?rev=261909 <-- it says "Property svn:executable set to *"
Comment 18 Carlos Alberto Lopez Perez 2020-05-21 09:27:14 PDT
(In reply to Carlos Alberto Lopez Perez from comment #17)
> (In reply to Philippe Normand from comment #16)
> > With svn or git/git-svn?
> 
> With git. There should be no difference.
> 
> You can also check 
> https://trac.webkit.org/browser/webkit/trunk/Tools/flatpak/webkit-
> bwrap?rev=261909 <-- it says "Property svn:executable set to *"

Or with command:
  svn diff -c 261909  https://svn.webkit.org/repository/webkit/trunk/
Comment 19 Philippe Normand 2020-05-22 02:03:03 PDT
Well I can't reproduce this issue anywhere and since the error message doesn't give much details, I can only guess here. Without relanding this patch I won't be able to debug this.
Comment 20 Philippe Normand 2020-05-22 02:04:16 PDT
Created attachment 400034 [details]
Patch
Comment 21 Carlos Alberto Lopez Perez 2020-05-22 06:33:07 PDT Comment hidden (obsolete)
Comment 22 Carlos Alberto Lopez Perez 2020-05-22 06:33:54 PDT
(In reply to Philippe Normand from comment #19)
> Well I can't reproduce this issue anywhere and since the error message
> doesn't give much details, I can only guess here. Without relanding this
> patch I won't be able to debug this.

I can. You need to run the command like the bot does.

python ./Tools/Scripts/run-webkit-tests --no-build --no-show-results --no-new-test-results --clobber-old-results --builder-name "GTK Linux 64-bit Release (Tests)" --build-number 13781 --buildbot-worker gtk-linux-slave-6 --master-name webkit.org --buildbot-master build.webkit.org --report https://results.webkit.org --exit-after-n-crashes-or-timeouts 50 --exit-after-n-failures 500 --release --gtk --results-directory layout-test-results --debug-rwt-logging

sh: 1: Syntax error: "(" unexpected



And the issue its still happening with this last version of the patch

It looks it is caused by the parameters: --builder-name "GTK Linux 64-bit Release (Tests)"

The parenthesis in "(Tests)" causes This. If the parameters are passed to a shell maybe they need to be quoted/escaped properly
Comment 23 Philippe Normand 2020-05-22 06:44:46 PDT
🤯
Comment 24 Philippe Normand 2020-05-22 07:14:02 PDT
Created attachment 400043 [details]
Patch
Comment 25 EWS 2020-05-22 07:58:46 PDT
Committed r262057: <https://trac.webkit.org/changeset/262057>

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