REOPENED 234612
[Flatpak] Fix a11y tests on some distros including Fedora
https://bugs.webkit.org/show_bug.cgi?id=234612
Summary [Flatpak] Fix a11y tests on some distros including Fedora
Patrick Griffis
Reported 2021-12-22 11:19:03 PST
[Flatpak] Fix a11y tests on some distros including Fedora
Attachments
Patch (3.50 KB, patch)
2021-12-22 11:19 PST, Patrick Griffis
no flags
Patch (2.23 KB, patch)
2021-12-22 11:30 PST, Patrick Griffis
no flags
Patch (2.08 KB, patch)
2021-12-22 11:49 PST, Patrick Griffis
no flags
Patch (2.29 KB, patch)
2021-12-22 16:29 PST, Patrick Griffis
no flags
Patch (2.24 KB, patch)
2021-12-22 19:43 PST, Patrick Griffis
no flags
Patch (1.49 KB, patch)
2021-12-23 12:04 PST, Patrick Griffis
no flags
Patrick Griffis
Comment 1 2021-12-22 11:19:46 PST Comment hidden (obsolete)
Patrick Griffis
Comment 2 2021-12-22 11:30:21 PST Comment hidden (obsolete)
Patrick Griffis
Comment 3 2021-12-22 11:49:12 PST Comment hidden (obsolete)
Michael Catanzaro
Comment 4 2021-12-22 12:27:06 PST
Comment on attachment 447824 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=447824&action=review > Tools/ChangeLog:10 > + On Debian the tests previously worked because the a11y bus is set > + to a filesystem path (unix:abstract=/tmp/dbus-YvSOaUDNJe) however > + on Fedora it is not a path (unix:abstract=00008). Using --no-a11y-bus fixes this. Something's wrong with this changelog explanation. unix:abstract=/tmp/dbus-YvSOaUDNJe is not a filesystem path. It's a D-Bus address that corresponds to an abstract socket with a name that *looks* like a filesystem path, but of course abstract sockets definitionally do not exist on the filesystem. Right? So what's *really* the difference between using that vs. unix:abstract=00008? (To access an abstract socket on the host, we would need to use --allow=net to get the host network namespace.) > Tools/flatpak/flatpakutils.py:767 > + # at-spi-registryd creates directories like `$XDG_RUNTIME_DIR/at-spi2-E6A5E1` on the host > + "--filesystem=" + GLib.get_user_runtime_dir(), But there is --filesystem=host just above, so surely this is not needed?
Michael Catanzaro
Comment 5 2021-12-22 12:29:32 PST
Patrick Griffis
Comment 6 2021-12-22 16:14:44 PST
(In reply to Michael Catanzaro from comment #4) > Comment on attachment 447824 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=447824&action=review > > > Tools/ChangeLog:10 > > + On Debian the tests previously worked because the a11y bus is set > > + to a filesystem path (unix:abstract=/tmp/dbus-YvSOaUDNJe) however > > + on Fedora it is not a path (unix:abstract=00008). Using --no-a11y-bus fixes this. > > Something's wrong with this changelog explanation. > unix:abstract=/tmp/dbus-YvSOaUDNJe is not a filesystem path. It's a D-Bus > address that corresponds to an abstract socket with a name that *looks* like > a filesystem path, but of course abstract sockets definitionally do not > exist on the filesystem. Right? So what's *really* the difference between > using that vs. unix:abstract=00008? It is a path on the filesystem. > (To access an abstract socket on the host, we would need to use --allow=net > to get the host network namespace.) > > > Tools/flatpak/flatpakutils.py:767 > > + # at-spi-registryd creates directories like `$XDG_RUNTIME_DIR/at-spi2-E6A5E1` on the host > > + "--filesystem=" + GLib.get_user_runtime_dir(), > > But there is --filesystem=host just above, so surely this is not needed? It fails without. The `host` option does not mount that directory.
Patrick Griffis
Comment 7 2021-12-22 16:17:03 PST
(In reply to Michael Catanzaro from comment #5) > BTW this might be fixed by > https://gitlab.gnome.org/GNOME/at-spi2-core/-/commit/ > 412bdee0a1daa3fe9bc7d1fa289a31495dff3387. I do expect that to fix the filesystem permission. Using `--no-a11y-bus` is still correct though because we don't want it to be proxied by flatpak which filters out calls we use.
Patrick Griffis
Comment 8 2021-12-22 16:18:04 PST Comment hidden (obsolete)
Patrick Griffis
Comment 9 2021-12-22 16:19:32 PST
(In reply to Michael Catanzaro from comment #5) > BTW this might be fixed by > https://gitlab.gnome.org/GNOME/at-spi2-core/-/commit/ > 412bdee0a1daa3fe9bc7d1fa289a31495dff3387. Actually this still uses `g_get_user_runtime_dir()` so those files are still unreadable.
Patrick Griffis
Comment 10 2021-12-22 16:29:19 PST
Michael Catanzaro
Comment 11 2021-12-22 17:14:10 PST
Comment on attachment 447840 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=447840&action=review OK, I don't see anything wrong with the change itself, but I'm almost positive the changelog is not correct. > Tools/ChangeLog:9 > + On Debian the tests previously worked because the a11y bus is set > + to a filesystem path (unix:abstract=/tmp/dbus-YvSOaUDNJe) however If this is truly the server address, then there should not be a socket created at /tmp/dbus-YvSOaUDNJe. Instead, there would be a socket created in the abstract socket namespace with name '\0' (NUL character) followed by YvSOaUDNJe. Correct? So I'm almost certain this changelog entry is wrong. I would believe that Debian is using and address like unix:path=/tmp/dbus-YvSOaUDNJe or unix:dir=/tmp. I don't doubt that the socket really gets created on the filesystem. But if so, the server address it's using surely does not start with unix:abstract. > Tools/flatpak/flatpakutils.py:764 > + # --session-bus is only a workaround for https://github.com/flatpak/flatpak/pull/4630 Please say "FIXME" so we know to drop this in the future. > Tools/flatpak/flatpakutils.py:769 > + # at-spi creates directories like `$XDG_RUNTIME_DIR/at-spi2-E6A5E1` on the host > + "--filesystem=" + self.get_user_runtime_dir(), OK, looking at the flatpak-builder documentation, I see that /tmp is on a denylist so that it doesn't get mounted by --filesystem=host.
Michael Catanzaro
Comment 12 2021-12-22 17:20:16 PST
Oh and I bet the behavior difference is dbus-daemon vs. dbus-broker.
Patrick Griffis
Comment 13 2021-12-22 19:43:05 PST
Patrick Griffis
Comment 14 2021-12-22 19:43:47 PST
I've updated the changelog. The exact details may indeed be wrong. This does allow the tests to run on Fedora at the very least.
EWS
Comment 15 2021-12-23 08:54:55 PST
Committed r287396 (245536@main): <https://commits.webkit.org/245536@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 447854 [details].
Patrick Griffis
Comment 16 2021-12-23 12:04:25 PST
Reopening to attach new patch.
Patrick Griffis
Comment 17 2021-12-23 12:04:27 PST
EWS
Comment 18 2021-12-23 12:38:00 PST
Committed r287407 (245542@main): <https://commits.webkit.org/245542@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 447897 [details].
Michael Catanzaro
Comment 19 2021-12-23 16:20:39 PST
Reopening due to the partial revert.
Note You need to log in before you can comment on or make changes to this bug.