From 614671981b9cb6a5538715c28e571cd9dcae8a92 Mon Sep 17 00:00:00 2001 From: ethanglaser Date: Fri, 11 Sep 2026 10:54:31 -0700 Subject: [PATCH 01/15] Initial ABI checking support --- .abicheck.yml | 21 +++ .github/abi-baseline-tag | 13 ++ .github/abi-suppressions.yml | 68 ++++++++++ .github/scripts/abi-check.sh | 126 ++++++++++++++++++ .../workflows/build-cpp-runtime-bindings.yml | 99 ++++++++++++++ 5 files changed, 327 insertions(+) create mode 100644 .abicheck.yml create mode 100644 .github/abi-baseline-tag create mode 100644 .github/abi-suppressions.yml create mode 100755 .github/scripts/abi-check.sh diff --git a/.abicheck.yml b/.abicheck.yml new file mode 100644 index 000000000..2e42816a0 --- /dev/null +++ b/.abicheck.yml @@ -0,0 +1,21 @@ +# abicheck configuration, picked up from the repo root by `abicheck compare`. +# Validated against the abicheck 0.5.0 config schema. +# +# Kept identical to .abicheck.yml in the innersource repo: the two repos build +# the same runtime library and must not disagree on how it is parsed. + +# castxml's default cc-gnu detection parses SVS's C++20 headers at the wrong +# standard (std::span / constexpr dtor fail). The clang AST frontend handles +# C++20 cleanly; layout-only checks fall back to the .so's DWARF. +compile: + frontend: clang + std: c++20 + +# Scope the compared surface to what we publish, and collapse the std/gnu_cxx +# template-instantiation noise that leaks past --exclude-libs,ALL. +scope: + public: true + collapse_versioned_symbols: true + +suppression: + require_justification: true diff --git a/.github/abi-baseline-tag b/.github/abi-baseline-tag new file mode 100644 index 000000000..4fc0d008b --- /dev/null +++ b/.github/abi-baseline-tag @@ -0,0 +1,13 @@ +# The released tag that runtime-bindings ABI compatibility is measured against. +# Assets are pulled from this repo's GitHub releases at check time. +# +# Bumped deliberately as part of cutting a release, so that resetting the ABI +# baseline is a reviewable act rather than a side effect of tagging. Pinning also +# keeps a new release from silently moving the target under already-open PRs. +# +# `latest` is accepted and resolves dynamically, but is not the default: patch +# tags can land out of order (v0.3.1 and v0.3.2 were both tagged after v0.4.0), +# so "newest tag" is not always "current release line". +# +# Keep in sync with .github/abi-baseline-tag in the innersource repo. +v0.4.0 diff --git a/.github/abi-suppressions.yml b/.github/abi-suppressions.yml new file mode 100644 index 000000000..267ff2ddf --- /dev/null +++ b/.github/abi-suppressions.yml @@ -0,0 +1,68 @@ +# ABI check suppression rules for the SVS C++ runtime bindings, used by +# napetrov/abicheck. Owned here rather than in the innersource repo so that both +# repos measure libsvs_runtime.so against the same rules -- a suppression present +# on one side only would make the innersource-vs-public equivalence check report +# a difference that is really just an asymmetric filter. +# +# Scope is the runtime bindings alone. The innersource repo keeps its own file +# for libsvs_shared_library.so, which leaks a far larger third-party surface. +# +# Measured on libsvs_runtime.so v0.4.0: 253 exported symbols, of which 55 are the +# declared svs::runtime::v0 API. Nearly everything suppressed below is a +# libstdc++ template instantiation that reached the export table because the +# bindings are built without -fvisibility=hidden. + +version: 1 +suppressions: + # --- C++ standard library / runtime (never part of the SVS API) --- + - symbol_pattern: "^_ZNSt.*" + reason: "C++ standard library symbols (leaked template instantiations)" + label: "stdlib" + + - symbol_pattern: "^_ZSt.*" + reason: "C++ standard library functions" + label: "stdlib" + + - symbol_pattern: "^_ZN9__gnu_cxx.*" + reason: "libstdc++ __gnu_cxx internals (numeric_traits etc.), leaked" + label: "stdlib" + + # RTTI/vtable/VTT for std types mangle as _ZTI/_ZTS/_ZTV/_ZTT, not _ZNSt. + - symbol_pattern: "^_ZT[VIST]N?St.*" + reason: "RTTI/vtable/VTT for std:: types (e.g. thread::_State_impl, future)" + label: "stdlib" + + - symbol_pattern: "^_ZT[VIST].*__gnu_cxx.*" + reason: "RTTI/vtable for __gnu_cxx types, leaked" + label: "stdlib" + + - symbol_pattern: "^__cxa_.*" + reason: "C++ ABI runtime symbols" + label: "cxx-abi" + + - symbol_pattern: "^__gxx_.*" + reason: "GCC C++ runtime symbols" + label: "cxx-abi" + + # --- Header-only third-party deps that leak via template instantiation --- + - symbol_pattern: "^_ZN[0-9]+toml.*" + reason: "toml++ header-only symbols leaked via instantiation" + label: "third-party" + + - symbol_pattern: "^_ZN3fmt.*" + reason: "fmt header-only symbols leaked via instantiation" + label: "third-party" + + - symbol_pattern: "^_ZN3tsl.*" + reason: "tsl::robin_map header-only symbols leaked via instantiation" + label: "third-party" + + - symbol_pattern: "^_ZN6spdlog.*" + reason: "spdlog header-only symbols leaked via instantiation" + label: "third-party" + + # --- svs::runtime::v0 and svs::...detail...: deliberately NOT suppressed --- + # The 55 declared v0 functions are the whole point of the check. `detail` + # symbols are not suppressed either: users reach them transitively through + # template instantiation, so a vtable or layout change there is a real break. + # See the same note in the innersource suppression file. diff --git a/.github/scripts/abi-check.sh b/.github/scripts/abi-check.sh new file mode 100755 index 000000000..87f16ccd6 --- /dev/null +++ b/.github/scripts/abi-check.sh @@ -0,0 +1,126 @@ +#!/bin/bash +# Copyright 2025 Intel Corporation +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +# Compare the ABI of two SVS build tarballs with napetrov/abicheck. +# +# One primitive serves both questions asked of it: +# temporal old = a release tarball, new = this build -- did we break users? +# equivalence old and new are the same source revision built two ways -- did +# the build paths diverge? +# The distinction is in what the caller passes, not in the logic, so this lives +# in one place. It is called from this repo's build-cpp-runtime-bindings.yml and +# from build-share-lib.yml in the innersource repo via the submodule path. +# +# Usage: abi-check.sh +# +# Overridable via environment: +# LIBRARY library basename to compare (default libsvs_runtime.so) +# HEADER_SUBDIR header root inside the tarball (default include/svs/runtime) +# SUPPRESSIONS suppression file (default .github/abi-suppressions.yml) +# POLICY abicheck policy (default strict_abi) +# REPORT markdown report path (default abi-report.md) +# WORKDIR scratch directory (default abi-work) +# +# Exit status is abicheck's: 0 compatible, 2 source-level API break, 4 binary +# ABI break, 64 invalid invocation. 77 is added to mean "an input was missing", +# which is not a finding and must never be reported as one. + +set -euo pipefail + +OLD_LABEL="${1:?Usage: abi-check.sh }" +OLD_TARBALL="${2:?missing old tarball}" +NEW_LABEL="${3:?missing new label}" +NEW_TARBALL="${4:?missing new tarball}" + +LIBRARY="${LIBRARY:-libsvs_runtime.so}" +HEADER_SUBDIR="${HEADER_SUBDIR:-include/svs/runtime}" +SUPPRESSIONS="${SUPPRESSIONS:-.github/abi-suppressions.yml}" +POLICY="${POLICY:-strict_abi}" +REPORT="${REPORT:-abi-report.md}" +WORKDIR="${WORKDIR:-abi-work}" + +# Prints the resolved library path on success. Release tarballs and CI artifacts +# have differed on whether the unversioned symlink is included, so accept the +# versioned real file too. +unpack() { + local tarball="$1" dest="$2" label="$3" lib + + if [ ! -f "$tarball" ]; then + echo "::warning::$label: tarball not found at $tarball" >&2 + return 1 + fi + + rm -rf "$dest" + mkdir -p "$dest" + tar -xzf "$tarball" -C "$dest" + + lib=$(find "$dest" -name "$LIBRARY" -o -name "$LIBRARY.*" | sort | head -1) + if [ -z "$lib" ]; then + echo "::error::$label: $LIBRARY not found in $tarball" >&2 + return 1 + fi + if [ ! -d "$dest/$HEADER_SUBDIR" ]; then + echo "::error::$label: headers missing from $tarball ($HEADER_SUBDIR)" >&2 + return 1 + fi + echo "$lib" +} + +OLD_DIR="$WORKDIR/old" +NEW_DIR="$WORKDIR/new" + +OLD_LIB=$(unpack "$OLD_TARBALL" "$OLD_DIR" "$OLD_LABEL") || exit 77 +NEW_LIB=$(unpack "$NEW_TARBALL" "$NEW_DIR" "$NEW_LABEL") || exit 77 + +echo "Comparing $LIBRARY: [$OLD_LABEL] -> [$NEW_LABEL]" + +suppress_args=() +[ -f "$SUPPRESSIONS" ] && suppress_args=(--suppress "$SUPPRESSIONS") + +rc=0 +# Without pipefail the pipe into tee masks abicheck's exit status. +set -o pipefail +# castxml is not installed on the runners; clang is. `auto` does not fall back to +# clang on its own. If header parsing degrades, abicheck still reports from the +# ELF tier -- which is where vtable-size findings come from anyway. +abicheck compare \ + "$OLD_LIB" \ + "$NEW_LIB" \ + --header old="$OLD_DIR/$HEADER_SUBDIR" \ + --header new="$NEW_DIR/$HEADER_SUBDIR" \ + --include old="$OLD_DIR/include" \ + --include new="$NEW_DIR/include" \ + --version old="$OLD_LABEL" \ + --version new="$NEW_LABEL" \ + --ast-frontend clang \ + --gcc-options "-std=c++20 -include cstddef" \ + --policy "$POLICY" \ + "${suppress_args[@]}" \ + --format markdown | tee "$REPORT" || rc=$? + +case "$rc" in + 0) + echo "ABI compatible: $OLD_LABEL -> $NEW_LABEL" + ;; + 64) + echo "::error::abicheck rejected the invocation (exit 64) comparing" \ + "$OLD_LABEL -> $NEW_LABEL. This is a harness bug, not an ABI finding." + ;; + *) + echo "::warning::ABI incompatibility: $NEW_LABEL differs from $OLD_LABEL (rc=$rc)" + ;; +esac + +exit "$rc" diff --git a/.github/workflows/build-cpp-runtime-bindings.yml b/.github/workflows/build-cpp-runtime-bindings.yml index e438752d0..810339ae2 100644 --- a/.github/workflows/build-cpp-runtime-bindings.yml +++ b/.github/workflows/build-cpp-runtime-bindings.yml @@ -126,3 +126,102 @@ jobs: -e SUFFIX=${{ matrix.suffix }} \ svs-manylinux228:latest \ /bin/bash .github/scripts/test-faiss.sh + + # Check the built runtime bindings against the last published release. + # + # Only the default variant is checked: releases publish + # svs-cpp-runtime-bindings.tar.gz but no -public-only asset, so that variant has + # nothing to compare against. It is covered indirectly -- both variants build + # from the same headers. + # + # The comparison primitive is .github/scripts/abi-check.sh, which the + # innersource repo also calls through its submodule path so the two repos cannot + # drift on how the same library is measured. + abi-check: + name: ABI check against the last release + needs: build-cpp-runtime-bindings + runs-on: ubuntu-22.04 + # An intentionally-breaking PR still gets a full report: the label downgrades + # this job from blocking to informational rather than skipping it, so the + # break is recorded on the PR instead of going unmeasured. + continue-on-error: ${{ contains(toJson(github.event.pull_request.labels.*.name), '"API/ABI breaking change"') }} + + steps: + - uses: actions/checkout@v6 + + # The hosted ubuntu-22.04 image ships only clang-13/14, which degrades + # abicheck's clang AST frontend to ELF-tier and drops constrained-template + # diffs. innersource gets a c++20-capable clang from its self-hosted + # toolchain; this keeps the two repos measuring at the same fidelity. + - name: Install clang for the AST frontend + run: | + wget -qO /tmp/llvm.sh https://apt.llvm.org/llvm.sh + chmod +x /tmp/llvm.sh + sudo /tmp/llvm.sh 18 + sudo apt-get install -y --no-install-recommends libclang-18-dev llvm-18-dev + sudo update-alternatives --install /usr/bin/clang clang /usr/bin/clang-18 100 + sudo update-alternatives --install /usr/bin/clang++ clang++ /usr/bin/clang++-18 100 + + - name: Install abicheck + run: pip install 'abicheck==0.5.*' + + - name: Resolve baseline release tag + id: baseline + env: + GH_TOKEN: ${{ github.token }} + run: | + tag=$(grep -v '^#' .github/abi-baseline-tag | tr -d '[:space:]') + if [ "$tag" = 'latest' ]; then + tag=$(gh release view --json tagName --jq '.tagName') + fi + echo "tag=$tag" >> "$GITHUB_OUTPUT" + + # A missing baseline is a hard failure: a green check that silently skipped + # the comparison is worse than a red one. + - name: Download release baseline + env: + GH_TOKEN: ${{ github.token }} + TAG: ${{ steps.baseline.outputs.tag }} + run: | + mkdir -p baseline + if ! gh release download "$TAG" --pattern svs-cpp-runtime-bindings.tar.gz \ + --dir baseline; then + echo "::error::Release $TAG has no svs-cpp-runtime-bindings.tar.gz asset." \ + "Fix the pin in .github/abi-baseline-tag." + exit 1 + fi + + - name: Download this build + uses: actions/download-artifact@v8 + with: + name: svs-cpp-runtime-bindings + path: current + + - name: Compare + env: + LIBRARY: libsvs_runtime.so + HEADER_SUBDIR: include/svs/runtime + SUPPRESSIONS: .github/abi-suppressions.yml + POLICY: strict_abi + run: | + chmod +x .github/scripts/abi-check.sh + label='${{ github.event.pull_request.number && format('PR {0}', github.event.pull_request.number) || github.ref_name }}' + rc=0 + .github/scripts/abi-check.sh \ + '${{ steps.baseline.outputs.tag }}' baseline/svs-cpp-runtime-bindings.tar.gz \ + "$label" current/svs-cpp-runtime-bindings.tar.gz || rc=$? + + if [ -s abi-report.md ]; then + { + echo "## ABI vs ${{ steps.baseline.outputs.tag }}" + cat abi-report.md + } >> "$GITHUB_STEP_SUMMARY" + fi + + # 77 means an input tarball was missing; that is a harness/plumbing + # problem and must fail loudly rather than pass as "no findings". + if [ "$rc" -eq 77 ] || [ "$rc" -eq 64 ]; then + echo "::error::ABI check could not run (rc=$rc)." + exit 1 + fi + exit "$rc" From 7058e9605a54180aa64fbb7a81a82aa47f07eeff Mon Sep 17 00:00:00 2001 From: ethanglaser Date: Fri, 11 Sep 2026 12:37:24 -0700 Subject: [PATCH 02/15] licenses --- .abicheck.yml | 14 ++++++++++++++ .github/abi-baseline-tag | 14 ++++++++++++++ .github/abi-suppressions.yml | 14 ++++++++++++++ .github/scripts/abi-check.sh | 2 +- 4 files changed, 43 insertions(+), 1 deletion(-) diff --git a/.abicheck.yml b/.abicheck.yml index 2e42816a0..b790f090c 100644 --- a/.abicheck.yml +++ b/.abicheck.yml @@ -1,3 +1,17 @@ +# Copyright 2026 Intel Corporation +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + # abicheck configuration, picked up from the repo root by `abicheck compare`. # Validated against the abicheck 0.5.0 config schema. # diff --git a/.github/abi-baseline-tag b/.github/abi-baseline-tag index 4fc0d008b..8eb83dd30 100644 --- a/.github/abi-baseline-tag +++ b/.github/abi-baseline-tag @@ -1,3 +1,17 @@ +# Copyright 2026 Intel Corporation +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + # The released tag that runtime-bindings ABI compatibility is measured against. # Assets are pulled from this repo's GitHub releases at check time. # diff --git a/.github/abi-suppressions.yml b/.github/abi-suppressions.yml index 267ff2ddf..cf2819f78 100644 --- a/.github/abi-suppressions.yml +++ b/.github/abi-suppressions.yml @@ -1,3 +1,17 @@ +# Copyright 2026 Intel Corporation +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + # ABI check suppression rules for the SVS C++ runtime bindings, used by # napetrov/abicheck. Owned here rather than in the innersource repo so that both # repos measure libsvs_runtime.so against the same rules -- a suppression present diff --git a/.github/scripts/abi-check.sh b/.github/scripts/abi-check.sh index 87f16ccd6..6cfd23073 100755 --- a/.github/scripts/abi-check.sh +++ b/.github/scripts/abi-check.sh @@ -1,5 +1,5 @@ #!/bin/bash -# Copyright 2025 Intel Corporation +# Copyright 2026 Intel Corporation # # Licensed under the Apache License, Version 2.0 (the "License"); # you may not use this file except in compliance with the License. From d9b275e1e729f2fa443cf8b961e421a4bcdebc8f Mon Sep 17 00:00:00 2001 From: ethanglaser <42726565+ethanglaser@users.noreply.github.com> Date: Mon, 14 Sep 2026 08:31:44 -0700 Subject: [PATCH 03/15] use abicheck 0.6 Co-authored-by: Nikolay Petrov --- .github/workflows/build-cpp-runtime-bindings.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/build-cpp-runtime-bindings.yml b/.github/workflows/build-cpp-runtime-bindings.yml index 330abd19f..8dbe6193c 100644 --- a/.github/workflows/build-cpp-runtime-bindings.yml +++ b/.github/workflows/build-cpp-runtime-bindings.yml @@ -170,7 +170,7 @@ jobs: sudo update-alternatives --install /usr/bin/clang++ clang++ /usr/bin/clang++-18 100 - name: Install abicheck - run: pip install 'abicheck==0.5.*' + run: pip install 'abicheck==0.6.*' - name: Resolve baseline release tag id: baseline From b22c521ef415a147e791e9aee03221cb8eaa5381 Mon Sep 17 00:00:00 2001 From: ethanglaser Date: Mon, 14 Sep 2026 12:16:54 -0700 Subject: [PATCH 04/15] updates to accommodate abicheck 0.6 --- .abicheck.yml | 6 +++++- .github/scripts/abi-check.sh | 22 ++++++++++++---------- 2 files changed, 17 insertions(+), 11 deletions(-) diff --git a/.abicheck.yml b/.abicheck.yml index b790f090c..a08a88db5 100644 --- a/.abicheck.yml +++ b/.abicheck.yml @@ -13,10 +13,13 @@ # limitations under the License. # abicheck configuration, picked up from the repo root by `abicheck compare`. -# Validated against the abicheck 0.5.0 config schema. +# Validated against the abicheck 0.6.0 config schema. # # Kept identical to .abicheck.yml in the innersource repo: the two repos build # the same runtime library and must not disagree on how it is parsed. +# +# 0.6 removed the --ast-frontend and --gcc-options flags, so these keys are the +# only way to set them; abi-check.sh relies on this file being found. # castxml's default cc-gnu detection parses SVS's C++20 headers at the wrong # standard (std::span / constexpr dtor fail). The clang AST frontend handles @@ -24,6 +27,7 @@ compile: frontend: clang std: c++20 + options: [-include, cstddef] # Scope the compared surface to what we publish, and collapse the std/gnu_cxx # template-instantiation noise that leaks past --exclude-libs,ALL. diff --git a/.github/scripts/abi-check.sh b/.github/scripts/abi-check.sh index 6cfd23073..e8a17afb9 100755 --- a/.github/scripts/abi-check.sh +++ b/.github/scripts/abi-check.sh @@ -92,9 +92,9 @@ suppress_args=() rc=0 # Without pipefail the pipe into tee masks abicheck's exit status. set -o pipefail -# castxml is not installed on the runners; clang is. `auto` does not fall back to -# clang on its own. If header parsing degrades, abicheck still reports from the -# ELF tier -- which is where vtable-size findings come from anyway. +# The clang AST frontend and the C++20 standard come from .abicheck.yml. abicheck +# 0.6 dropped the --ast-frontend/--gcc-options flags, so that file is now the only +# place to set them -- do not reintroduce them here. abicheck compare \ "$OLD_LIB" \ "$NEW_LIB" \ @@ -104,22 +104,24 @@ abicheck compare \ --include new="$NEW_DIR/include" \ --version old="$OLD_LABEL" \ --version new="$NEW_LABEL" \ - --ast-frontend clang \ - --gcc-options "-std=c++20 -include cstddef" \ --policy "$POLICY" \ "${suppress_args[@]}" \ - --format markdown | tee "$REPORT" || rc=$? + --output markdown=- | tee "$REPORT" || rc=$? +# Only 2 and 4 are findings. Anything else -- a removed flag, an unparseable +# header, a crash -- means the comparison did not happen, and reporting that as an +# ABI break is worse than reporting nothing: it teaches reviewers to ignore a red +# ABI check. case "$rc" in 0) echo "ABI compatible: $OLD_LABEL -> $NEW_LABEL" ;; - 64) - echo "::error::abicheck rejected the invocation (exit 64) comparing" \ - "$OLD_LABEL -> $NEW_LABEL. This is a harness bug, not an ABI finding." + 2 | 4) + echo "::warning::ABI incompatibility: $NEW_LABEL differs from $OLD_LABEL (rc=$rc)" ;; *) - echo "::warning::ABI incompatibility: $NEW_LABEL differs from $OLD_LABEL (rc=$rc)" + echo "::error::abicheck could not complete the comparison of $OLD_LABEL ->" \ + "$NEW_LABEL (rc=$rc). This is a harness bug, not an ABI finding." ;; esac From c7d84ae6cd972fe9e5e21673c323743252dbfff2 Mon Sep 17 00:00:00 2001 From: ethanglaser Date: Wed, 16 Sep 2026 12:55:39 -0700 Subject: [PATCH 05/15] compare to main instead of release, fix glob, revise shared lib logic --- .abicheck.yml | 28 ++++++- .github/abi-baseline-tag | 27 ------ .github/abi-suppressions.yml | 71 +++++++++------- .github/scripts/abi-check.sh | 45 +++++++++- .../workflows/build-cpp-runtime-bindings.yml | 82 +++++++++++++------ 5 files changed, 166 insertions(+), 87 deletions(-) delete mode 100644 .github/abi-baseline-tag diff --git a/.abicheck.yml b/.abicheck.yml index a08a88db5..f1b40c31c 100644 --- a/.abicheck.yml +++ b/.abicheck.yml @@ -24,10 +24,36 @@ # castxml's default cc-gnu detection parses SVS's C++20 headers at the wrong # standard (std::span / constexpr dtor fail). The clang AST frontend handles # C++20 cleanly; layout-only checks fall back to the .so's DWARF. +# +# The options below reproduce build assumptions the headers make but do not state. +# They are one fixed set applied to BOTH sides on purpose, not derived per side: +# abicheck's profile_fingerprint hashes compiler + macros + include content and +# refuses to compare (rc=16) when the two sides disagree, and the tarballs do +# disagree -- the private-source build declares SVS_ENABLE_IVF=1, the public one 0. +# So this list is a reviewed contract. Changing it changes what is compared. +# +# -fsized-deallocation svs::lib::Allocator calls the sized+aligned +# `::operator delete`; clang leaves the feature off, so +# libstdc++ hides that overload and the header fails. +# -DSPDLOG_FMT_EXTERNAL the shipped spdlog has no fmt/bundled/. +# -DSVS_* svs/lib/preprocessor.h static_asserts that each of +# these is exactly 0 or 1, so they cannot be left unset. +# -DSVS_VERSION_* svs/lib/version.h needs them defined. Deliberately 0/0/0 +# rather than the real version: they are constructor +# arguments, not types, so they cannot move the ABI, and +# the two sides must be given identical macros anyway. +# +# SVS_HAVE_MKL=0 matches how the compared surface is scoped, but note it does NOT +# by itself avoid MKL: svs/index/ivf/common.h includes unguarded, so the +# IVF headers are excluded in abi-check.sh instead. Revisit both together, by +# installing MKL headers on the runner, if these legs ever become blocking. compile: frontend: clang std: c++20 - options: [-include, cstddef] + options: [-include, cstddef, -fsized-deallocation, -DSPDLOG_FMT_EXTERNAL, + -DSVS_CHECK_BOUNDS=0, -DSVS_ENABLE_NUMA=0, -DSVS_INITIALIZE_LOGGER=1, + -DSVS_ENABLE_IVF=0, -DSVS_HAVE_MKL=0, + -DSVS_VERSION_MAJOR=0, -DSVS_VERSION_MINOR=0, -DSVS_VERSION_PATCH=0] # Scope the compared surface to what we publish, and collapse the std/gnu_cxx # template-instantiation noise that leaks past --exclude-libs,ALL. diff --git a/.github/abi-baseline-tag b/.github/abi-baseline-tag deleted file mode 100644 index 8eb83dd30..000000000 --- a/.github/abi-baseline-tag +++ /dev/null @@ -1,27 +0,0 @@ -# Copyright 2026 Intel Corporation -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. - -# The released tag that runtime-bindings ABI compatibility is measured against. -# Assets are pulled from this repo's GitHub releases at check time. -# -# Bumped deliberately as part of cutting a release, so that resetting the ABI -# baseline is a reviewable act rather than a side effect of tagging. Pinning also -# keeps a new release from silently moving the target under already-open PRs. -# -# `latest` is accepted and resolves dynamically, but is not the default: patch -# tags can land out of order (v0.3.1 and v0.3.2 were both tagged after v0.4.0), -# so "newest tag" is not always "current release line". -# -# Keep in sync with .github/abi-baseline-tag in the innersource repo. -v0.4.0 diff --git a/.github/abi-suppressions.yml b/.github/abi-suppressions.yml index cf2819f78..1fb15a64e 100644 --- a/.github/abi-suppressions.yml +++ b/.github/abi-suppressions.yml @@ -21,10 +21,20 @@ # Scope is the runtime bindings alone. The innersource repo keeps its own file # for libsvs_shared_library.so, which leaks a far larger third-party surface. # -# Measured on libsvs_runtime.so v0.4.0: 253 exported symbols, of which 55 are the -# declared svs::runtime::v0 API. Nearly everything suppressed below is a -# libstdc++ template instantiation that reached the export table because the -# bindings are built without -fvisibility=hidden. +# Measured on libsvs_runtime.so as this repo's CI builds it: 138 exported symbols +# -- 55 svs::runtime::v0 functions/objects, 17 svs vtable/RTTI entries, 35 leaked +# libstdc++ symbols (what the rules below cover), and 31 std::once_flag guard +# thunks emitted for local statics inside svs::index::ivf instantiations. Those 31 +# mangle as _ZZ..., so the ^_ZNSt rule does not reach them, deliberately: they are +# svs code, not libstdc++, even though the enclosing name is std::. +# +# The leak is not a missing -fvisibility=hidden -- that is set in +# bindings/cpp/CMakeLists.txt. It tracks the build path: linking the prebuilt LTO +# archive (this repo's CI) exports 138, compiling svs in-tree (innersource, and the +# release assets) exports 276. Same compiler on both sides -- verified after the +# bindings build was pinned to gcc-11 -- so this is configuration, not toolchain. +# No pattern here can close that gap; a linker version script defining the +# intended surface would. version: 1 suppressions: @@ -37,43 +47,42 @@ suppressions: reason: "C++ standard library functions" label: "stdlib" - - symbol_pattern: "^_ZN9__gnu_cxx.*" - reason: "libstdc++ __gnu_cxx internals (numeric_traits etc.), leaked" - label: "stdlib" - # RTTI/vtable/VTT for std types mangle as _ZTI/_ZTS/_ZTV/_ZTT, not _ZNSt. - symbol_pattern: "^_ZT[VIST]N?St.*" reason: "RTTI/vtable/VTT for std:: types (e.g. thread::_State_impl, future)" label: "stdlib" + # 2 of these are exported (verified with nm), so this rule is load-bearing even + # on runs where it matches nothing. - symbol_pattern: "^_ZT[VIST].*__gnu_cxx.*" reason: "RTTI/vtable for __gnu_cxx types, leaked" label: "stdlib" - - symbol_pattern: "^__cxa_.*" - reason: "C++ ABI runtime symbols" - label: "cxx-abi" - - - symbol_pattern: "^__gxx_.*" - reason: "GCC C++ runtime symbols" - label: "cxx-abi" - - # --- Header-only third-party deps that leak via template instantiation --- - - symbol_pattern: "^_ZN[0-9]+toml.*" - reason: "toml++ header-only symbols leaked via instantiation" - label: "third-party" - - - symbol_pattern: "^_ZN3fmt.*" - reason: "fmt header-only symbols leaked via instantiation" - label: "third-party" - - - symbol_pattern: "^_ZN3tsl.*" - reason: "tsl::robin_map header-only symbols leaked via instantiation" - label: "third-party" + # No rules for third-party (fmt/spdlog/tsl/toml++) or __cxa_*: neither appears in + # this library's dynamic symbol table. The vendored deps ship as static archives + # and -Wl,--exclude-libs,ALL keeps them out; __cxa_* are imports, never defined + # here. libstdc++ is a shared library, which is why the std:: rules above stay. + # + # abicheck lists every rule that matched nothing under "Stale rules", but that + # means "no change touched it this run" -- not "delete it". A rule is only dead + # once nm confirms the symbols it covers are absent from both sides. - - symbol_pattern: "^_ZN6spdlog.*" - reason: "spdlog header-only symbols leaked via instantiation" - label: "third-party" + # --- No finding_id waivers --- + # There were four, all justified against the v0.4.0 release baseline: two for the + # #345/#385 VamanaIndex vtable append, two for DynamicVamanaIndex::add appearing + # on one side only. Both checks now compare a PR against the newest build on main + # instead of a release, so neither situation can arise -- the vtable changes are + # already on main, and both sides are built the same way. + # + # If a real finding ever needs waiving, add it here as a finding_id rule (copy the + # id from the report) with a reason and an `expires`. finding_id matches one + # finding kind on one symbol but does NOT pin the old->new values, so a different + # future change to the same symbol would be waived too -- hence the expiry. + # + # Two things the old waivers were tracking are still open and no longer have a + # rule to remind us: the VamanaIndex vtable append is a break for anyone deriving + # from it, so v0.5.0 should bump SOVERSION; and DynamicVamanaIndex still emits its + # virtual dtor inline, which is why LTO can internalize it. # --- svs::runtime::v0 and svs::...detail...: deliberately NOT suppressed --- # The 55 declared v0 functions are the whole point of the check. `detail` diff --git a/.github/scripts/abi-check.sh b/.github/scripts/abi-check.sh index e8a17afb9..492d99afe 100755 --- a/.github/scripts/abi-check.sh +++ b/.github/scripts/abi-check.sh @@ -89,6 +89,47 @@ echo "Comparing $LIBRARY: [$OLD_LABEL] -> [$NEW_LABEL]" suppress_args=() [ -f "$SUPPRESSIONS" ] && suppress_args=(--suppress "$SUPPRESSIONS") +# Vendored deps ship under a versioned root (include/eve-2023.2.15/eve/...), so +# -I include alone cannot resolve their own `#include `. +# +# Only roots present on BOTH sides are added, in one sorted order. abicheck +# fingerprints the include sequence and refuses to compare (rc=16) when the two +# sides differ, so a root added to one side only voids the whole run -- and the +# private-source and public tarballs genuinely disagree about shipping eve. +include_args=(--include old="$OLD_DIR/include" --include new="$NEW_DIR/include") +for dir in "$OLD_DIR"/include/*-[0-9]*/; do + [ -d "$dir" ] || continue + vendored=$(basename "$dir") + [ -d "$NEW_DIR/include/$vendored" ] || continue + include_args+=(--include old="$OLD_DIR/include/$vendored" \ + --include new="$NEW_DIR/include/$vendored") +done + +# Headers that cannot be parsed as a translation unit, so they are never part of +# the compared surface. Only reachable when HEADER_SUBDIR is broad (the shared +# library legs); a no-op for the runtime bindings. +# core.h / lib.h documentation umbrellas, literally `static_assert(false, ...)` +# cpuid.h abicheck puts each parsed header's own directory on the +# include path, so include/svs shadows the system +# that svs/cpuid.h itself includes -- #pragma once then makes +# it a no-op and __cpuid is undeclared. Costs two inline +# svs::detail functions. +# vendored trees parsed transitively via -I where svs actually uses them; eve +# sweeps in ARM SVE headers that cannot compile on x86. +# ivf svs/index/ivf/common.h includes *unconditionally* -- +# no SVS_HAVE_MKL guard -- and no tarball ships MKL headers. So +# IVF is compared at symbol level only, not header-aware. To +# restore it, install MKL headers on the runner and drop these +# three patterns; SVS_HAVE_MKL will not do it. +exclude_args=() +# Every pattern is leading-* so it matches the full path, not just a path relative +# to HEADER_SUBDIR, which varies per leg. +for pattern in '*svs/core/core.h' '*svs/lib/lib.h' '*svs/cpuid.h' \ + '*svs/index/ivf/*' '*svs/extensions/ivf/*' '*svs/orchestrators/*ivf*' \ + '*/eve-*/*' '*/fmt/*' '*/spdlog/*' '*/tsl/*' '*/toml++/*'; do + exclude_args+=(--exclude-header "$pattern") +done + rc=0 # Without pipefail the pipe into tee masks abicheck's exit status. set -o pipefail @@ -100,8 +141,8 @@ abicheck compare \ "$NEW_LIB" \ --header old="$OLD_DIR/$HEADER_SUBDIR" \ --header new="$NEW_DIR/$HEADER_SUBDIR" \ - --include old="$OLD_DIR/include" \ - --include new="$NEW_DIR/include" \ + "${include_args[@]}" \ + "${exclude_args[@]}" \ --version old="$OLD_LABEL" \ --version new="$NEW_LABEL" \ --policy "$POLICY" \ diff --git a/.github/workflows/build-cpp-runtime-bindings.yml b/.github/workflows/build-cpp-runtime-bindings.yml index 8dbe6193c..1119d06c4 100644 --- a/.github/workflows/build-cpp-runtime-bindings.yml +++ b/.github/workflows/build-cpp-runtime-bindings.yml @@ -69,7 +69,10 @@ jobs: with: name: svs-cpp-runtime-bindings${{ matrix.suffix }} path: svs-cpp-runtime-bindings${{ matrix.suffix }}.tar.gz - retention-days: 7 # Reduce retention due to size + # Reduced due to size, but kept longer on main: those artifacts are the + # ABI baseline every open PR is measured against, so expiring them in a + # week makes the check fail on any quiet stretch. + retention-days: ${{ github.ref == 'refs/heads/main' && 30 || 7 }} - name: Upload conda package artifacts uses: actions/upload-artifact@v7 @@ -134,20 +137,33 @@ jobs: svs-manylinux228:latest \ /bin/bash .github/scripts/test-faiss.sh - # Check the built runtime bindings against the last published release. + # Check the built runtime bindings against the newest build on main. # - # Only the default variant is checked: releases publish - # svs-cpp-runtime-bindings.tar.gz but no -public-only asset, so that variant has - # nothing to compare against. It is covered indirectly -- both variants build - # from the same headers. + # Deliberately not against a published release: release assets are produced by + # the innersource private-source build, which compiles svs in-tree with + # IVF/LeanVec, while this repo's CI links the prebuilt LTO archive. Those two + # paths export genuinely different symbol sets (253 vs 138 on v0.4.0), so a + # release baseline reports build-path divergence as ABI findings and needs + # standing waivers to stay green. main-vs-PR is like-for-like by construction. + # + # The trade-off is real and accepted: this measures drift since main, not + # compatibility with what users have installed. Nothing checks the latter per-PR + # any more -- it is a release-time comparison against the previous release's + # assets, and has to be run deliberately when cutting one. + # + # Only the default variant is checked; -public-only is covered indirectly since + # both variants build from the same headers. # # The comparison primitive is .github/scripts/abi-check.sh, which the # innersource repo also calls through its submodule path so the two repos cannot # drift on how the same library is measured. abi-check: - name: ABI check against the last release + name: ABI check against main needs: build-cpp-runtime-bindings runs-on: ubuntu-22.04 + permissions: + contents: read + actions: read # gh run list/download, to fetch main's artifact as baseline # An intentionally-breaking PR still gets a full report: the label downgrades # this job from blocking to informational rather than skipping it, so the # break is recorded on the PR instead of going unmeasured. @@ -172,31 +188,45 @@ jobs: - name: Install abicheck run: pip install 'abicheck==0.6.*' - - name: Resolve baseline release tag - id: baseline - env: - GH_TOKEN: ${{ github.token }} - run: | - tag=$(grep -v '^#' .github/abi-baseline-tag | tr -d '[:space:]') - if [ "$tag" = 'latest' ]; then - tag=$(gh release view --json tagName --jq '.tagName') - fi - echo "tag=$tag" >> "$GITHUB_OUTPUT" - + # The newest main run that still has a downloadable artifact. This is main's + # tip rather than the PR's true merge-base: a PR far behind main will also + # see main's own changes, which is noisy but never a false "compatible". + # # A missing baseline is a hard failure: a green check that silently skipped # the comparison is worse than a red one. - - name: Download release baseline + - name: Download baseline build from main + id: baseline env: GH_TOKEN: ${{ github.token }} - TAG: ${{ steps.baseline.outputs.tag }} run: | mkdir -p baseline - if ! gh release download "$TAG" --pattern svs-cpp-runtime-bindings.tar.gz \ - --dir baseline; then - echo "::error::Release $TAG has no svs-cpp-runtime-bindings.tar.gz asset." \ - "Fix the pin in .github/abi-baseline-tag." + # Not filtered on conclusion: a run whose tests failed still produced a + # valid tarball, and a run that produced none just fails to download and + # falls through to the next candidate. + candidates=$(gh run list --repo "$GITHUB_REPOSITORY" \ + --workflow build-cpp-runtime-bindings.yml \ + --branch main --event push --limit 20 \ + --json databaseId,headSha --jq '.[] | "\(.databaseId) \(.headSha)"') + + label= + while read -r run_id sha; do + [ -n "$run_id" ] || continue + [ "$run_id" != "$GITHUB_RUN_ID" ] || continue + if gh run download "$run_id" --repo "$GITHUB_REPOSITORY" \ + --name svs-cpp-runtime-bindings --dir baseline; then + label="main@${sha:0:12}" + echo "Baseline: run $run_id ($label)" + break + fi + done <<< "$candidates" + + if [ -z "$label" ]; then + echo "::error::No recent main run has a downloadable" \ + "svs-cpp-runtime-bindings artifact. Re-run this workflow on main" \ + "to refresh the baseline." exit 1 fi + echo "label=$label" >> "$GITHUB_OUTPUT" - name: Download this build uses: actions/download-artifact@v8 @@ -215,12 +245,12 @@ jobs: label='${{ github.event.pull_request.number && format('PR {0}', github.event.pull_request.number) || github.ref_name }}' rc=0 .github/scripts/abi-check.sh \ - '${{ steps.baseline.outputs.tag }}' baseline/svs-cpp-runtime-bindings.tar.gz \ + '${{ steps.baseline.outputs.label }}' baseline/svs-cpp-runtime-bindings.tar.gz \ "$label" current/svs-cpp-runtime-bindings.tar.gz || rc=$? if [ -s abi-report.md ]; then { - echo "## ABI vs ${{ steps.baseline.outputs.tag }}" + echo "## ABI vs ${{ steps.baseline.outputs.label }}" cat abi-report.md } >> "$GITHUB_STEP_SUMMARY" fi From 3aa7d80d63ec95de7bf8d92dafe75c0b227c5b66 Mon Sep 17 00:00:00 2001 From: ethanglaser Date: Wed, 16 Sep 2026 17:01:18 -0700 Subject: [PATCH 06/15] Add DEPTH override to abi-check.sh The shared library's header root is the whole of svs, which does not finish parsing in an hour; --depth binary compares it in seconds. Unused by this repo's own check, which only covers the runtime bindings, but abi-check.sh is the shared primitive and its env-override surface is its API -- the innersource nightly's release comparison is the consumer. Also treats abicheck's exit 5 (budget exceeded) as a finding rather than a harness bug, and retargets comments that described the deleted innersource-vs-public equivalence check. Co-Authored-By: Claude Opus 5 --- .github/abi-suppressions.yml | 15 +++++++------- .github/scripts/abi-check.sh | 40 +++++++++++++++++++++++------------- 2 files changed, 34 insertions(+), 21 deletions(-) diff --git a/.github/abi-suppressions.yml b/.github/abi-suppressions.yml index 1fb15a64e..b4f5f09c6 100644 --- a/.github/abi-suppressions.yml +++ b/.github/abi-suppressions.yml @@ -13,10 +13,10 @@ # limitations under the License. # ABI check suppression rules for the SVS C++ runtime bindings, used by -# napetrov/abicheck. Owned here rather than in the innersource repo so that both -# repos measure libsvs_runtime.so against the same rules -- a suppression present -# on one side only would make the innersource-vs-public equivalence check report -# a difference that is really just an asymmetric filter. +# napetrov/abicheck. Owned here rather than in the innersource repo because both +# repos build and check libsvs_runtime.so: with one rule set their verdicts mean +# the same thing, and a break cannot be reported in one repo and filtered out in +# the other. # # Scope is the runtime bindings alone. The innersource repo keeps its own file # for libsvs_shared_library.so, which leaks a far larger third-party surface. @@ -70,9 +70,10 @@ suppressions: # --- No finding_id waivers --- # There were four, all justified against the v0.4.0 release baseline: two for the # #345/#385 VamanaIndex vtable append, two for DynamicVamanaIndex::add appearing - # on one side only. Both checks now compare a PR against the newest build on main - # instead of a release, so neither situation can arise -- the vtable changes are - # already on main, and both sides are built the same way. + # on one side only. Every check that uses this file compares against the newest + # build on main, so neither situation can arise -- the vtable changes are already + # on main, and both sides are built the same way. Release-baseline skew is the + # innersource nightly's problem and has its own file there. # # If a real finding ever needs waiving, add it here as a finding_id rule (copy the # id from the report) with a reason and an `expires`. finding_id matches one diff --git a/.github/scripts/abi-check.sh b/.github/scripts/abi-check.sh index 492d99afe..e0b196192 100755 --- a/.github/scripts/abi-check.sh +++ b/.github/scripts/abi-check.sh @@ -15,13 +15,13 @@ # Compare the ABI of two SVS build tarballs with napetrov/abicheck. # -# One primitive serves both questions asked of it: -# temporal old = a release tarball, new = this build -- did we break users? -# equivalence old and new are the same source revision built two ways -- did -# the build paths diverge? -# The distinction is in what the caller passes, not in the logic, so this lives -# in one place. It is called from this repo's build-cpp-runtime-bindings.yml and -# from build-share-lib.yml in the innersource repo via the submodule path. +# One primitive, three callers, all asking the same temporal question -- is `new` +# a drop-in replacement for `old`? Only the baseline differs: +# per-PR old = the newest main build, new = this build (both repos) +# nightly old = the last release asset, new = today's main build +# The distinction is entirely in what the caller passes, so this lives in one +# place: this repo's build-cpp-runtime-bindings.yml and build-share-lib.yml in +# the innersource repo, which reaches it through the submodule path. # # Usage: abi-check.sh # @@ -30,12 +30,16 @@ # HEADER_SUBDIR header root inside the tarball (default include/svs/runtime) # SUPPRESSIONS suppression file (default .github/abi-suppressions.yml) # POLICY abicheck policy (default strict_abi) +# DEPTH abicheck --depth (default: unset, i.e. abicheck's own +# 'headers'). Set to 'binary' for the shared library, whose +# header set does not finish parsing in any usable time. # REPORT markdown report path (default abi-report.md) # WORKDIR scratch directory (default abi-work) # # Exit status is abicheck's: 0 compatible, 2 source-level API break, 4 binary -# ABI break, 64 invalid invocation. 77 is added to mean "an input was missing", -# which is not a finding and must never be reported as one. +# ABI break, 5 budget exceeded, 16 not comparable, 64 invalid invocation. 77 is +# added to mean "an input was missing", which is not a finding and must never be +# reported as one. set -euo pipefail @@ -48,6 +52,7 @@ LIBRARY="${LIBRARY:-libsvs_runtime.so}" HEADER_SUBDIR="${HEADER_SUBDIR:-include/svs/runtime}" SUPPRESSIONS="${SUPPRESSIONS:-.github/abi-suppressions.yml}" POLICY="${POLICY:-strict_abi}" +DEPTH="${DEPTH:-}" REPORT="${REPORT:-abi-report.md}" WORKDIR="${WORKDIR:-abi-work}" @@ -89,6 +94,12 @@ echo "Comparing $LIBRARY: [$OLD_LABEL] -> [$NEW_LABEL]" suppress_args=() [ -f "$SUPPRESSIONS" ] && suppress_args=(--suppress "$SUPPRESSIONS") +# Left unset by default so .abicheck.yml stays the single place that configures +# depth. The --header/--include arguments below are still passed at binary depth, +# where they are simply unused, so nothing here is conditional on this. +depth_args=() +[ -n "$DEPTH" ] && depth_args=(--depth "$DEPTH") + # Vendored deps ship under a versioned root (include/eve-2023.2.15/eve/...), so # -I include alone cannot resolve their own `#include `. # @@ -146,18 +157,19 @@ abicheck compare \ --version old="$OLD_LABEL" \ --version new="$NEW_LABEL" \ --policy "$POLICY" \ + "${depth_args[@]}" \ "${suppress_args[@]}" \ --output markdown=- | tee "$REPORT" || rc=$? -# Only 2 and 4 are findings. Anything else -- a removed flag, an unparseable -# header, a crash -- means the comparison did not happen, and reporting that as an -# ABI break is worse than reporting nothing: it teaches reviewers to ignore a red -# ABI check. +# Only 2, 4 and 5 are findings. Anything else -- a removed flag, an unparseable +# header, a fingerprint mismatch (16), a crash -- means the comparison did not +# happen, and reporting that as an ABI break is worse than reporting nothing: it +# teaches reviewers to ignore a red ABI check. case "$rc" in 0) echo "ABI compatible: $OLD_LABEL -> $NEW_LABEL" ;; - 2 | 4) + 2 | 4 | 5) echo "::warning::ABI incompatibility: $NEW_LABEL differs from $OLD_LABEL (rc=$rc)" ;; *) From 3289c758466b197f3724ff5eb2648d28033a5a76 Mon Sep 17 00:00:00 2001 From: ethanglaser Date: Thu, 17 Sep 2026 07:30:45 -0700 Subject: [PATCH 07/15] Drop the dead SVS_ENABLE_IVF / SVS_HAVE_MKL defines Neither macro is referenced by any header the tarballs ship (grep over all 1992 headers of the runtime bindings tarball: 0 hits each; they are only read from .cpp files). Both build paths force IVF on -- innersource passes -DSVS_EXPERIMENTAL_ENABLE_IVF=YES and public -DSVS_RUNTIME_ENABLE_IVF=ON -- so the comment claiming the two sides disagree on SVS_ENABLE_IVF was also wrong. Verified the runtime comparison produces a byte-identical report without them. The svs/index/ivf/* header exclusions stay: svs/index/ivf/common.h really does include unguarded and no tarball ships MKL headers. Co-Authored-By: Claude Opus 5 --- .abicheck.yml | 15 ++++----------- .github/scripts/abi-check.sh | 8 +++----- 2 files changed, 7 insertions(+), 16 deletions(-) diff --git a/.abicheck.yml b/.abicheck.yml index f1b40c31c..728f1cd82 100644 --- a/.abicheck.yml +++ b/.abicheck.yml @@ -26,11 +26,10 @@ # C++20 cleanly; layout-only checks fall back to the .so's DWARF. # # The options below reproduce build assumptions the headers make but do not state. -# They are one fixed set applied to BOTH sides on purpose, not derived per side: -# abicheck's profile_fingerprint hashes compiler + macros + include content and -# refuses to compare (rc=16) when the two sides disagree, and the tarballs do -# disagree -- the private-source build declares SVS_ENABLE_IVF=1, the public one 0. -# So this list is a reviewed contract. Changing it changes what is compared. +# One fixed set applied to BOTH sides on purpose, not derived per side: abicheck's +# profile_fingerprint hashes compiler + macros + include content and refuses to +# compare (rc=16) when the two sides disagree. Changing this list changes what is +# compared, so keep it a reviewed contract. # # -fsized-deallocation svs::lib::Allocator calls the sized+aligned # `::operator delete`; clang leaves the feature off, so @@ -42,17 +41,11 @@ # rather than the real version: they are constructor # arguments, not types, so they cannot move the ABI, and # the two sides must be given identical macros anyway. -# -# SVS_HAVE_MKL=0 matches how the compared surface is scoped, but note it does NOT -# by itself avoid MKL: svs/index/ivf/common.h includes unguarded, so the -# IVF headers are excluded in abi-check.sh instead. Revisit both together, by -# installing MKL headers on the runner, if these legs ever become blocking. compile: frontend: clang std: c++20 options: [-include, cstddef, -fsized-deallocation, -DSPDLOG_FMT_EXTERNAL, -DSVS_CHECK_BOUNDS=0, -DSVS_ENABLE_NUMA=0, -DSVS_INITIALIZE_LOGGER=1, - -DSVS_ENABLE_IVF=0, -DSVS_HAVE_MKL=0, -DSVS_VERSION_MAJOR=0, -DSVS_VERSION_MINOR=0, -DSVS_VERSION_PATCH=0] # Scope the compared surface to what we publish, and collapse the std/gnu_cxx diff --git a/.github/scripts/abi-check.sh b/.github/scripts/abi-check.sh index e0b196192..175952b70 100755 --- a/.github/scripts/abi-check.sh +++ b/.github/scripts/abi-check.sh @@ -127,11 +127,9 @@ done # svs::detail functions. # vendored trees parsed transitively via -I where svs actually uses them; eve # sweeps in ARM SVE headers that cannot compile on x86. -# ivf svs/index/ivf/common.h includes *unconditionally* -- -# no SVS_HAVE_MKL guard -- and no tarball ships MKL headers. So -# IVF is compared at symbol level only, not header-aware. To -# restore it, install MKL headers on the runner and drop these -# three patterns; SVS_HAVE_MKL will not do it. +# ivf svs/index/ivf/common.h includes unguarded and no +# tarball ships MKL headers, so IVF is compared at symbol level +# only. Install MKL headers on the runner to restore it. exclude_args=() # Every pattern is leading-* so it matches the full path, not just a path relative # to HEADER_SUBDIR, which varies per leg. From ea877d7652d682fdae4764f43e55bdf1e2402bbf Mon Sep 17 00:00:00 2001 From: ethanglaser Date: Thu, 17 Sep 2026 08:41:48 -0700 Subject: [PATCH 08/15] fix for CI --- .github/workflows/build-cpp-runtime-bindings.yml | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/.github/workflows/build-cpp-runtime-bindings.yml b/.github/workflows/build-cpp-runtime-bindings.yml index 1119d06c4..42dc7a60e 100644 --- a/.github/workflows/build-cpp-runtime-bindings.yml +++ b/.github/workflows/build-cpp-runtime-bindings.yml @@ -203,10 +203,13 @@ jobs: # Not filtered on conclusion: a run whose tests failed still produced a # valid tarball, and a run that produced none just fails to download and # falls through to the next candidate. - candidates=$(gh run list --repo "$GITHUB_REPOSITORY" \ - --workflow build-cpp-runtime-bindings.yml \ - --branch main --event push --limit 20 \ - --json databaseId,headSha --jq '.[] | "\(.databaseId) \(.headSha)"') + # + # The REST endpoint rather than `gh run list`: the latter was observed + # returning stale pages (all candidates months-expired, newest run + # missing), which burns every attempt and fails the leg spuriously. + candidates=$(gh api \ + "repos/$GITHUB_REPOSITORY/actions/workflows/build-cpp-runtime-bindings.yml/runs?branch=main&event=push&per_page=20" \ + --jq '.workflow_runs[] | "\(.id) \(.head_sha)"') label= while read -r run_id sha; do From ae9199a3d79e4a8ceeb30ef054c034430a11f90d Mon Sep 17 00:00:00 2001 From: ethanglaser Date: Thu, 17 Sep 2026 15:10:07 -0700 Subject: [PATCH 09/15] Fix a stale comment on the actions: read permission --- .github/workflows/build-cpp-runtime-bindings.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/build-cpp-runtime-bindings.yml b/.github/workflows/build-cpp-runtime-bindings.yml index 42dc7a60e..8448c3119 100644 --- a/.github/workflows/build-cpp-runtime-bindings.yml +++ b/.github/workflows/build-cpp-runtime-bindings.yml @@ -163,7 +163,7 @@ jobs: runs-on: ubuntu-22.04 permissions: contents: read - actions: read # gh run list/download, to fetch main's artifact as baseline + actions: read # list runs and download artifacts, to fetch main's as baseline # An intentionally-breaking PR still gets a full report: the label downgrades # this job from blocking to informational rather than skipping it, so the # break is recorded on the PR instead of going unmeasured. From d5460d7db1281aea41aa853757309a64d61f2870 Mon Sep 17 00:00:00 2001 From: ethanglaser Date: Thu, 17 Sep 2026 16:30:50 -0700 Subject: [PATCH 10/15] Check the C API's ABI against main on every PR Mirrors the runtime bindings' check: build-c-api-bindings.yml grows an abi-check job comparing this build's svs-c-api tarball against the newest downloadable one from a main push run, through the same .github/scripts/abi-check.sh the innersource repo calls, so neither repo can drift on how libsvs_c_api.so is measured. The C API is the best-conditioned target of the three libraries. It is compiled -fvisibility=hidden and exports only `svs_*` -- which the existing integration test independently asserts -- so there are no mangled names, no template instantiations leaked from header-only dependencies and no std:: internals in the exported set. svs_c.h reaches stdbool/stddef/stdint and its own generated version header, so full header depth is both affordable and meaningful. The suppression file is therefore empty, and stays a file rather than being omitted because abi-check.sh skips a missing --suppress path silently: a typo there would otherwise be indistinguishable from deliberate emptiness. No release-relative check, in this repo or innersource: no published release ships a C API, so there is nothing to compare against yet. The artifact's retention on main goes to 30 days, since it is now the baseline every open PR is measured against. Co-Authored-By: Claude Opus 5 --- .github/abi-suppressions-c-api.yml | 33 ++++++ .github/scripts/abi-check.sh | 23 ++-- .github/workflows/build-c-api-bindings.yml | 131 ++++++++++++++++++++- 3 files changed, 177 insertions(+), 10 deletions(-) create mode 100644 .github/abi-suppressions-c-api.yml diff --git a/.github/abi-suppressions-c-api.yml b/.github/abi-suppressions-c-api.yml new file mode 100644 index 000000000..5299de480 --- /dev/null +++ b/.github/abi-suppressions-c-api.yml @@ -0,0 +1,33 @@ +# Copyright 2026 Intel Corporation +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +# ABI check suppression rules for libsvs_c_api.so, used by napetrov/abicheck in +# both this repo and the innersource one, so the same library is measured against +# the same rules wherever it is built. +# +# Intentionally empty, and expected to stay that way. The C API is compiled with +# -fvisibility=hidden and exports only `svs_*`, which bindings/c's own integration +# test already asserts, so there is no leaked-symbol class to waive: unlike the +# C++ libraries there are no mangled names, no template instantiations from +# header-only dependencies, and no std:: internals in the exported set. +# +# The file exists rather than being omitted so the --suppress path the workflows +# pass is a real one. abi-check.sh skips a missing file silently, which would make +# a future typo in that path indistinguishable from this deliberate emptiness. +# +# If something does need waiving here, that is a signal worth reading: either a +# symbol escaped the visibility annotations, or the C surface grew a C++ type. + +version: 1 +suppressions: [] diff --git a/.github/scripts/abi-check.sh b/.github/scripts/abi-check.sh index 175952b70..60c0b3da1 100755 --- a/.github/scripts/abi-check.sh +++ b/.github/scripts/abi-check.sh @@ -15,24 +15,28 @@ # Compare the ABI of two SVS build tarballs with napetrov/abicheck. # -# One primitive, three callers, all asking the same temporal question -- is `new` -# a drop-in replacement for `old`? Only the baseline differs: +# One primitive, every caller asking the same temporal question -- is `new` a +# drop-in replacement for `old`? Only the baseline differs: # per-PR old = the newest main build, new = this build (both repos) # nightly old = the last release asset, new = today's main build # The distinction is entirely in what the caller passes, so this lives in one -# place: this repo's build-cpp-runtime-bindings.yml and build-share-lib.yml in -# the innersource repo, which reaches it through the submodule path. +# place. Callers: build-cpp-runtime-bindings.yml and build-c-api-bindings.yml in +# this repo; build-share-lib.yml and build-c-api.yml in the innersource repo, +# which reach it through the submodule path. # # Usage: abi-check.sh # # Overridable via environment: # LIBRARY library basename to compare (default libsvs_runtime.so) # HEADER_SUBDIR header root inside the tarball (default include/svs/runtime) -# SUPPRESSIONS suppression file (default .github/abi-suppressions.yml) +# SUPPRESSIONS suppression file (default .github/abi-suppressions.yml). +# A missing file is skipped silently, so callers that mean to +# suppress nothing should still point at a real empty one. # POLICY abicheck policy (default strict_abi) # DEPTH abicheck --depth (default: unset, i.e. abicheck's own -# 'headers'). Set to 'binary' for the shared library, whose -# header set does not finish parsing in any usable time. +# 'headers'). 'binary' is the escape hatch for a header set that +# does not finish parsing in any usable time, as the shared +# library's does not; no current caller needs it. # REPORT markdown report path (default abi-report.md) # WORKDIR scratch directory (default abi-work) # @@ -117,8 +121,9 @@ for dir in "$OLD_DIR"/include/*-[0-9]*/; do done # Headers that cannot be parsed as a translation unit, so they are never part of -# the compared surface. Only reachable when HEADER_SUBDIR is broad (the shared -# library legs); a no-op for the runtime bindings. +# the compared surface. Only reachable when HEADER_SUBDIR is broad, which no +# current caller's is -- a no-op for the runtime bindings and the C API, kept for +# whenever a whole-of-svs header root is compared again. # core.h / lib.h documentation umbrellas, literally `static_assert(false, ...)` # cpuid.h abicheck puts each parsed header's own directory on the # include path, so include/svs shadows the system diff --git a/.github/workflows/build-c-api-bindings.yml b/.github/workflows/build-c-api-bindings.yml index a70c68ef1..280966dd0 100644 --- a/.github/workflows/build-c-api-bindings.yml +++ b/.github/workflows/build-c-api-bindings.yml @@ -69,7 +69,10 @@ jobs: with: name: svs-c-api${{ matrix.suffix }} path: svs-c-api${{ matrix.suffix }}.tar.gz - retention-days: 7 + # Reduced due to size, but kept longer on main: those artifacts are the ABI + # baseline every open PR is measured against, so expiring them in a week + # makes the check fail on any quiet stretch. + retention-days: ${{ github.ref == 'refs/heads/main' && 30 || 7 }} # Run unit tests that were built as part of this job - name: Run unit tests in Docker container @@ -121,3 +124,129 @@ jobs: -e SUFFIX=${{ matrix.suffix }} \ svs-manylinux228:latest \ /bin/bash .github/scripts/test-c-api-bindings.sh + + # Check the built C API against the newest build on main. + # + # Deliberately not against a published release: no release ships a C API yet, so + # there is nothing to compare against. Once svs-c-api.tar.gz is a release asset + # that becomes a release-time comparison, run when cutting one, the same way the + # runtime bindings are handled -- see the note in build-cpp-runtime-bindings.yml. + # + # The best-conditioned of the libraries checked here. Pure C behind + # -fvisibility=hidden, so the exported set is already exactly the intended ABI, + # which the integration test above independently asserts; and svs_c.h reaches only + # stdbool/stddef/stdint plus its own generated version header. Hence full header + # depth and an empty suppression file. + # + # Only the default variant is checked; -public-only is covered indirectly since + # both variants build from the same headers. + # + # The comparison primitive is .github/scripts/abi-check.sh, which the innersource + # repo also calls through its submodule path so the two repos cannot drift on how + # the same library is measured. + abi-check: + name: ABI check against main + needs: build-c-api-bindings + runs-on: ubuntu-22.04 + permissions: + contents: read + actions: read # list runs and download artifacts, to fetch main's as baseline + # An intentionally-breaking PR still gets a full report: the label downgrades + # this job from blocking to informational rather than skipping it, so the + # break is recorded on the PR instead of going unmeasured. + continue-on-error: ${{ contains(toJson(github.event.pull_request.labels.*.name), '"API/ABI breaking change"') }} + + steps: + - uses: actions/checkout@v6 + + # Same reason as in build-cpp-runtime-bindings.yml: the hosted image's + # clang-13/14 degrades abicheck's AST frontend, and innersource measures with a + # c++20-capable clang from its self-hosted toolchain. + - name: Install clang for the AST frontend + run: | + wget -qO /tmp/llvm.sh https://apt.llvm.org/llvm.sh + chmod +x /tmp/llvm.sh + sudo /tmp/llvm.sh 18 + sudo apt-get install -y --no-install-recommends libclang-18-dev llvm-18-dev + sudo update-alternatives --install /usr/bin/clang clang /usr/bin/clang-18 100 + sudo update-alternatives --install /usr/bin/clang++ clang++ /usr/bin/clang++-18 100 + + - name: Install abicheck + run: pip install 'abicheck==0.6.*' + + # The newest main run that still has a downloadable artifact. This is main's + # tip rather than the PR's true merge-base: a PR far behind main will also + # see main's own changes, which is noisy but never a false "compatible". + # + # A missing baseline is a hard failure: a green check that silently skipped + # the comparison is worse than a red one. + - name: Download baseline build from main + id: baseline + env: + GH_TOKEN: ${{ github.token }} + run: | + mkdir -p baseline + # Not filtered on conclusion: a run whose tests failed still produced a + # valid tarball, and a run that produced none just fails to download and + # falls through to the next candidate. + # + # The REST endpoint rather than `gh run list`: the latter was observed + # returning stale pages (all candidates months-expired, newest run + # missing), which burns every attempt and fails the leg spuriously. + candidates=$(gh api \ + "repos/$GITHUB_REPOSITORY/actions/workflows/build-c-api-bindings.yml/runs?branch=main&event=push&per_page=20" \ + --jq '.workflow_runs[] | "\(.id) \(.head_sha)"') + + label= + while read -r run_id sha; do + [ -n "$run_id" ] || continue + [ "$run_id" != "$GITHUB_RUN_ID" ] || continue + if gh run download "$run_id" --repo "$GITHUB_REPOSITORY" \ + --name svs-c-api --dir baseline; then + label="main@${sha:0:12}" + echo "Baseline: run $run_id ($label)" + break + fi + done <<< "$candidates" + + if [ -z "$label" ]; then + echo "::error::No recent main run has a downloadable svs-c-api" \ + "artifact. Re-run this workflow on main to refresh the baseline." + exit 1 + fi + echo "label=$label" >> "$GITHUB_OUTPUT" + + - name: Download this build + uses: actions/download-artifact@v8 + with: + name: svs-c-api + path: current + + - name: Compare + env: + LIBRARY: libsvs_c_api.so + HEADER_SUBDIR: include/svs/c + SUPPRESSIONS: .github/abi-suppressions-c-api.yml + POLICY: strict_abi + run: | + chmod +x .github/scripts/abi-check.sh + label='${{ github.event.pull_request.number && format('PR {0}', github.event.pull_request.number) || github.ref_name }}' + rc=0 + .github/scripts/abi-check.sh \ + '${{ steps.baseline.outputs.label }}' baseline/svs-c-api.tar.gz \ + "$label" current/svs-c-api.tar.gz || rc=$? + + if [ -s abi-report.md ]; then + { + echo "## ABI vs ${{ steps.baseline.outputs.label }}" + cat abi-report.md + } >> "$GITHUB_STEP_SUMMARY" + fi + + # 77 means an input tarball was missing; that is a harness/plumbing + # problem and must fail loudly rather than pass as "no findings". + if [ "$rc" -eq 77 ] || [ "$rc" -eq 64 ]; then + echo "::error::ABI check could not run (rc=$rc)." + exit 1 + fi + exit "$rc" From af279120a94e6782ba3fc1c05be1fec2fde55a67 Mon Sep 17 00:00:00 2001 From: ethanglaser Date: Tue, 22 Sep 2026 10:55:17 -0700 Subject: [PATCH 11/15] clean up and reduce verbosity --- .abicheck.yml | 33 +++----- .github/abi-suppressions-c-api.yml | 18 ++-- .github/abi-suppressions.yml | 73 ++++++---------- .github/dependabot.yml | 11 +++ .github/scripts/abi-check.sh | 83 +++++++------------ .github/scripts/abi-requirements.txt | 3 + .github/workflows/build-c-api-bindings.yml | 65 ++++----------- .../workflows/build-cpp-runtime-bindings.yml | 65 ++++----------- 8 files changed, 121 insertions(+), 230 deletions(-) create mode 100644 .github/scripts/abi-requirements.txt diff --git a/.abicheck.yml b/.abicheck.yml index 728f1cd82..c2a58b1e3 100644 --- a/.abicheck.yml +++ b/.abicheck.yml @@ -13,34 +13,21 @@ # limitations under the License. # abicheck configuration, picked up from the repo root by `abicheck compare`. -# Validated against the abicheck 0.6.0 config schema. -# -# Kept identical to .abicheck.yml in the innersource repo: the two repos build -# the same runtime library and must not disagree on how it is parsed. -# -# 0.6 removed the --ast-frontend and --gcc-options flags, so these keys are the -# only way to set them; abi-check.sh relies on this file being found. -# castxml's default cc-gnu detection parses SVS's C++20 headers at the wrong -# standard (std::span / constexpr dtor fail). The clang AST frontend handles -# C++20 cleanly; layout-only checks fall back to the .so's DWARF. +# frontend: clang because castxml's cc-gnu detection parses SVS's C++20 headers at +# the wrong standard. Layout-only checks fall back to the .so's DWARF. # -# The options below reproduce build assumptions the headers make but do not state. -# One fixed set applied to BOTH sides on purpose, not derived per side: abicheck's -# profile_fingerprint hashes compiler + macros + include content and refuses to -# compare (rc=16) when the two sides disagree. Changing this list changes what is -# compared, so keep it a reviewed contract. +# The options reproduce build assumptions the headers make but do not state. One +# fixed set applied to BOTH sides on purpose: abicheck fingerprints compiler + +# macros + includes and refuses to compare (rc=16) if the two sides disagree. # # -fsized-deallocation svs::lib::Allocator calls the sized+aligned -# `::operator delete`; clang leaves the feature off, so -# libstdc++ hides that overload and the header fails. +# `::operator delete`, which clang leaves off by default. # -DSPDLOG_FMT_EXTERNAL the shipped spdlog has no fmt/bundled/. -# -DSVS_* svs/lib/preprocessor.h static_asserts that each of -# these is exactly 0 or 1, so they cannot be left unset. -# -DSVS_VERSION_* svs/lib/version.h needs them defined. Deliberately 0/0/0 -# rather than the real version: they are constructor -# arguments, not types, so they cannot move the ABI, and -# the two sides must be given identical macros anyway. +# -DSVS_* svs/lib/preprocessor.h static_asserts each is 0 or 1. +# -DSVS_VERSION_* svs/lib/version.h needs them defined; 0/0/0 because they +# are constructor arguments, not types, so they cannot +# move the ABI and must match on both sides anyway. compile: frontend: clang std: c++20 diff --git a/.github/abi-suppressions-c-api.yml b/.github/abi-suppressions-c-api.yml index 5299de480..1bcbb60f5 100644 --- a/.github/abi-suppressions-c-api.yml +++ b/.github/abi-suppressions-c-api.yml @@ -12,22 +12,16 @@ # See the License for the specific language governing permissions and # limitations under the License. -# ABI check suppression rules for libsvs_c_api.so, used by napetrov/abicheck in -# both this repo and the innersource one, so the same library is measured against -# the same rules wherever it is built. +# ABI check suppression rules for libsvs_c_api.so. # -# Intentionally empty, and expected to stay that way. The C API is compiled with -# -fvisibility=hidden and exports only `svs_*`, which bindings/c's own integration -# test already asserts, so there is no leaked-symbol class to waive: unlike the -# C++ libraries there are no mangled names, no template instantiations from -# header-only dependencies, and no std:: internals in the exported set. +# Intentionally empty, and expected to stay that way: the C API is compiled with +# -fvisibility=hidden and exports only `svs_*`, so there are no mangled names, no +# template instantiations from header-only dependencies, and no std:: internals to +# waive -- unlike the C++ libraries, whose exported set leaks all three. # # The file exists rather than being omitted so the --suppress path the workflows # pass is a real one. abi-check.sh skips a missing file silently, which would make -# a future typo in that path indistinguishable from this deliberate emptiness. -# -# If something does need waiving here, that is a signal worth reading: either a -# symbol escaped the visibility annotations, or the C surface grew a C++ type. +# a typo in that path indistinguishable from this deliberate emptiness. version: 1 suppressions: [] diff --git a/.github/abi-suppressions.yml b/.github/abi-suppressions.yml index b4f5f09c6..502c9fd1f 100644 --- a/.github/abi-suppressions.yml +++ b/.github/abi-suppressions.yml @@ -12,29 +12,19 @@ # See the License for the specific language governing permissions and # limitations under the License. -# ABI check suppression rules for the SVS C++ runtime bindings, used by -# napetrov/abicheck. Owned here rather than in the innersource repo because both -# repos build and check libsvs_runtime.so: with one rule set their verdicts mean -# the same thing, and a break cannot be reported in one repo and filtered out in -# the other. +# ABI check suppression rules for the SVS C++ runtime bindings. # -# Scope is the runtime bindings alone. The innersource repo keeps its own file -# for libsvs_shared_library.so, which leaks a far larger third-party surface. +# Measured on libsvs_runtime.so as this repo's CI builds it: of 138 exported +# symbols, 35 are leaked libstdc++ template instantiations, which is what the rules +# below cover. Another 31 are std::once_flag guard thunks for local statics inside +# svs::index::ivf instantiations; they mangle as _ZZ..., so `^_ZNSt` deliberately +# does not reach them -- they are svs code despite the std:: in the name. # -# Measured on libsvs_runtime.so as this repo's CI builds it: 138 exported symbols -# -- 55 svs::runtime::v0 functions/objects, 17 svs vtable/RTTI entries, 35 leaked -# libstdc++ symbols (what the rules below cover), and 31 std::once_flag guard -# thunks emitted for local statics inside svs::index::ivf instantiations. Those 31 -# mangle as _ZZ..., so the ^_ZNSt rule does not reach them, deliberately: they are -# svs code, not libstdc++, even though the enclosing name is std::. -# -# The leak is not a missing -fvisibility=hidden -- that is set in -# bindings/cpp/CMakeLists.txt. It tracks the build path: linking the prebuilt LTO -# archive (this repo's CI) exports 138, compiling svs in-tree (innersource, and the -# release assets) exports 276. Same compiler on both sides -- verified after the -# bindings build was pinned to gcc-11 -- so this is configuration, not toolchain. -# No pattern here can close that gap; a linker version script defining the -# intended surface would. +# The leak is not a missing -fvisibility=hidden (that is set in +# bindings/cpp/CMakeLists.txt); it tracks the build path. Linking the prebuilt LTO +# archive exports 138 symbols, compiling svs in-tree exports 276, with the same +# compiler either way. No pattern here can close that gap; a linker version script +# defining the intended surface would. version: 1 suppressions: @@ -58,35 +48,20 @@ suppressions: reason: "RTTI/vtable for __gnu_cxx types, leaked" label: "stdlib" - # No rules for third-party (fmt/spdlog/tsl/toml++) or __cxa_*: neither appears in - # this library's dynamic symbol table. The vendored deps ship as static archives - # and -Wl,--exclude-libs,ALL keeps them out; __cxa_* are imports, never defined - # here. libstdc++ is a shared library, which is why the std:: rules above stay. + # No rules for third-party (fmt/spdlog/tsl/toml++) or __cxa_*: the vendored deps + # ship as static archives that -Wl,--exclude-libs,ALL keeps out, and __cxa_* are + # imports, never defined here. libstdc++ is a shared library, hence the std:: rules. # - # abicheck lists every rule that matched nothing under "Stale rules", but that - # means "no change touched it this run" -- not "delete it". A rule is only dead - # once nm confirms the symbols it covers are absent from both sides. + # abicheck lists rules that matched nothing under "Stale rules", which means "no + # change touched it this run", not "delete it". - # --- No finding_id waivers --- - # There were four, all justified against the v0.4.0 release baseline: two for the - # #345/#385 VamanaIndex vtable append, two for DynamicVamanaIndex::add appearing - # on one side only. Every check that uses this file compares against the newest - # build on main, so neither situation can arise -- the vtable changes are already - # on main, and both sides are built the same way. Release-baseline skew is the - # innersource nightly's problem and has its own file there. - # - # If a real finding ever needs waiving, add it here as a finding_id rule (copy the - # id from the report) with a reason and an `expires`. finding_id matches one - # finding kind on one symbol but does NOT pin the old->new values, so a different - # future change to the same symbol would be waived too -- hence the expiry. - # - # Two things the old waivers were tracking are still open and no longer have a - # rule to remind us: the VamanaIndex vtable append is a break for anyone deriving - # from it, so v0.5.0 should bump SOVERSION; and DynamicVamanaIndex still emits its - # virtual dtor inline, which is why LTO can internalize it. + # --- Waivers --- + # None. To waive a real finding, add a finding_id rule (id copied from the report) + # with a reason and an `expires`. finding_id matches one finding kind on one symbol + # but does not pin the old->new values, so a later unrelated change to the same + # symbol would be waived silently too -- hence the expiry. # --- svs::runtime::v0 and svs::...detail...: deliberately NOT suppressed --- - # The 55 declared v0 functions are the whole point of the check. `detail` - # symbols are not suppressed either: users reach them transitively through - # template instantiation, so a vtable or layout change there is a real break. - # See the same note in the innersource suppression file. + # The 55 declared v0 functions are the whole point of the check. `detail` symbols + # are not suppressed either: users reach them transitively through template + # instantiation, so a vtable or layout change there is a real break. diff --git a/.github/dependabot.yml b/.github/dependabot.yml index f60e31d3f..893e8b2ae 100644 --- a/.github/dependabot.yml +++ b/.github/dependabot.yml @@ -27,3 +27,14 @@ updates: github-actions: patterns: - "*" + + # A bump PR runs the ABI check under the new abicheck, so tool-level behaviour + # changes show up as a diff in the report rather than as a surprise on main. + - package-ecosystem: "pip" + directory: "/.github/scripts" + schedule: + interval: "monthly" + commit-message: + prefix: "chore(abicheck)" + labels: + - dependencies diff --git a/.github/scripts/abi-check.sh b/.github/scripts/abi-check.sh index 60c0b3da1..37fb4df5e 100755 --- a/.github/scripts/abi-check.sh +++ b/.github/scripts/abi-check.sh @@ -13,37 +13,29 @@ # See the License for the specific language governing permissions and # limitations under the License. -# Compare the ABI of two SVS build tarballs with napetrov/abicheck. +# Compare the ABI of two SVS build tarballs with abicheck. # -# One primitive, every caller asking the same temporal question -- is `new` a -# drop-in replacement for `old`? Only the baseline differs: -# per-PR old = the newest main build, new = this build (both repos) -# nightly old = the last release asset, new = today's main build -# The distinction is entirely in what the caller passes, so this lives in one -# place. Callers: build-cpp-runtime-bindings.yml and build-c-api-bindings.yml in -# this repo; build-share-lib.yml and build-c-api.yml in the innersource repo, -# which reach it through the submodule path. +# Every caller asks the same question -- is `new` a drop-in replacement for `old`? +# -- and differs only in the baseline it passes (newest main build per PR, previous +# release asset when cutting one), so the comparison lives in one place. # # Usage: abi-check.sh # # Overridable via environment: # LIBRARY library basename to compare (default libsvs_runtime.so) # HEADER_SUBDIR header root inside the tarball (default include/svs/runtime) -# SUPPRESSIONS suppression file (default .github/abi-suppressions.yml). -# A missing file is skipped silently, so callers that mean to -# suppress nothing should still point at a real empty one. +# SUPPRESSIONS suppression file (default .github/abi-suppressions.yml); a +# missing file is skipped silently, so callers that suppress +# nothing should still point at a real empty one # POLICY abicheck policy (default strict_abi) -# DEPTH abicheck --depth (default: unset, i.e. abicheck's own -# 'headers'). 'binary' is the escape hatch for a header set that -# does not finish parsing in any usable time, as the shared -# library's does not; no current caller needs it. +# DEPTH abicheck --depth (default unset, i.e. abicheck's 'headers'); +# 'binary' is the escape hatch for a header set too large to parse # REPORT markdown report path (default abi-report.md) # WORKDIR scratch directory (default abi-work) # -# Exit status is abicheck's: 0 compatible, 2 source-level API break, 4 binary -# ABI break, 5 budget exceeded, 16 not comparable, 64 invalid invocation. 77 is -# added to mean "an input was missing", which is not a finding and must never be -# reported as one. +# Exit status is abicheck's: 0 compatible, 2 source-level API break, 4 binary ABI +# break, 5 budget exceeded, 16 not comparable, 64 invalid invocation. 77 is added +# to mean "an input was missing", which is not a finding. set -euo pipefail @@ -98,19 +90,17 @@ echo "Comparing $LIBRARY: [$OLD_LABEL] -> [$NEW_LABEL]" suppress_args=() [ -f "$SUPPRESSIONS" ] && suppress_args=(--suppress "$SUPPRESSIONS") -# Left unset by default so .abicheck.yml stays the single place that configures -# depth. The --header/--include arguments below are still passed at binary depth, -# where they are simply unused, so nothing here is conditional on this. +# Left unset by default so .abicheck.yml stays the single place that sets depth. depth_args=() [ -n "$DEPTH" ] && depth_args=(--depth "$DEPTH") # Vendored deps ship under a versioned root (include/eve-2023.2.15/eve/...), so # -I include alone cannot resolve their own `#include `. # -# Only roots present on BOTH sides are added, in one sorted order. abicheck +# Only roots present on BOTH sides are added, in one sorted order: abicheck # fingerprints the include sequence and refuses to compare (rc=16) when the two -# sides differ, so a root added to one side only voids the whole run -- and the -# private-source and public tarballs genuinely disagree about shipping eve. +# sides differ, so a root added to one side only voids the whole run. Tarballs do +# genuinely disagree about shipping eve, hence the intersection rather than a union. include_args=(--include old="$OLD_DIR/include" --include new="$NEW_DIR/include") for dir in "$OLD_DIR"/include/*-[0-9]*/; do [ -d "$dir" ] || continue @@ -120,24 +110,19 @@ for dir in "$OLD_DIR"/include/*-[0-9]*/; do --include new="$NEW_DIR/include/$vendored") done -# Headers that cannot be parsed as a translation unit, so they are never part of -# the compared surface. Only reachable when HEADER_SUBDIR is broad, which no -# current caller's is -- a no-op for the runtime bindings and the C API, kept for -# whenever a whole-of-svs header root is compared again. +# Headers that cannot be parsed as a translation unit. Inert for the narrow header +# roots the current callers pass; needed as soon as a whole-of-svs root is compared. # core.h / lib.h documentation umbrellas, literally `static_assert(false, ...)` -# cpuid.h abicheck puts each parsed header's own directory on the -# include path, so include/svs shadows the system -# that svs/cpuid.h itself includes -- #pragma once then makes -# it a no-op and __cpuid is undeclared. Costs two inline -# svs::detail functions. -# vendored trees parsed transitively via -I where svs actually uses them; eve -# sweeps in ARM SVE headers that cannot compile on x86. -# ivf svs/index/ivf/common.h includes unguarded and no -# tarball ships MKL headers, so IVF is compared at symbol level -# only. Install MKL headers on the runner to restore it. +# cpuid.h abicheck puts each parsed header's own directory on the include +# path, so include/svs shadows the system that +# svs/cpuid.h includes; #pragma once then makes it a no-op and +# __cpuid is undeclared. Costs two inline svs::detail functions. +# vendored trees parsed transitively via -I where svs uses them; eve sweeps in +# ARM SVE headers that cannot compile on x86. +# ivf svs/index/ivf/common.h includes unguarded and no tarball +# ships MKL headers, so IVF is compared at symbol level only. exclude_args=() -# Every pattern is leading-* so it matches the full path, not just a path relative -# to HEADER_SUBDIR, which varies per leg. +# Leading-* so each pattern matches the full path, not one relative to HEADER_SUBDIR. for pattern in '*svs/core/core.h' '*svs/lib/lib.h' '*svs/cpuid.h' \ '*svs/index/ivf/*' '*svs/extensions/ivf/*' '*svs/orchestrators/*ivf*' \ '*/eve-*/*' '*/fmt/*' '*/spdlog/*' '*/tsl/*' '*/toml++/*'; do @@ -145,11 +130,8 @@ for pattern in '*svs/core/core.h' '*svs/lib/lib.h' '*svs/cpuid.h' \ done rc=0 -# Without pipefail the pipe into tee masks abicheck's exit status. -set -o pipefail -# The clang AST frontend and the C++20 standard come from .abicheck.yml. abicheck -# 0.6 dropped the --ast-frontend/--gcc-options flags, so that file is now the only -# place to set them -- do not reintroduce them here. +# The AST frontend, C++20 standard and compile options come from .abicheck.yml; +# abicheck 0.6 dropped the equivalent flags, so do not reintroduce them here. abicheck compare \ "$OLD_LIB" \ "$NEW_LIB" \ @@ -164,10 +146,9 @@ abicheck compare \ "${suppress_args[@]}" \ --output markdown=- | tee "$REPORT" || rc=$? -# Only 2, 4 and 5 are findings. Anything else -- a removed flag, an unparseable -# header, a fingerprint mismatch (16), a crash -- means the comparison did not -# happen, and reporting that as an ABI break is worse than reporting nothing: it -# teaches reviewers to ignore a red ABI check. +# Only 2, 4 and 5 are findings. Anything else -- an unparseable header, a +# fingerprint mismatch (16), a crash -- means the comparison did not happen, and +# must not be reported as an ABI break. case "$rc" in 0) echo "ABI compatible: $OLD_LABEL -> $NEW_LABEL" diff --git a/.github/scripts/abi-requirements.txt b/.github/scripts/abi-requirements.txt new file mode 100644 index 000000000..4d61b6d0d --- /dev/null +++ b/.github/scripts/abi-requirements.txt @@ -0,0 +1,3 @@ +# Pinned so an abicheck release cannot turn a green ABI check red, or vice versa, +# without a reviewed commit. .abicheck.yml is written against this version's schema. +abicheck==0.6.0 diff --git a/.github/workflows/build-c-api-bindings.yml b/.github/workflows/build-c-api-bindings.yml index 280966dd0..7c4653099 100644 --- a/.github/workflows/build-c-api-bindings.yml +++ b/.github/workflows/build-c-api-bindings.yml @@ -69,9 +69,7 @@ jobs: with: name: svs-c-api${{ matrix.suffix }} path: svs-c-api${{ matrix.suffix }}.tar.gz - # Reduced due to size, but kept longer on main: those artifacts are the ABI - # baseline every open PR is measured against, so expiring them in a week - # makes the check fail on any quiet stretch. + # Extend main retention for ABI comparison; keep PR builds short-lived. retention-days: ${{ github.ref == 'refs/heads/main' && 30 || 7 }} # Run unit tests that were built as part of this job @@ -125,25 +123,6 @@ jobs: svs-manylinux228:latest \ /bin/bash .github/scripts/test-c-api-bindings.sh - # Check the built C API against the newest build on main. - # - # Deliberately not against a published release: no release ships a C API yet, so - # there is nothing to compare against. Once svs-c-api.tar.gz is a release asset - # that becomes a release-time comparison, run when cutting one, the same way the - # runtime bindings are handled -- see the note in build-cpp-runtime-bindings.yml. - # - # The best-conditioned of the libraries checked here. Pure C behind - # -fvisibility=hidden, so the exported set is already exactly the intended ABI, - # which the integration test above independently asserts; and svs_c.h reaches only - # stdbool/stddef/stdint plus its own generated version header. Hence full header - # depth and an empty suppression file. - # - # Only the default variant is checked; -public-only is covered indirectly since - # both variants build from the same headers. - # - # The comparison primitive is .github/scripts/abi-check.sh, which the innersource - # repo also calls through its submodule path so the two repos cannot drift on how - # the same library is measured. abi-check: name: ABI check against main needs: build-c-api-bindings @@ -151,17 +130,15 @@ jobs: permissions: contents: read actions: read # list runs and download artifacts, to fetch main's as baseline - # An intentionally-breaking PR still gets a full report: the label downgrades - # this job from blocking to informational rather than skipping it, so the - # break is recorded on the PR instead of going unmeasured. + # The label downgrades this job to informational rather than skipping it, so an + # intentional break is still reported rather than going unmeasured. continue-on-error: ${{ contains(toJson(github.event.pull_request.labels.*.name), '"API/ABI breaking change"') }} steps: - uses: actions/checkout@v6 - # Same reason as in build-cpp-runtime-bindings.yml: the hosted image's - # clang-13/14 degrades abicheck's AST frontend, and innersource measures with a - # c++20-capable clang from its self-hosted toolchain. + # The hosted image's clang-13/14 degrades abicheck's AST frontend to ELF-tier + # and drops constrained-template diffs, so install a C++20-capable clang. - name: Install clang for the AST frontend run: | wget -qO /tmp/llvm.sh https://apt.llvm.org/llvm.sh @@ -172,14 +149,10 @@ jobs: sudo update-alternatives --install /usr/bin/clang++ clang++ /usr/bin/clang++-18 100 - name: Install abicheck - run: pip install 'abicheck==0.6.*' - - # The newest main run that still has a downloadable artifact. This is main's - # tip rather than the PR's true merge-base: a PR far behind main will also - # see main's own changes, which is noisy but never a false "compatible". - # - # A missing baseline is a hard failure: a green check that silently skipped - # the comparison is worse than a red one. + run: pip install -r .github/scripts/abi-requirements.txt + + # Baseline is main's tip, not the PR's merge-base: a PR far behind main also + # sees main's own changes, which is noisy but never a false "compatible". - name: Download baseline build from main id: baseline env: @@ -190,9 +163,8 @@ jobs: # valid tarball, and a run that produced none just fails to download and # falls through to the next candidate. # - # The REST endpoint rather than `gh run list`: the latter was observed - # returning stale pages (all candidates months-expired, newest run - # missing), which burns every attempt and fails the leg spuriously. + # REST rather than `gh run list`, which was observed returning stale pages + # that omitted the newest run and burned every candidate on expired ones. candidates=$(gh api \ "repos/$GITHUB_REPOSITORY/actions/workflows/build-c-api-bindings.yml/runs?branch=main&event=push&per_page=20" \ --jq '.workflow_runs[] | "\(.id) \(.head_sha)"') @@ -216,6 +188,7 @@ jobs: fi echo "label=$label" >> "$GITHUB_OUTPUT" + # Default variant only; -public-only builds from the same headers. - name: Download this build uses: actions/download-artifact@v8 with: @@ -229,7 +202,6 @@ jobs: SUPPRESSIONS: .github/abi-suppressions-c-api.yml POLICY: strict_abi run: | - chmod +x .github/scripts/abi-check.sh label='${{ github.event.pull_request.number && format('PR {0}', github.event.pull_request.number) || github.ref_name }}' rc=0 .github/scripts/abi-check.sh \ @@ -243,10 +215,9 @@ jobs: } >> "$GITHUB_STEP_SUMMARY" fi - # 77 means an input tarball was missing; that is a harness/plumbing - # problem and must fail loudly rather than pass as "no findings". - if [ "$rc" -eq 77 ] || [ "$rc" -eq 64 ]; then - echo "::error::ABI check could not run (rc=$rc)." - exit 1 - fi - exit "$rc" + # Only 2/4/5 are ABI findings. Any other non-zero code means the comparison + # never happened, which must fail as plumbing rather than read as a break. + case "$rc" in + 0 | 2 | 4 | 5) exit "$rc" ;; + *) echo "::error::ABI check could not run (rc=$rc)."; exit 1 ;; + esac diff --git a/.github/workflows/build-cpp-runtime-bindings.yml b/.github/workflows/build-cpp-runtime-bindings.yml index 8448c3119..fbf2339c2 100644 --- a/.github/workflows/build-cpp-runtime-bindings.yml +++ b/.github/workflows/build-cpp-runtime-bindings.yml @@ -69,9 +69,7 @@ jobs: with: name: svs-cpp-runtime-bindings${{ matrix.suffix }} path: svs-cpp-runtime-bindings${{ matrix.suffix }}.tar.gz - # Reduced due to size, but kept longer on main: those artifacts are the - # ABI baseline every open PR is measured against, so expiring them in a - # week makes the check fail on any quiet stretch. + # Extend main retention for ABI comparison; keep PR builds short-lived. retention-days: ${{ github.ref == 'refs/heads/main' && 30 || 7 }} - name: Upload conda package artifacts @@ -137,26 +135,6 @@ jobs: svs-manylinux228:latest \ /bin/bash .github/scripts/test-faiss.sh - # Check the built runtime bindings against the newest build on main. - # - # Deliberately not against a published release: release assets are produced by - # the innersource private-source build, which compiles svs in-tree with - # IVF/LeanVec, while this repo's CI links the prebuilt LTO archive. Those two - # paths export genuinely different symbol sets (253 vs 138 on v0.4.0), so a - # release baseline reports build-path divergence as ABI findings and needs - # standing waivers to stay green. main-vs-PR is like-for-like by construction. - # - # The trade-off is real and accepted: this measures drift since main, not - # compatibility with what users have installed. Nothing checks the latter per-PR - # any more -- it is a release-time comparison against the previous release's - # assets, and has to be run deliberately when cutting one. - # - # Only the default variant is checked; -public-only is covered indirectly since - # both variants build from the same headers. - # - # The comparison primitive is .github/scripts/abi-check.sh, which the - # innersource repo also calls through its submodule path so the two repos cannot - # drift on how the same library is measured. abi-check: name: ABI check against main needs: build-cpp-runtime-bindings @@ -164,18 +142,15 @@ jobs: permissions: contents: read actions: read # list runs and download artifacts, to fetch main's as baseline - # An intentionally-breaking PR still gets a full report: the label downgrades - # this job from blocking to informational rather than skipping it, so the - # break is recorded on the PR instead of going unmeasured. + # The label downgrades this job to informational rather than skipping it, so an + # intentional break is still reported rather than going unmeasured. continue-on-error: ${{ contains(toJson(github.event.pull_request.labels.*.name), '"API/ABI breaking change"') }} steps: - uses: actions/checkout@v6 - # The hosted ubuntu-22.04 image ships only clang-13/14, which degrades - # abicheck's clang AST frontend to ELF-tier and drops constrained-template - # diffs. innersource gets a c++20-capable clang from its self-hosted - # toolchain; this keeps the two repos measuring at the same fidelity. + # The hosted image's clang-13/14 degrades abicheck's AST frontend to ELF-tier + # and drops constrained-template diffs, so install a C++20-capable clang. - name: Install clang for the AST frontend run: | wget -qO /tmp/llvm.sh https://apt.llvm.org/llvm.sh @@ -186,14 +161,10 @@ jobs: sudo update-alternatives --install /usr/bin/clang++ clang++ /usr/bin/clang++-18 100 - name: Install abicheck - run: pip install 'abicheck==0.6.*' + run: pip install -r .github/scripts/abi-requirements.txt - # The newest main run that still has a downloadable artifact. This is main's - # tip rather than the PR's true merge-base: a PR far behind main will also - # see main's own changes, which is noisy but never a false "compatible". - # - # A missing baseline is a hard failure: a green check that silently skipped - # the comparison is worse than a red one. + # Baseline is main's tip, not the PR's merge-base: a PR far behind main also + # sees main's own changes, which is noisy but never a false "compatible". - name: Download baseline build from main id: baseline env: @@ -204,9 +175,8 @@ jobs: # valid tarball, and a run that produced none just fails to download and # falls through to the next candidate. # - # The REST endpoint rather than `gh run list`: the latter was observed - # returning stale pages (all candidates months-expired, newest run - # missing), which burns every attempt and fails the leg spuriously. + # REST rather than `gh run list`, which was observed returning stale pages + # that omitted the newest run and burned every candidate on expired ones. candidates=$(gh api \ "repos/$GITHUB_REPOSITORY/actions/workflows/build-cpp-runtime-bindings.yml/runs?branch=main&event=push&per_page=20" \ --jq '.workflow_runs[] | "\(.id) \(.head_sha)"') @@ -231,6 +201,7 @@ jobs: fi echo "label=$label" >> "$GITHUB_OUTPUT" + # Default variant only; -public-only builds from the same headers. - name: Download this build uses: actions/download-artifact@v8 with: @@ -244,7 +215,6 @@ jobs: SUPPRESSIONS: .github/abi-suppressions.yml POLICY: strict_abi run: | - chmod +x .github/scripts/abi-check.sh label='${{ github.event.pull_request.number && format('PR {0}', github.event.pull_request.number) || github.ref_name }}' rc=0 .github/scripts/abi-check.sh \ @@ -258,10 +228,9 @@ jobs: } >> "$GITHUB_STEP_SUMMARY" fi - # 77 means an input tarball was missing; that is a harness/plumbing - # problem and must fail loudly rather than pass as "no findings". - if [ "$rc" -eq 77 ] || [ "$rc" -eq 64 ]; then - echo "::error::ABI check could not run (rc=$rc)." - exit 1 - fi - exit "$rc" + # Only 2/4/5 are ABI findings. Any other non-zero code means the comparison + # never happened, which must fail as plumbing rather than read as a break. + case "$rc" in + 0 | 2 | 4 | 5) exit "$rc" ;; + *) echo "::error::ABI check could not run (rc=$rc)."; exit 1 ;; + esac From b24bd950863a66591a5913cf7bedce2e42cb3e19 Mon Sep 17 00:00:00 2001 From: ethanglaser Date: Tue, 22 Sep 2026 10:58:02 -0700 Subject: [PATCH 12/15] add licnese to requirements --- .github/scripts/abi-requirements.txt | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/.github/scripts/abi-requirements.txt b/.github/scripts/abi-requirements.txt index 4d61b6d0d..918730c0f 100644 --- a/.github/scripts/abi-requirements.txt +++ b/.github/scripts/abi-requirements.txt @@ -1,3 +1,15 @@ -# Pinned so an abicheck release cannot turn a green ABI check red, or vice versa, -# without a reviewed commit. .abicheck.yml is written against this version's schema. +# Copyright 2026 Intel Corporation +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + abicheck==0.6.0 From 91088d5bbe2675fcfaac1f905ae068569c660354 Mon Sep 17 00:00:00 2001 From: ethanglaser Date: Tue, 22 Sep 2026 12:32:19 -0700 Subject: [PATCH 13/15] clean up supressions and future comment --- .github/abi-suppressions.yml | 21 ++++++++++++------- .github/workflows/build-c-api-bindings.yml | 3 +++ .../workflows/build-cpp-runtime-bindings.yml | 3 +++ 3 files changed, 19 insertions(+), 8 deletions(-) diff --git a/.github/abi-suppressions.yml b/.github/abi-suppressions.yml index 502c9fd1f..9e2eb16bf 100644 --- a/.github/abi-suppressions.yml +++ b/.github/abi-suppressions.yml @@ -15,10 +15,10 @@ # ABI check suppression rules for the SVS C++ runtime bindings. # # Measured on libsvs_runtime.so as this repo's CI builds it: of 138 exported -# symbols, 35 are leaked libstdc++ template instantiations, which is what the rules -# below cover. Another 31 are std::once_flag guard thunks for local statics inside -# svs::index::ivf instantiations; they mangle as _ZZ..., so `^_ZNSt` deliberately -# does not reach them -- they are svs code despite the std:: in the name. +# symbols the rules below suppress 66 leaked libstdc++ instantiations, leaving 72 -- +# 56 svs::runtime::v0 functions/objects and 16 svs vtable/RTTI entries. Every one of +# the 72 names an svs type, so the compared surface is SVS's own symbols and nothing +# else. Re-verify with nm before trusting these counts after a toolchain change. # # The leak is not a missing -fvisibility=hidden (that is set in # bindings/cpp/CMakeLists.txt); it tracks the build path. Linking the prebuilt LTO @@ -42,12 +42,17 @@ suppressions: reason: "RTTI/vtable/VTT for std:: types (e.g. thread::_State_impl, future)" label: "stdlib" - # 2 of these are exported (verified with nm), so this rule is load-bearing even - # on runs where it matches nothing. - - symbol_pattern: "^_ZT[VIST].*__gnu_cxx.*" - reason: "RTTI/vtable for __gnu_cxx types, leaked" + # Emitted for the local statics that std::call_once creates. Mangled in function + # scope as _ZZ..., which none of the rules above reach: they are libstdc++ + # entities instantiated over svs types, not svs code. + - symbol_pattern: "^_ZZNSt9once_flag18_Prepare_execution.*" + reason: "std::call_once local-static thunks (leaked template instantiations)" label: "stdlib" + # No rule for __gnu_cxx RTTI. The obvious `^_ZT[VIST].*__gnu_cxx.*` is unanchored, + # so it matches std:: types that merely take __gnu_cxx::_Lock_policy as a template + # argument -- already covered above. nm finds no RTTI for an actual __gnu_cxx type. + # # No rules for third-party (fmt/spdlog/tsl/toml++) or __cxa_*: the vendored deps # ship as static archives that -Wl,--exclude-libs,ALL keeps out, and __cxa_* are # imports, never defined here. libstdc++ is a shared library, hence the std:: rules. diff --git a/.github/workflows/build-c-api-bindings.yml b/.github/workflows/build-c-api-bindings.yml index 7c4653099..50037476d 100644 --- a/.github/workflows/build-c-api-bindings.yml +++ b/.github/workflows/build-c-api-bindings.yml @@ -139,6 +139,9 @@ jobs: # The hosted image's clang-13/14 degrades abicheck's AST frontend to ELF-tier # and drops constrained-template diffs, so install a C++20-capable clang. + # + # TODO: drop this step after either bumping to ubuntu-24.04 (which preinstalls clang-18) + # or run the compare inside svs-manylinux228. Broader change outside this PR scope. - name: Install clang for the AST frontend run: | wget -qO /tmp/llvm.sh https://apt.llvm.org/llvm.sh diff --git a/.github/workflows/build-cpp-runtime-bindings.yml b/.github/workflows/build-cpp-runtime-bindings.yml index fbf2339c2..007213502 100644 --- a/.github/workflows/build-cpp-runtime-bindings.yml +++ b/.github/workflows/build-cpp-runtime-bindings.yml @@ -151,6 +151,9 @@ jobs: # The hosted image's clang-13/14 degrades abicheck's AST frontend to ELF-tier # and drops constrained-template diffs, so install a C++20-capable clang. + # + # TODO: drop this step after either bumping to ubuntu-24.04 (which preinstalls clang-18) + # or run the compare inside svs-manylinux228. Broader change outside this PR scope. - name: Install clang for the AST frontend run: | wget -qO /tmp/llvm.sh https://apt.llvm.org/llvm.sh From b3b55c39efd97a04c1d0e25dd91015495bc6752a Mon Sep 17 00:00:00 2001 From: ethanglaser Date: Wed, 23 Sep 2026 12:51:36 -0700 Subject: [PATCH 14/15] address comments about comments and suppressions --- .github/abi-suppressions-c-api.yml | 27 ----------- .github/abi-suppressions.yml | 54 +++++++--------------- .github/dependabot.yml | 2 - .github/scripts/abi-check.sh | 9 ++-- .github/workflows/build-c-api-bindings.yml | 3 +- 5 files changed, 22 insertions(+), 73 deletions(-) delete mode 100644 .github/abi-suppressions-c-api.yml diff --git a/.github/abi-suppressions-c-api.yml b/.github/abi-suppressions-c-api.yml deleted file mode 100644 index 1bcbb60f5..000000000 --- a/.github/abi-suppressions-c-api.yml +++ /dev/null @@ -1,27 +0,0 @@ -# Copyright 2026 Intel Corporation -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. - -# ABI check suppression rules for libsvs_c_api.so. -# -# Intentionally empty, and expected to stay that way: the C API is compiled with -# -fvisibility=hidden and exports only `svs_*`, so there are no mangled names, no -# template instantiations from header-only dependencies, and no std:: internals to -# waive -- unlike the C++ libraries, whose exported set leaks all three. -# -# The file exists rather than being omitted so the --suppress path the workflows -# pass is a real one. abi-check.sh skips a missing file silently, which would make -# a typo in that path indistinguishable from this deliberate emptiness. - -version: 1 -suppressions: [] diff --git a/.github/abi-suppressions.yml b/.github/abi-suppressions.yml index 9e2eb16bf..48d79a8c5 100644 --- a/.github/abi-suppressions.yml +++ b/.github/abi-suppressions.yml @@ -14,21 +14,16 @@ # ABI check suppression rules for the SVS C++ runtime bindings. # -# Measured on libsvs_runtime.so as this repo's CI builds it: of 138 exported -# symbols the rules below suppress 66 leaked libstdc++ instantiations, leaving 72 -- -# 56 svs::runtime::v0 functions/objects and 16 svs vtable/RTTI entries. Every one of -# the 72 names an svs type, so the compared surface is SVS's own symbols and nothing -# else. Re-verify with nm before trusting these counts after a toolchain change. +# Every rule below drops libstdc++ template instantiations that leak into +# libsvs_runtime.so's exports -- 66 of 138 symbols. All 72 survivors name an svs type, +# so the compared surface is SVS's own. Re-check with nm after a toolchain bump. # -# The leak is not a missing -fvisibility=hidden (that is set in -# bindings/cpp/CMakeLists.txt); it tracks the build path. Linking the prebuilt LTO -# archive exports 138 symbols, compiling svs in-tree exports 276, with the same -# compiler either way. No pattern here can close that gap; a linker version script -# defining the intended surface would. +# The leak is not a missing -fvisibility=hidden (bindings/cpp/CMakeLists.txt sets it); +# it tracks the build path -- linking the prebuilt LTO archive exports 138 symbols, +# compiling svs in-tree exports 276. Only a linker version script closes that gap. version: 1 suppressions: - # --- C++ standard library / runtime (never part of the SVS API) --- - symbol_pattern: "^_ZNSt.*" reason: "C++ standard library symbols (leaked template instantiations)" label: "stdlib" @@ -37,36 +32,19 @@ suppressions: reason: "C++ standard library functions" label: "stdlib" - # RTTI/vtable/VTT for std types mangle as _ZTI/_ZTS/_ZTV/_ZTT, not _ZNSt. - symbol_pattern: "^_ZT[VIST]N?St.*" - reason: "RTTI/vtable/VTT for std:: types (e.g. thread::_State_impl, future)" + reason: "RTTI/vtable/VTT for std:: types (thread::_State_impl, future); mangles _ZT[VIST], not _ZNSt" label: "stdlib" - # Emitted for the local statics that std::call_once creates. Mangled in function - # scope as _ZZ..., which none of the rules above reach: they are libstdc++ - # entities instantiated over svs types, not svs code. - symbol_pattern: "^_ZZNSt9once_flag18_Prepare_execution.*" - reason: "std::call_once local-static thunks (leaked template instantiations)" + reason: "std::call_once local statics; _ZZ-mangled in function scope, libstdc++ entities instantiated over svs types" label: "stdlib" - # No rule for __gnu_cxx RTTI. The obvious `^_ZT[VIST].*__gnu_cxx.*` is unanchored, - # so it matches std:: types that merely take __gnu_cxx::_Lock_policy as a template - # argument -- already covered above. nm finds no RTTI for an actual __gnu_cxx type. - # - # No rules for third-party (fmt/spdlog/tsl/toml++) or __cxa_*: the vendored deps - # ship as static archives that -Wl,--exclude-libs,ALL keeps out, and __cxa_* are - # imports, never defined here. libstdc++ is a shared library, hence the std:: rules. - # - # abicheck lists rules that matched nothing under "Stale rules", which means "no - # change touched it this run", not "delete it". - - # --- Waivers --- - # None. To waive a real finding, add a finding_id rule (id copied from the report) - # with a reason and an `expires`. finding_id matches one finding kind on one symbol - # but does not pin the old->new values, so a later unrelated change to the same - # symbol would be waived silently too -- hence the expiry. - - # --- svs::runtime::v0 and svs::...detail...: deliberately NOT suppressed --- - # The 55 declared v0 functions are the whole point of the check. `detail` symbols - # are not suppressed either: users reach them transitively through template - # instantiation, so a vtable or layout change there is a real break. +# No __gnu_cxx RTTI rule: the obvious `^_ZT[VIST].*__gnu_cxx.*` is unanchored, so it +# only matches std:: types taking __gnu_cxx::_Lock_policy as a template argument, which +# the rules above already cover. Nothing for fmt/spdlog/tsl/toml++ or __cxa_*: the +# vendored deps ship as static archives that -Wl,--exclude-libs,ALL keeps out, and +# __cxa_* are imports, never defined here. +# +# svs::...detail... is deliberately not suppressed: users reach it through template +# instantiation, so a vtable or layout change there is a real break. diff --git a/.github/dependabot.yml b/.github/dependabot.yml index 893e8b2ae..0eded803f 100644 --- a/.github/dependabot.yml +++ b/.github/dependabot.yml @@ -28,8 +28,6 @@ updates: patterns: - "*" - # A bump PR runs the ABI check under the new abicheck, so tool-level behaviour - # changes show up as a diff in the report rather than as a surprise on main. - package-ecosystem: "pip" directory: "/.github/scripts" schedule: diff --git a/.github/scripts/abi-check.sh b/.github/scripts/abi-check.sh index 37fb4df5e..3df295d8e 100755 --- a/.github/scripts/abi-check.sh +++ b/.github/scripts/abi-check.sh @@ -24,9 +24,8 @@ # Overridable via environment: # LIBRARY library basename to compare (default libsvs_runtime.so) # HEADER_SUBDIR header root inside the tarball (default include/svs/runtime) -# SUPPRESSIONS suppression file (default .github/abi-suppressions.yml); a -# missing file is skipped silently, so callers that suppress -# nothing should still point at a real empty one +# SUPPRESSIONS suppression file (default unset, i.e. suppress nothing); a +# path that does not exist is an error, not an empty rule set # POLICY abicheck policy (default strict_abi) # DEPTH abicheck --depth (default unset, i.e. abicheck's 'headers'); # 'binary' is the escape hatch for a header set too large to parse @@ -46,7 +45,7 @@ NEW_TARBALL="${4:?missing new tarball}" LIBRARY="${LIBRARY:-libsvs_runtime.so}" HEADER_SUBDIR="${HEADER_SUBDIR:-include/svs/runtime}" -SUPPRESSIONS="${SUPPRESSIONS:-.github/abi-suppressions.yml}" +SUPPRESSIONS="${SUPPRESSIONS-}" POLICY="${POLICY:-strict_abi}" DEPTH="${DEPTH:-}" REPORT="${REPORT:-abi-report.md}" @@ -88,7 +87,7 @@ NEW_LIB=$(unpack "$NEW_TARBALL" "$NEW_DIR" "$NEW_LABEL") || exit 77 echo "Comparing $LIBRARY: [$OLD_LABEL] -> [$NEW_LABEL]" suppress_args=() -[ -f "$SUPPRESSIONS" ] && suppress_args=(--suppress "$SUPPRESSIONS") +[ -n "$SUPPRESSIONS" ] && suppress_args=(--suppress "$SUPPRESSIONS") # Left unset by default so .abicheck.yml stays the single place that sets depth. depth_args=() diff --git a/.github/workflows/build-c-api-bindings.yml b/.github/workflows/build-c-api-bindings.yml index 50037476d..827181168 100644 --- a/.github/workflows/build-c-api-bindings.yml +++ b/.github/workflows/build-c-api-bindings.yml @@ -199,10 +199,11 @@ jobs: path: current - name: Compare + # Currently no SUPPRESSIONS necessary for C API (-fvisibility=hidden leaves only svs_* exported) + # If that changes, add another suppressions file for C API with similar structure to .github/abi-suppressions.yml env: LIBRARY: libsvs_c_api.so HEADER_SUBDIR: include/svs/c - SUPPRESSIONS: .github/abi-suppressions-c-api.yml POLICY: strict_abi run: | label='${{ github.event.pull_request.number && format('PR {0}', github.event.pull_request.number) || github.ref_name }}' From ad58393dd14b4b30e0b34cbf0ace7403f7ba304a Mon Sep 17 00:00:00 2001 From: ethanglaser Date: Thu, 24 Sep 2026 19:56:41 -0700 Subject: [PATCH 15/15] simplify comment --- .github/workflows/build-c-api-bindings.yml | 3 +-- .github/workflows/build-cpp-runtime-bindings.yml | 3 +-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/.github/workflows/build-c-api-bindings.yml b/.github/workflows/build-c-api-bindings.yml index 827181168..c77bc2a9e 100644 --- a/.github/workflows/build-c-api-bindings.yml +++ b/.github/workflows/build-c-api-bindings.yml @@ -130,8 +130,7 @@ jobs: permissions: contents: read actions: read # list runs and download artifacts, to fetch main's as baseline - # The label downgrades this job to informational rather than skipping it, so an - # intentional break is still reported rather than going unmeasured. + # Run informative, green CI check (non-breaking) if ABI breaking label added to PR continue-on-error: ${{ contains(toJson(github.event.pull_request.labels.*.name), '"API/ABI breaking change"') }} steps: diff --git a/.github/workflows/build-cpp-runtime-bindings.yml b/.github/workflows/build-cpp-runtime-bindings.yml index 007213502..e398c0982 100644 --- a/.github/workflows/build-cpp-runtime-bindings.yml +++ b/.github/workflows/build-cpp-runtime-bindings.yml @@ -142,8 +142,7 @@ jobs: permissions: contents: read actions: read # list runs and download artifacts, to fetch main's as baseline - # The label downgrades this job to informational rather than skipping it, so an - # intentional break is still reported rather than going unmeasured. + # Run informative, green CI check (non-breaking) if ABI breaking label added to PR continue-on-error: ${{ contains(toJson(github.event.pull_request.labels.*.name), '"API/ABI breaking change"') }} steps: