Bug 234612 – [Flatpak] Fix a11y tests on some distros including Fedora

• Component: WebKitGTK
• Status: REOPENED

[Patrick Griffis]

[Flatpak] Fix a11y tests on some distros including Fedora

[Patrick Griffis]

Created attachment 447820 [details]
Patch

[Patrick Griffis]

Created attachment 447821 [details]
Patch

[Patrick Griffis]

Created attachment 447824 [details]
Patch

[Michael Catanzaro]

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]

BTW this might be fixed by https://gitlab.gnome.org/GNOME/at-spi2-core/-/commit/412bdee0a1daa3fe9bc7d1fa289a31495dff3387.

[Patrick Griffis]

(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]

(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]

Our build server doesn't have pygobject. I could implement `GLib.get_user_runtime_dir()` directly if that is preferred.

[Patrick Griffis]

(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]

Created attachment 447840 [details]
Patch

[Michael Catanzaro]

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]

Oh and I bet the behavior difference is dbus-daemon vs. dbus-broker.

[Patrick Griffis]

Created attachment 447854 [details]
Patch

[Patrick Griffis]

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]

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]

Reopening to attach new patch.

[Patrick Griffis]

Created attachment 447897 [details]
Patch

[EWS]

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]

Reopening due to the partial revert.