Repository navigation
Get CI building again + Drop VLD - #110
Conversation
Signed-off-by: Simeon Widdis <sawiddis@amazon.com>
Signed-off-by: Simeon Widdis <sawiddis@amazon.com>
Signed-off-by: Simeon Widdis <sawiddis@amazon.com>
macos-latest runners are arm64, where Homebrew lives under /opt/homebrew rather than /usr/local. Locate iodbc and iodbcinst via brew --prefix, with both standard prefixes as fallbacks, instead of hardcoding the Intel path. Signed-off-by: Simeon Widdis <sawiddis@amazon.com>
macos-13 and windows-2019 are no longer available, so every run sat queued until GitHub cancelled it. Use the -latest images, drop the pinned CMake 3.26 (too old for newer Visual Studio), add a weekly scheduled run so image drift gets caught, and add an ubuntu job that builds the driver and unit tests via build_linux_release64.sh. Signed-off-by: Simeon Widdis <sawiddis@amazon.com>
With a null connection this was undefined behavior (throws or crashes). Newer Apple clang flags it via -Wnonnull, breaking the macos-latest build under -Werror. Return an empty string, matching GetServerVersion. Signed-off-by: Simeon Widdis <sawiddis@amazon.com>
vcpkg's default x64-linux triplet is static, so libodbcinst.a and its bundled libltdl (both LGPL-2.1) were being compiled into libsqlodbc.so and re-exported. Add an overlay triplet that builds only unixodbc as shared, link only libodbcinst (the driver never calls the driver manager), and fail configure if a static odbcinst is ever picked up. Include the triplet files in the Linux CI cache key so stale static libs aren't restored. Signed-off-by: Simeon Widdis <sawiddis@amazon.com>
0946a74 to
d11b8af
Compare
mengweieric
left a comment
mengweieric
left a comment
There was a problem hiding this comment.
Checked the Linux artifact from the latest run. The LGPL fix works: libsqlodbc.so only needs libodbcinst.so.2 and imports just the three installer symbols. Two notes inline.
Separately, the windows64-build and windows32-build artifacts are never uploaded ("No files were found"), but that is not from this PR. scripts/prepare_ci_output.ps1 copies from build/odbc/bin, while MSBuild writes to build/odbc/bin/Release. That script has not changed since 2022, so it can be a follow-up.
| if(UNIXODBCINST_LIB MATCHES "\\.a$") | ||
| message(FATAL_ERROR "Refusing to statically link LGPL unixODBC (${UNIXODBCINST_LIB}); use a shared libodbcinst") | ||
| endif() | ||
| target_link_libraries(sqlodbc aws-cpp-sdk-core ${UNIXODBCINST_LIB}) |
There was a problem hiding this comment.
The built libsqlodbc.so ends up with RUNPATH /home/runner/work/sql-odbc/sql-odbc/src/vcpkg_installed/x64-linux/lib: (from readelf -d on the CI artifact). The CI path is useless on user machines, and the trailing colon adds an empty entry. glibc treats an empty entry as the current directory, so a libodbcinst.so.2 in the app's working directory would be loaded first. I confirmed that with a small test lib. Could we drop the build RPATH for the driver, for example set_target_properties(sqlodbc PROPERTIES SKIP_BUILD_RPATH TRUE), and rely on the system unixODBC at runtime?
| cmake -S src -B cmake-build64 \ | ||
| -DCMAKE_BUILD_TYPE=Release \ | ||
| -DCMAKE_PREFIX_PATH="${PREFIX_PATH}" \ | ||
| -DWARNINGS_AS_ERRORS=OFF \ |
There was a problem hiding this comment.
Fine for getting Linux building. Worth a follow-up issue though, since the Linux job currently prints about 700 warnings (mostly -Wpedantic, plus -Wreorder, -Wsign-compare, and two -Wnonnull in convert.c around line 1303) that would fail the -Werror build on the other platforms.
There was a problem hiding this comment.
yeah, i have it on my todo list to fix a lot of these. as you point out, there's 700 of them.
The CI artifact carried RUNPATH /home/runner/.../vcpkg_installed/x64-linux/lib: which is useless on user machines, and its trailing empty entry makes glibc search the application's working directory, so a stray libodbcinst.so.2 there would be loaded first. Skip the build RPATH for sqlodbc and resolve libodbcinst from the system unixODBC at runtime. Test executables keep theirs. Signed-off-by: Simeon Widdis <sawiddis@amazon.com>
With no RPATH on the driver, libodbcinst.so.2 resolves from the system library path, as on a user machine. Ubuntu links with --as-needed, so test binaries that only reach unixODBC through libsqlodbc.so (ut_critical_section) need it installed system-wide. Signed-off-by: Simeon Widdis <sawiddis@amazon.com>
mengweieric
left a comment
mengweieric
left a comment
There was a problem hiding this comment.
Verified on the latest Linux CI artifact (run 41700140478). libsqlodbc.so carries no RPATH or RUNPATH, its only unixODBC dependency is the shared libodbcinst.so.2, it imports just the three odbcinst installer symbols, and no unixODBC or ltdl code is compiled in. All platform builds are green and the Linux unit tests run and pass. The remaining items (compiler warnings and the pre-existing Windows build-artifact upload path) are fine as follow-ups. LGTM, thanks for the quick turnaround.
Description
Follows: #109
Feel free to merge this instead & close the other one
Issues Resolved
[List any issues this PR will resolve]
Check List
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.