Repository navigation
Patch atkversion.h in sysroots to add C linkage declarations - #195883
vhaudiquet wants to merge 1 commit into
Conversation
github-actions
Bot
added
engine
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request automates patching of atkversion.h in the Linux sysroot to add missing C linkage declarations, allowing the removal of local extern "C" workarounds in C++ files. Feedback on the changes highlights that the newly added Python function uses 2-space indentation instead of the 4-space indentation required by the Google Python Style Guide, and suggests specifying the file encoding when opening files.
75de51f to
9f8a780
Compare
|
FYI I also experimented with enabling Trixie builds but for x86-64 and tracked here Regarding the ATK headers I took more of a work around approach, while you fixed at sysroot level which I believe is better root cause solution I will track and close duplicate if your approach is accepted. |
robert-ancell
left a comment
robert-ancell
left a comment
There was a problem hiding this comment.
The solution is a more complicated/obscure than I'd like, but it does work around the problem. I'm currently looking at replacing the ATK code, so this can be removed in the future if we progress with that.
robert-ancell
added
CICD
auto-submit
Bot
removed
the
autosubmit
|
autosubmit label was removed for flutter/flutter/195883, because This PR has not met approval requirements for merging. The PR author is not a member of flutter-hackers and needs 1 more review(s) in order to merge this PR.
|
|
autosubmit label was removed for flutter/flutter/195883, because - The status or check suite ci.yaml validation has failed. Please fix the issues identified (or deflake) before re-applying this label. |
The atkversion.h header in at-spi2-core (Debian Bullseye 2.38 and Trixie 2.56.2) lacks G_BEGIN_DECLS/G_END_DECLS, causing link failures when included from C++ code. This was fixed upstream in https://gitlab.gnome.org/GNOME/at-spi2-core/-/merge_requests/219 but the fix has not yet propagated to all Debian stable releases. Previously, fl_view_accessible.cc and fl_accessibility_handler_test.cc worked around this by wrapping #include <atk/atk.h> in extern "C" { }. This breaks on Trixie sysroots where atk.h pulls in headers with C++ features that cannot be inside an extern "C" block. This commit: - Adds PatchAtkVersionHeader() to install-sysroot.py, which patches atkversion.h after sysroot extraction to add G_BEGIN_DECLS/ G_END_DECLS (the same fix as upstream MR 219). Idempotent. - Removes the extern "C" workaround from fl_view_accessible.cc and fl_accessibility_handler_test.cc, relying on the patched header for proper C linkage.
9f8a780 to
1bc8231
Compare
|
Rebased on top of main. As far as I understand, this is what was making CI fail.
Should be ok now. |
The atkversion.h header in at-spi2-core (Debian Bullseye 2.38 and Trixie 2.56.2) lacks G_BEGIN_DECLS/G_END_DECLS, causing link failures when included from C++ code. This was fixed upstream in https://gitlab.gnome.org/GNOME/at-spi2-core/-/merge_requests/219 but the fix has not yet propagated to all Debian stable releases.
Previously, fl_view_accessible.cc and fl_accessibility_handler_test.cc worked around this by wrapping #include <atk/atk.h> in extern "C" { }. This breaks on Trixie sysroots where atk.h pulls in headers with C++ features that cannot be inside an extern "C" block.
This commit:
Debian Trixie sysroot is needed for #178712, so this fix is needed in order to be able to build riscv64 at all, but also more generally to upgrade any build to use a Trixie sysroot instead of bookworm.
Pre-launch Checklist
///).