fix: link vcpkg static libroaring by exact path - #235
Open
GavinPizza wants to merge 1 commit into
Open
Conversation
The pgxs -L/usr/lib prefix otherwise wins the linker search order and binds -lroaring to the ancient system libroaring.so (0.2.66), which lacks symbols such as roaring_free. When ROARING_LIB_DIR is set, link the static archive by full path; otherwise fall back to -lroaring.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
The build installs and pins CRoaring 4.7.2 through vcpkg manifest mode, then does not reliably link it.
pgxsputs-L/usr/libahead of the vcpkg-L, so-lroaringresolves against whatever system CRoaring the host happens to have. Which library ends up inside the extension is a property of the machine, which is what pinning is meant to remove.Where a system copy exists and is old, the build looks clean and the extension will not load.
Debian bookworm ships 0.2.66, which does not export
roaring_free:The link still exits 0 and produces the
.so, because a shared object may carry undefined symbols. Nothing fails untildlopen, so any check that builds without loading passes it straight through.CI is unaffected today because its images carry no system CRoaring. That is luck rather than construction, since nothing asserts it. If a base image or a new apt dependency ever pulls one in, the build silently changes which CRoaring it links, with no signal.
Fix
Link
$(ROARING_LIB_DIR)/libroaring.aby full path when it is set, so search order cannot override it. Fall back to-lroaringwhen unset, unchanged, for builds that intend a system copy.Testing
Both arms on this branch, on Debian with libroaring 0.2.66 installed.
CREATE EXTENSION,LOAD, and a preloaded postmaster all succeed, a ducklake table round-trips, regression suite 52/52. The.sohas no undefined roaring symbols and noDT_NEEDEDon libroaring..sorecordsDT_NEEDED libroaring.so.0, and load fails with the error above (asFATALwhen preloaded).Verified on x64-linux only. The archive path is built from
$(VCPKG_TARGET_TRIPLET), so arm64 should resolve the same way, and the Docker workflow exercises it.