WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
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
Details
Formatted Diff
Diff
Patch
(2.23 KB, patch)
2021-12-22 11:30 PST
,
Patrick Griffis
no flags
Details
Formatted Diff
Diff
Patch
(2.08 KB, patch)
2021-12-22 11:49 PST
,
Patrick Griffis
no flags
Details
Formatted Diff
Diff
Patch
(2.29 KB, patch)
2021-12-22 16:29 PST
,
Patrick Griffis
no flags
Details
Formatted Diff
Diff
Patch
(2.24 KB, patch)
2021-12-22 19:43 PST
,
Patrick Griffis
no flags
Details
Formatted Diff
Diff
Patch
(1.49 KB, patch)
2021-12-23 12:04 PST
,
Patrick Griffis
no flags
Details
Formatted Diff
Diff
Show Obsolete
(5)
View All
Add attachment
proposed patch, testcase, etc.
Patrick Griffis
Comment 1
2021-12-22 11:19:46 PST
Comment hidden (obsolete)
Created
attachment 447820
[details]
Patch
Patrick Griffis
Comment 2
2021-12-22 11:30:21 PST
Comment hidden (obsolete)
Created
attachment 447821
[details]
Patch
Patrick Griffis
Comment 3
2021-12-22 11:49:12 PST
Comment hidden (obsolete)
Created
attachment 447824
[details]
Patch
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
BTW this might be fixed by
https://gitlab.gnome.org/GNOME/at-spi2-core/-/commit/412bdee0a1daa3fe9bc7d1fa289a31495dff3387
.
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)
Our build server doesn't have pygobject. I could implement `GLib.get_user_runtime_dir()` directly if that is preferred.
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
Created
attachment 447840
[details]
Patch
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
Created
attachment 447854
[details]
Patch
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
Created
attachment 447897
[details]
Patch
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.
Top of Page
Format For Printing
XML
Clone This Bug