Repository navigation
Download SVS's prebuilt C API release instead of building from source - #102
Conversation
SVS now ships its C API fully prebuilt as a GitHub release asset (svs-c-api.tar.gz) starting at tag v0.5.0, extracting directly to the lib/ + include/svs/c/ layout SVS_INSTALL already expects. build_svs.sh no longer needs to clone the source, pin a commit, or run a cmake build against a separately-fetched core library tarball. Also switches set -e to set -euo pipefail: without pipefail, a failed curl piped into tar would not fail the script, since the pipeline's exit status came from tar, which can exit 0 on an empty or truncated stream. CI workflows and docs still reference the old SVS_COMMIT/source-build variables and are intentionally left for a follow-up pass. Signed-off-by: Olasoji <olasoji.denloye@intel.com>
docs/build_guide/config is now the single, definitive source for which SVS release to fetch (SVS_TAG, defaulting to v0.5.0, with SVS_URL derived from it). The four workflows and ci/build_and_test.sh still hard-coded their own SVS_URL/SVS_COMMIT pair pointing at the old nightly tarball, left over from when CI needed to pin a commit to keep a source build from drifting ahead of a fetched core library. That mechanism no longer exists, so these overrides were pure duplication of a now-stale value. Drop the env vars and their explanatory comments from all four workflows, drop the SVS_COMMIT segment from each cache key (SVS_URL drops too, since it no longer exists as an override -- the existing hashFiles(docs/build_guide/config, ...) component already invalidates the cache whenever the pin changes). assert-build.yml's job summary sourced SVS_URL directly for its "SVS library" line; source docs/build_guide/config instead and report SVS_TAG. ci/build_and_test.sh drops its own dead DEFAULT_SVS_URL and the override it threaded into build_svs.sh, which now gets everything it needs from docs/build_guide/config on its own. Signed-off-by: Olasoji <olasoji.denloye@intel.com>
matt-welch
left a comment
matt-welch
left a comment
There was a problem hiding this comment.
Nice split, doing the build script in one commit and the CI cleanup in the next makes this easy to review.
For the build script commit (28bd94b), I built and tested with the SVS tree produced by the PR's own build_svs.sh, and it works. The download takes about 4 seconds and gives the expected lib/ and include/svs/c/ layout. The extension builds with no warnings and svs.so links against libsvs_c_api.so.0.5.0 from that install. TAP (71 files, 1517 tests), the hardware TAP files (2 files, 55 tests), the SQL regression suite (10/10) and the hardware regression tests (2/2) all pass. The only skip is 18_build_offset_overflow.pl, which needs enable_slow_tests=yes.
For the CI commit (2659c59), removing the nightly SVS_URL and SVS_COMMIT exports is needed, because docs/build_guide/config:21 lets an exported SVS_URL win. Left in place, the old nightly tarball has no libsvs_c_api.so and no include/svs/c/, and build_svs.sh would exit 0 having installed nothing usable. I reproduced that by running the script with the nightly URL. After the commit, nothing else references SVS_COMMIT, DEFAULT_SVS_URL or SVS_BRANCH, and actionlint and shellcheck are clean on the changed files. I could not run CI here, so this commit is checked by reading and linting only.
A few small things, none blocking:
ci/build_and_test.sh:20still saysbuild-svsbuilds the bindings from a pinned nightly tarball, which is no longer true.docs/build_guide/README.md:59still says "Builds the SVS C API library".build_svs.sh:12could check thatlib/libsvs_c_api.soexists after extracting, so a wrongSVS_URLoverride fails right there instead of at the extension build.- The PR description still has the unfilled template sections below the first paragraph.
Looks good to me with those cleaned up.
Description
SVS now ships its C API fully prebuilt as a GitHub release asset (svs-c-api.tar.gz) starting at tag v0.5.0, extracting directly to the lib/ + include/svs/c/ layout SVS_INSTALL already expects. build_svs.sh no longer needs to clone the source, pin a commit, or run a cmake build against a separately-fetched core library tarball.
Also switches set -e to set -euo pipefail: without pipefail, a failed curl piped into tar would not fail the script, since the pipeline's exit status came from tar, which can exit 0 on an empty or truncated stream.
This PR also updates CI to use the release version of SVS
Related Issues
Type of Change
Pre-Merge Checklist
Build
makecompletes without errors or warningsmake installcompletes successfullyTests
make installcheck) and TAP tests (test/t/) pass with no failurestest/sql/and/ortest/t/test/modules/: it builds and passes (make -C test/modules/<module> installcheck)Documentation
docs/updated if architecture or usage changedTesting Notes