vcpkg port: release-bump workflow, overlay port bump, and consumer binary-footprint reductions#1475
Merged
bmehta001 merged 30 commits intoJun 23, 2026
Conversation
On a published version release, open a PR to microsoft/vcpkg bumping the cpp-client-telemetry port (REF -> tag, recomputed SHA512, version, then x-add-version). Runs only on published, non-prerelease version tags (vX.Y.Z.W) or manual dispatch, and opens no PR when the port already matches the release. Requires repo variable VCPKG_FORK_REPO and secret VCPKG_BUMP_TOKEN. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Repoint tools/ports/cpp-client-telemetry REF from the pre-release commit to the published v3.10.161.1 tag (SHA512 updated) for exact parity with the official microsoft/vcpkg port. Version was already 3.10.161.1. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
bmehta001
changed the title
Contributor
There was a problem hiding this comment.
Pull request overview
This pull request adds automation to keep the upstream microsoft/vcpkg port for cpp-client-telemetry in sync with SDK releases, and aligns the in-repo overlay port’s REF with the published v3.10.161.1 tag.
Changes:
- Add a new GitHub Actions workflow to open/update a vcpkg port bump PR when a release is published (or via manual dispatch).
- Update the in-repo overlay port (
tools/ports/.../portfile.cmake) to useREF v3.10.161.1and its corresponding SHA512.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
tools/ports/cpp-client-telemetry/portfile.cmake |
Repoint overlay port source REF/SHA512 to the published v3.10.161.1 tag for parity with the released SDK. |
.github/workflows/vcpkg-release-bump.yml |
New workflow that computes the release tarball SHA512, updates the vcpkg port files, runs vcpkg formatting/version DB update, and opens/refreshes a PR in microsoft/vcpkg. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…alls building-with-vcpkg.md only told manifest-mode users to add `cpp-client-telemetry` to vcpkg.json, which fails with an unknown-port error until the port is accepted into the official vcpkg registry. Document the `vcpkg-configuration.json` `overlay-ports` fallback that points manifest mode at the in-repo overlay port, giving parity with the classic-mode `--overlay-ports` instructions already in the doc. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
.github/workflows/vcpkg-release-bump.yml (Copilot): `git push
--force-with-lease` could fail on reruns because a fresh clone has no
remote-tracking ref for an already-existing bump branch, so the workflow
couldn't refresh an open bump PR (contradicting the "force-pushed branch
refreshes it" intent). Fetch the branch into refs/remotes/origin/${BR}
(|| true on the first run, when it doesn't exist yet) before the
force-with-lease push so the lease has a ref to compare against.
Verified at .github/workflows/vcpkg-release-bump.yml:146-152.
docs/building-with-vcpkg.md: remove the manifest-mode overlay-ports note
added in 136e010, per maintainer request.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The cpp-client-telemetry port was merged into the upstream vcpkg registry (microsoft/vcpkg#52316, version 3.10.161.1), so docs/building-with-vcpkg.md no longer needs the conditional "once the port is accepted" phrasing. - Intro: state the port is published in the official registry and consumable directly; drop the stale "build recipe / CONTROL file" wording (vcpkg uses vcpkg.json, and the port is registry-resolved now). - "Installing from the vcpkg registry": present tense, link to the upstream ports/cpp-client-telemetry directory. - "Installing from the overlay port": reframe as development-only (test local port changes or a newer SDK revision before they reach the registry). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The cpp-client-telemetry port merged into microsoft/vcpkg (ports/cpp-client-telemetry) ships only portfile.cmake + vcpkg.json. Bring the in-repo overlay back in sync so testing the overlay validates exactly what is published. - Drop the custom 'usage' file and its install step in portfile.cmake. The two lines it printed (find_package(MSTelemetry CONFIG REQUIRED) + target_link_libraries ... MSTelemetry::mat) duplicate vcpkg's auto-generated heuristic usage, and the upstream port carries no usage file. - Reorder vcpkg.json dependencies to vcpkg format-manifest canonical (alphabetical) order; same dependency set, no resolution change. After this, the overlay portfile.cmake and vcpkg.json are byte-identical to the upstream port blobs (cfdab23 / a74cc08). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot review (round): the open-PR existence guard used
--jq '.[0].number'. On the first run, when no PR exists yet, gh pr list
returns [] and .[0].number evaluates to null, which gh prints as the literal
string "null". [ -n "null" ] is true, so the workflow wrongly logged "An open
PR already exists" and skipped 'gh pr create' -- the release-bump PR would
never be opened on a clean run.
Fix: --jq '.[0].number // empty' yields empty output when no PR exists (guard
false -> PR created) and the PR number when one does (guard true -> skipped).
Verified jq semantics (jq 1.x): '[] | .[0].number' -> null (prints "null");
'[] | .[0].number // empty' -> no output; '[{number:42}] | .[0].number // empty'
-> 42. Confirmed at .github/workflows/vcpkg-release-bump.yml:158.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ken in URL Address Copilot round comments on the release-bump workflow. vcpkg-release-bump.yml:77 - non-version tag handling was contradictory: the message said "skipping" but the step ran exit 1, failing the workflow. A release published with a non-4-part tag (the SDK has historical 3-part tags like v3.3.8) would mark the automatic run red for what should be a no-op. Now: manual workflow_dispatch with a bad tag still fails loudly (user error), but the automatic release trigger emits a notice, sets a 'skip' output, and exits 0. All downstream steps are gated on steps.ver.outputs.skip != 'true'. vcpkg-release-bump.yml:100 - the PAT was embedded in the clone URL, which persists it in .git/config and risks leaking if git echoes the remote. Switch to 'gh auth setup-git' (writes a credential helper to the global gitconfig) plus a tokenless https clone; the later push step reuses that helper via its GH_TOKEN env. No token appears in any URL or on disk. Validated: workflow YAML parses (PyYAML) and every embedded run block passes 'bash -n'. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Two review fixes: - CMakeLists.txt: the hidden-visibility block is described as non-Windows but was gated on NOT MSVC, which also matches MinGW/Clang-GNU Windows and would apply ELF-style -fvisibility=hidden to a PE/COFF target. Gate it on NOT WIN32 so all Windows toolchains rely on __declspec(dllexport) as intended. - docs/sharing-a-single-sdk-runtime.md: the C-API bullet said it 'links without __declspec(dllimport)', which contradicted the new MATSDK_IMPORT_LIB interface define. Reworded to: dllimport is not required (a C function resolves via the import-lib thunk) but is applied automatically to shared consumers as a harmless optimization. Files changed: - CMakeLists.txt: NOT MSVC -> NOT WIN32 for the visibility block - docs/sharing-a-single-sdk-runtime.md: C-API dllimport wording Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ow' into bhamehta/vcpkg-release-bump-workflow
| # elimination -- the non-Windows analog of what /Gy plus the consumer's /OPT:REF | ||
| # achieve on MSVC. Windows already restricts exports via __declspec(dllexport) | ||
| # on MATSDK_LIBABI (lib/include/public/ctmacros.hpp). | ||
| if(NOT MSVC) |
Contributor
Author
Comment on lines
+38
to
+40
| * **It links without `__declspec(dllimport)`.** A plain C function resolves | ||
| through the shared library's import lib, so the C API works across the boundary | ||
| today. |
Contributor
Author
There was a problem hiding this comment.
Comment on lines
+197
to
+198
| The SDK links statically into your binary, so most footprint control lives on | ||
| *your* side of the link. |
Contributor
Author
There was a problem hiding this comment.
Fixed in 7e652a9. Scoped the 'Reducing binary footprint' section to the static-link case (the *-static triplets) and added a note that a dynamic mat (e.g. the default x64-windows triplet or BUILD_SHARED_LIBS=ON) ships its own runtime whose export table is already trimmed by the SDK's -fvisibility=hidden and /Gy /Gw -- so the consumer-side dead-strip options below are specific to static linkage.
The 'Reducing binary footprint' section assumed the SDK is always linked statically, but vcpkg's default triplets (e.g. x64-windows) build dynamic libraries and the port also supports BUILD_SHARED_LIBS=ON. Clarify that the consumer-side dead-stripping guidance applies to static linkage, and note that a dynamic mat ships its own runtime whose export table is already trimmed by the SDK's -fvisibility=hidden and /Gy /Gw. Files changed: - docs/building-with-vcpkg.md: scope footprint section to static-link case Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Comment on lines
+55
to
+63
| #ifndef MATSDK_LIBABI | ||
| // Mark the public API as default-visibility so it stays exported when the SDK is | ||
| // built with -fvisibility=hidden (see CMakeLists.txt). This is the non-Windows | ||
| // analog of the __declspec(dllexport) gating above. Harmless without the flag. | ||
| # if defined(__GNUC__) || defined(__clang__) | ||
| # define MATSDK_LIBABI __attribute__((visibility("default"))) | ||
| # else | ||
| # define MATSDK_LIBABI | ||
| # endif |
Contributor
Author
There was a problem hiding this comment.
Fixed in 94434f7. Gated the non-Windows visibility("default") attribute on MATSDK_SHARED_LIB so it mirrors the Windows dllexport gating: static builds now omit the attribute and the public API inherits -fvisibility=hidden. Defined MATSDK_SHARED_LIB PRIVATE for all shared builds in lib/CMakeLists.txt. Verified on Linux with readelf -- static: evt_api_call_default is GLOBAL HIDDEN (still resolvable by static linking; a consumer links+runs against libmat.a, but it is no longer re-exported when absorbed into a consumer .so); shared: GLOBAL DEFAULT (exported from libmat.so).
On non-Windows, MATSDK_LIBABI unconditionally expanded to
__attribute__((visibility("default"))), so even in a static build the
public API kept default visibility despite -fvisibility=hidden. A consumer
that statically absorbs libmat.a into its own .so/.dylib would then
unintentionally re-export the SDK's public API (larger dynamic symbol
table, leaked SDK surface) -- the non-Windows analog of the Windows
re-export that MATSDK_STATIC_LIB already prevents.
Gate the visibility attribute on MATSDK_SHARED_LIB so it mirrors the
__declspec(dllexport) gating: shared builds export the API; static builds
omit the attribute, letting the public symbols inherit -fvisibility=hidden.
Define MATSDK_SHARED_LIB PRIVATE for all shared builds in lib/CMakeLists.txt
(keeping the Windows-only INTERFACE MATSDK_IMPORT_LIB and MATSDK_STATIC_LIB).
Verified on Linux (readelf): static build -> evt_api_call_default is
GLOBAL HIDDEN (still resolvable by static linking -- a consumer links+runs
against libmat.a -- but not re-exported); shared build -> GLOBAL DEFAULT
(exported from libmat.so).
Files changed:
- lib/include/public/ctmacros.hpp: non-Windows MATSDK_LIBABI gated on MATSDK_SHARED_LIB
- lib/CMakeLists.txt: define MATSDK_SHARED_LIB for all shared builds
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
lkarra2
approved these changes
Jun 23, 2026
bmehta001
added a commit
to microsoft/onnxruntime
that referenced
this pull request
Jun 24, 2026
…DS SDK When ONNX Runtime statically links the 1DS telemetry SDK (cpp-client-telemetry), the SDK uses SQLite only for plain offline event storage and never needs the json1 extension. Request sqlite3 with default-features:false in the telemetry feature so json1 (SQLITE_OMIT_JSON, ~50 KB) is omitted from the build. vcpkg ignores a transitive default-features:false (the port's own edge), so the consumer must also opt out in its own manifest. This takes full effect once ONNX Runtime's vcpkg baseline includes the cpp-client-telemetry port revision that also opts out (microsoft/cpp_client_telemetry#1475); with the current baseline it resolves cleanly and is a no-op (json1 stays). Verified with 'vcpkg install --dry-run' that json1 is dropped when both edges opt out, and that sqlite3 is only pulled when the telemetry feature is enabled. The other static-link footprint levers are already in place in ONNX Runtime: -ffunction-sections/-fdata-sections + -fvisibility=hidden at compile, and --gc-sections / -dead_strip / /OPT:REF,ICF at link, so the SDK's PR-1475 function-sectioning and hidden visibility let the existing dead-strip remove unreferenced SDK code. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
bmehta001
added a commit
to microsoft/onnxruntime
that referenced
this pull request
Jun 24, 2026
…build) 3.10.173.1 carries the SDK's binary-footprint / compilation improvements (microsoft/cpp_client_telemetry#1475): function-level linking + hidden visibility, which let ONNX Runtime's existing dead-strip (--gc-sections / -dead_strip / /OPT:REF,ICF) discard unreferenced SDK code, plus the sqlite3 json1 opt-out. - cmake/deps.txt: bump the FetchContent fallback to v3.10.173.1 (+ archive SHA1). - cgmanifests/cgmanifest.json: bump the tracked commit to the v3.10.173.1 tag. - cmake/vcpkg-ports/cpp-client-telemetry: add an overlay port pinned to v3.10.173.1 (mirrors the SDK's port, including sqlite3 default-features:false) so the vcpkg path resolves the new version now, before the registry merge. Removable once the vcpkg baseline includes 3.10.173.1. Combined with the sqlite3 json1 opt-out in cmake/vcpkg.json, verified via 'vcpkg install --dry-run' that the vcpkg path resolves cpp-client-telemetry@3.10.173.1 and sqlite3 with json1 dropped. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR bundles vcpkg-port maintenance with a set of consumer binary-footprint / symbol-visibility reductions for the library the port produces.
1.
.github/workflows/vcpkg-release-bump.yml(new)Automatically opens a version-bump PR against microsoft/vcpkg for the
cpp-client-telemetryport whenever a new SDK release is cut.vMAJOR.MINOR.PATCH.BUILD); manually viaworkflow_dispatchwith ataginput; never on ordinary pushes, and it opens no PR if the port already matches the release.masterin the configured fork, updatesports/cpp-client-telemetry/{portfile.cmake (REF + SHA512), vcpkg.json (version)}, runsformat-manifest+x-add-version, and opens a[cpp-client-telemetry] Update to <version>PR (the vcpkg team reviews/merges).VCPKG_FORK_REPOand secretVCPKG_BUMP_TOKEN. Until the port is merged into the registry ([cpp-client-telemetry] Add new port (version 3.10.161.1) vcpkg#52316), the workflow fails fast with a clear message.2. Bump the in-repo overlay port to the published release tag
tools/ports/cpp-client-telemetry/portfile.cmakepreviously pointed at a pre-release commit. RepointedREF→ the publishedv3.10.161.1tag (SHA512 updated) for exact parity with the official microsoft/vcpkg port.3. Consumer binary-footprint reductions
These shrink the SDK code a consumer links from the vcpkg-packaged library:
CMakeLists.txt): emit/Gy /Gw(MSVC) /-ffunction-sections -fdata-sections(GCC/Clang) in both vendored and vcpkg modes. Previously the whole flags block was gated toNOT MATSDK_USE_VCPKG_DEPSand MSVC never set/Gy, so the vcpkg-packaged library linked whole.objfiles into consumers. This lets a consumer's linker dead-strip unreferenced SDK code (/OPT:REF,ICF/--gc-sections/-dead_strip)./Gwis gated to realcl.exe(clang-cl rejects it). No source/ABI change.CMakeLists.txt+lib/include/public/ctmacros.hpp): build with-fvisibility=hidden -fvisibility-inlines-hiddenon non-MSVC, and makeMATSDK_LIBABIexpand to__attribute__((visibility("default")))on GCC/Clang so the public API stays exported while internal symbols are hidden — trimming the dynamic export table and enabling more aggressive optimization. Harmless without the flag (the non-Windows analog of the existing__declspec(dllexport)gating).tools/ports/cpp-client-telemetry/vcpkg.json):sqlite3withdefault-features: false(json1 is unused by the SDK), ~−52 KB. Note: vcpkg ignores transitivedefault-features:false, so a consumer that wants this saving must also opt out at its own root manifest — documented below.docs/building-with-vcpkg.md): new "Reducing binary footprint" section covering the consumer-side/OPT:REF,ICFlever and the json1 opt-out.No public API or ABI change (the visibility annotation does not alter Itanium mangling).