build: fix: Ubuntu 26.04 LTS#133
Conversation
📝 WalkthroughWalkthroughThe Ubuntu workflow adds three Ubuntu 26.04 Clang build variants. GNOME 3 configuration conditionally resolves and uses GLib’s size-aware freeing function for allocated strings. ChangesUbuntu 26.04 build matrix
GNOME 3 size-aware freeing
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #133 +/- ##
==========================================
+ Coverage 58.57% 63.98% +5.40%
==========================================
Files 32 34 +2
Lines 2607 2918 +311
Branches 526 546 +20
==========================================
+ Hits 1527 1867 +340
+ Misses 745 706 -39
- Partials 335 345 +10
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
sergio-nsk
force-pushed
the
sergio-nsk/u26/1
branch
from
18df4f9 to
aa98740
Compare
sergio-nsk
force-pushed
the
sergio-nsk/u26/1
branch
from
aa98740 to
6db4285
Compare
|
Rebased for re-running the checks: |
sergio-nsk
requested review from
Copilot and
nmoinvaz
and removed request for
Copilot
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@config_gnome3.c`:
- Around line 37-48: The fallback branch must define g_free, not gfree, and the
wrapper must be installed regardless of whether GLib already defines g_free as a
macro. Update the preprocessor structure around the g_free wrapper so any
existing definition is removed when present, then define g_free to call
g_proxy_config_gnome3.g_free in builds without sized-free support.
- Around line 183-188: Update the g_free_sized resolution in the GNOME3
initialization flow to be optional: when dlsym cannot resolve it, assign the
already-resolved g_free function instead of jumping to gnome3_init_error.
Preserve the existing g_free_sized lookup and conditional compilation while
allowing backend initialization to continue on older GLib runtimes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6cdbebd9-2164-4f6f-a149-e14e5a0c7d9a
📒 Files selected for processing (2)
.github/workflows/ubuntu.yamlconfig_gnome3.c
| #ifdef g_free | ||
| # undef g_free | ||
| # if G_GNUC_CHECK_VERSION(4, 1) && GLIB_VERSION_MAX_ALLOWED >= GLIB_VERSION_2_78 && defined(G_HAVE_FREE_SIZED) | ||
| # define g_free(mem) \ | ||
| (__builtin_object_size((mem), 0) != ((size_t)-1)) \ | ||
| ? (g_proxy_config_gnome3.g_free_sized)(mem, __builtin_object_size((mem), 0)) \ | ||
| : (g_proxy_config_gnome3.g_free)(mem) | ||
| # else | ||
| # define gfree(mem) (g_proxy_config_gnome3.g_free)(mem) | ||
| # endif | ||
| #endif | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== config_gnome3.c relevant lines ==\n'
nl -ba config_gnome3.c | sed -n '1,220p'
printf '\n== search for gfree / g_free references ==\n'
rg -n '\bgfree\b|\bg_free\b|G_HAVE_FREE_SIZED|GLIB_VERSION_MAX_ALLOWED' config_gnome3.cRepository: snxd/proxyres
Length of output: 229
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== surrounding context for call sites ==\n'
nl -ba config_gnome3.c | sed -n '48,170p'Repository: snxd/proxyres
Length of output: 233
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# If the file exists elsewhere, locate it and inspect exact sections.
file="$(git ls-files '*config_gnome3.c' | head -n 1)"
printf 'file=%s\n' "$file"
nl -ba "$file" | sed -n '1,220p'Repository: snxd/proxyres
Length of output: 212
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== config_gnome3.c relevant lines ==\n'
sed -n '1,220p' config_gnome3.c
printf '\n== search for gfree / g_free references ==\n'
grep -nE '\bgfree\b|\bg_free\b|G_HAVE_FREE_SIZED|GLIB_VERSION_MAX_ALLOWED' config_gnome3.cRepository: snxd/proxyres
Length of output: 9392
Fix the g_free fallback macro The #else branch currently defines gfree, so g_free(...) falls back to the GLib symbol instead of g_proxy_config_gnome3.g_free on builds without sized-free support. The outer #ifdef g_free also skips the wrapper entirely unless GLib exposes g_free as a macro.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@config_gnome3.c` around lines 37 - 48, The fallback branch must define
g_free, not gfree, and the wrapper must be installed regardless of whether GLib
already defines g_free as a macro. Update the preprocessor structure around the
g_free wrapper so any existing definition is removed when present, then define
g_free to call g_proxy_config_gnome3.g_free in builds without sized-free
support.
6db4285 to
cc1eccd
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (1)
config_gnome3.c (1)
37-47: 🎯 Functional Correctness | 🟠 MajorStill remove the outer
#ifdef g_freeguard.GLib declares
g_freeas a function and only conditionally defines the macro. When that macro is absent, this block is skipped entirely, so the new call sites bypass thedlsym-resolved function and may reintroduce a direct link dependency. (docs.gtk.org)Move the
#ifdefaround only#undef g_free, then define the fallback wrapper unconditionally.Proposed fix
-#ifdef g_free -# undef g_free -# if ... +#ifdef g_free +# undef g_free +#endif +#if ... ... -# endif `#endif`🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@config_gnome3.c` around lines 37 - 47, Update the g_free compatibility block so the outer `#ifdef` g_free guards only `#undef` g_free, then define the replacement g_free wrapper unconditionally using the existing version check and dlsym-resolved proxy functions. Ensure call sites use g_proxy_config_gnome3.g_free or g_free_sized even when GLib did not originally define a g_free macro.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@config_gnome3.c`:
- Around line 37-47: Update the g_free compatibility block so the outer `#ifdef`
g_free guards only `#undef` g_free, then define the replacement g_free wrapper
unconditionally using the existing version check and dlsym-resolved proxy
functions. Ensure call sites use g_proxy_config_gnome3.g_free or g_free_sized
even when GLib did not originally define a g_free macro.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 04261c87-c26f-4125-8d73-1edf9953666c
📒 Files selected for processing (2)
.github/workflows/ubuntu.yamlconfig_gnome3.c
There was a problem hiding this comment.
Pull request overview
This PR updates the GNOME3 configuration backend to cope with newer GLib memory-free APIs/macros, and expands CI to validate builds on Ubuntu 26.04 across multiple feature configurations.
Changes:
- Add a
g_freewrapper strategy inconfig_gnome3.cto handle environments where GLib providesg_freeas a macro and may useg_free_sized. - Extend the Ubuntu GitHub Actions workflow matrix with Ubuntu 26.04 variants (default, curl, duktape) and corresponding llvm-cov settings.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| config_gnome3.c | Adds support for GLib g_free_sized / g_free macro interactions by routing frees through dynamically resolved symbols. |
| .github/workflows/ubuntu.yaml | Adds Ubuntu 26.04 build+test+coverage jobs for multiple configurations. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| #if G_GNUC_CHECK_VERSION(4, 1) && GLIB_VERSION_MAX_ALLOWED >= GLIB_VERSION_2_78 && defined(G_HAVE_FREE_SIZED) | ||
| g_proxy_config_gnome3.g_free_sized = | ||
| (void (*)(gpointer, size_t))dlsym(g_proxy_config_gnome3.glib_module, "g_free_sized"); | ||
| if (!g_proxy_config_gnome3.g_free_sized) | ||
| goto gnome3_init_error; | ||
| #endif |
There was a problem hiding this comment.
IMHO this is not necessary. proxyres is OSS. If it's built with GLIB_VERSION_MAX_ALLOWED >= GLIB_VERSION_2_78, proxyres is not supposed to run with older glib. And, such choice is based on gnome solution.
| # define g_free(mem) \ | ||
| (__builtin_object_size((mem), 0) != ((size_t)-1)) \ | ||
| ? (g_proxy_config_gnome3.g_free_sized)(mem, __builtin_object_size((mem), 0)) \ | ||
| : (g_proxy_config_gnome3.g_free)(mem) |
Summary by CodeRabbit
Bug Fixes
Chores