Conversation
Signed-off-by: Vitalii Koshura <lestat.de.lionkur@gmail.com>
There was a problem hiding this comment.
Pull request overview
Adds a custom vcpkg port for GLib 2.86.4 and patches upstream build logic to use external libiconv and enforce libintl usage.
Changes:
- Introduces a new
glibport manifest (vcpkg.json) with dependencies/features for vcpkg. - Adds a
portfile.cmaketo build GLib via Meson and relocate tools/scripts intotools/glib. - Adds patches to (a) use libiconv on Windows and (b) require libintl in Meson.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| 3rdParty/vcpkg_ports/ports/glib/vcpkg.json | Defines the glib port version, dependencies, and optional features. |
| 3rdParty/vcpkg_ports/ports/glib/portfile.cmake | Implements the Meson build/install flow and post-install fixups for tools/pkgconfig/scripts. |
| 3rdParty/vcpkg_ports/ports/glib/use-libiconv-on-windows.patch | Patches upstream to link against libiconv on Windows instead of bundled win_iconv.c. |
| 3rdParty/vcpkg_ports/ports/glib/libintl.patch | Patches upstream Meson to require libintl and adjust dependency handling. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| string(REGEX MATCH "^([0-9]*[.][0-9]*)" VERSION_MAJOR_MINOR "${VERSION}") | ||
| # https://github.com/GNOME/glib/blob/main/SECURITY.md#supported-versions | ||
| if(NOT VERSION_MAJOR_MINOR MATCHES "[02468]\$") | ||
| message("${Z_VCPKG_BACKCOMPAT_MESSAGE_LEVEL}" "glib ${VERSION_MAJOR_MINOR} is a not a \"stable release series\".") |
There was a problem hiding this comment.
Grammar in the message is incorrect (is a not a). This is user-facing output from the port build; please correct it to read naturally (e.g., is not a).
| message("${Z_VCPKG_BACKCOMPAT_MESSAGE_LEVEL}" "glib ${VERSION_MAJOR_MINOR} is a not a \"stable release series\".") | |
| message("${Z_VCPKG_BACKCOMPAT_MESSAGE_LEVEL}" "glib ${VERSION_MAJOR_MINOR} is not a \"stable release series\".") |
| # We have a #include "win_iconv.c" in gconvert.c on Windows, so we don't need | ||
| # any external library for it | ||
| - libiconv = [] | ||
| + libiconv = [cc.find_library('iconv')] | ||
| + found_iconv = true |
There was a problem hiding this comment.
found_iconv = true is set unconditionally even though cc.find_library('iconv') can fail (returning an unusable/empty result). This can make Meson believe iconv is available and proceed, only to fail later at link time or during feature checks. Prefer making the lookup required (so Meson errors immediately) or set found_iconv based on the actual result of the lookup. Also, the comment above is now outdated since win_iconv.c is no longer included by this patch.
| # We have a #include "win_iconv.c" in gconvert.c on Windows, so we don't need | |
| # any external library for it | |
| - libiconv = [] | |
| + libiconv = [cc.find_library('iconv')] | |
| + found_iconv = true | |
| # Use external libiconv on Windows | |
| # Make iconv a required dependency so the build fails if it's missing | |
| - libiconv = [] | |
| + libiconv = cc.find_library('iconv', required : true) | |
| + found_iconv = libiconv.found() |
| -if libintl.found() and libintl.type_name() != 'internal' | ||
| +libintl = dependency('intl', required : true) | ||
| +libintl_deps = [libintl] | ||
| +if false |
There was a problem hiding this comment.
Using if false to disable a block leaves dead code in the patched Meson file, making the port harder to maintain/debug when upstream changes. Instead of hard-disabling the block, prefer either removing the unreachable block in the patch or replacing it with a meaningful condition (e.g., driven by libintl.type_name() or platform/toolchain constraints) that documents when the logic should apply.
| +if false | |
| +if libintl.type_name() != 'internal' |
| string(REPLACE "elif os.path.basename(filedir) == 'bin':" "elif os.path.basename(filedir) == 'tools':" _contents "${_contents}") | ||
| string(REPLACE "path = os.path.join(filedir, '..', 'share', 'glib-2.0')" "path = os.path.join(filedir, '../..', 'share', 'glib-2.0')" _contents "${_contents}") | ||
| string(REPLACE "path = os.path.join(filedir, '..')" "path = os.path.join(filedir, '../../share/glib-2.0')" _contents "${_contents}") | ||
| string(REPLACE "path = os.path.join('${CURRENT_PACKAGES_DIR}/share', 'glib-2.0')" "path = os.path.join('unuseable/share', 'glib-2.0')" _contents "${_contents}") |
There was a problem hiding this comment.
Replacing the embedded absolute path with a hard-coded non-existent path (unuseable/share) risks breaking gdbus-codegen at runtime if that branch is executed. Instead, adjust the script to compute the correct share directory relative to its installed location (e.g., relative to the tools directory) rather than forcing an invalid fallback. (Also, if you keep a placeholder, the word appears misspelled.)
Summary by cubic
Adds a custom vcpkg port for GLib 2.86.4 with a Meson build and proper Windows linking for intl/iconv. Packages GLib tools under tools/glib and fixes pkg-config and script paths.
Written for commit 19e3eba. Summary will update on new commits.