Add vcpkg-release-bump workflow and bump in-repo overlay port to v3.10.161.1#1475
Open
bmehta001 wants to merge 19 commits into
Open
Add vcpkg-release-bump workflow and bump in-repo overlay port to v3.10.161.1#1475bmehta001 wants to merge 19 commits into
bmehta001 wants to merge 19 commits into
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>
…LAGS dupes) Copilot review: the non-vcpkg REL_FLAGS already injected -ffunction-sections (and -fdata-sections for GNU), duplicating the new global block on Release builds. Remove them from REL_FLAGS so the global add_compile_options block is the single source of truth for section splitting across both dependency modes (it now also covers MSVC /Gy /Gw and AppleClang, which CI confirms build clean). No behavior change; eliminates redundant flags and future drift. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The SDK uses SQLite only for offline event storage (plain tables/indexes,
no JSON/FTS/RTREE/vtab), so the port now declares sqlite3 with
default-features:false instead of pulling json1. This is the necessary
floor for footprint-conscious consumers: vcpkg unions features across the
dependency graph and ignores default-features:false on transitive deps, so
without this the SDK's own edge forces json1 on and no consumer can opt out.
Measured: a consumer that also sets {"name":"sqlite3","default-features":false}
in its root manifest links ~52 KB smaller (SQLITE_OMIT_JSON) on
x64-windows-static Release; the vcpkg integration test stays 10/10.
- tools/ports/cpp-client-telemetry/vcpkg.json: sqlite3 default-features:false;
port-version 1 (port-only change over the published 3.10.161.1#0)
- docs/building-with-vcpkg.md: document the required root-manifest opt-out
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…pt-out Address Copilot round comments on docs/building-with-vcpkg.md and document the footprint lever consumers actually need (validated end-to-end on a real downstream DLL, which shrank substantially once /OPT:REF,ICF were on). - Add "Enable linker dead-stripping" section: the SDK's /Gy /Gw only enable stripping; the consumer's link must set /OPT:REF + /OPT:ICF (with the /DEBUG gotcha that silently disables them) + /INCREMENTAL:NO, or --gc-sections / -dead_strip on GNU/Clang/Apple. Note static-link vs DLL-reexport caveat. - json1 section: clarify this is the in-repo OVERLAY port (registry port to follow upstream), addressing the "registry still pulls defaults" comment. - Reword the resolution explanation around vcpkg's union model instead of "ignores transitive default-features:false". Verified by dry-run: SDK edge opt-out alone keeps sqlite3[core,json1]; adding the same at the root yields sqlite3 (no json1); any edge requesting defaults restores json1 -- so the consumer must opt out at the root too. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The in-repo overlay is only used for local testing (overlays ignore the version database), and the vcpkg-release-bump workflow deletes port-version on every bump, so it served no purpose here. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Address Copilot comment on the linker-stripping guidance. Per the MSVC /OPT
docs, /DEBUG changes the /OPT default from REF/ICF to NOREF/NOICF (it does
disable them by default, contrary to the comment's premise). Reword to the
exact behavior ("flips their default to off, /OPT:NOREF,NOICF") and note that
/OPT:REF is also incompatible with incremental linking (hence /INCREMENTAL:NO).
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Address Copilot comment: reword the AppleClang branch to state the actual mechanism -- clang emits .subsections_via_symbols on Mach-O, so ld64's -dead_strip removes unreferenced code at per-symbol (function) granularity without -ffunction-sections (which we add only for cross-toolchain consistency). -fdata-sections stays omitted due to the historical bitcode conflict. No behavior change. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add -fvisibility=hidden -fvisibility-inlines-hidden for non-MSVC builds and make
MATSDK_LIBABI = __attribute__((visibility("default"))) on GCC/Clang, so only the
MATSDK_LIBABI-decorated public API (the 56 public classes + the EVTSDK_LIBABI C
API in mat.h) is exported; SDK internals and the bundled sqlite3/zlib are hidden.
Non-Windows analog of the __declspec(dllexport)-gated export on Windows and of
/Gy + the consumer's /OPT:REF: a much smaller dynamic symbol table -> faster
dynamic linking/loading, smaller shared binaries, and more inlining/dead-code
elimination. No behavior change for static consumers (hidden symbols remain
usable within the same link); for shared-lib builds it restricts exports to the
public API.
Validated (NDK aarch64): compiling EventProperties.cpp with the flag yields the
decorated public methods (EventProperties::SetType/GetType) as GLOBAL DEFAULT
while internals (EventPropertiesStorage) are WEAK HIDDEN -- 64 exported vs 300
hidden in that one TU. Full cross-platform validation (build a shared lib and
link a separate consumer on Linux/macOS/iOS/Android to confirm no public symbol
is missing) should run in CI before merge.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ow_dispatch input - CMakeLists.txt: if(MSVC) is also true for the ClangCL toolset, which supports /Gy but not /Gw. Apply /Gy for all MSVC-like compilers and gate /Gw to real cl.exe (CMAKE_CXX_COMPILER_ID STREQUAL "MSVC") so ClangCL builds don't choke on /Gw. - vcpkg-release-bump.yml: read the workflow_dispatch tag via github.event.inputs.tag (concurrency key + resolve step) so manual dispatches resolve the tag unambiguously. 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 two vcpkg-port maintenance changes:
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.When it runs
vMAJOR.MINOR.PATCH.BUILD).workflow_dispatchwith ataginput (recovery / re-runs).What it does: resolves the tag/version, computes the source SHA512, branches off vcpkg
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 still reviews/merges it.One-time setup required (repo settings):
VCPKG_FORK_REPO→ the vcpkg fork to push branches to (e.g.your-org/vcpkg).VCPKG_BUMP_TOKEN→ a PAT (classic:repo+workflow, or fine-grained: Contents + Pull requests RW on the fork) able to push to the fork and open PRs on microsoft/vcpkg.Until the port is merged into the registry (PR microsoft/vcpkg#52316), the workflow fails fast with a clear message that the port must exist first.
2. Bump the in-repo overlay port to the
v3.10.161.1tagtools/ports/cpp-client-telemetry/portfile.cmakepreviously pointed at the pre-release commit4485b820. RepointedREF→ the publishedv3.10.161.1tag (SHA512 updated) for exact parity with the official microsoft/vcpkg port. (The version was already3.10.161.1from the release-prep.)Also in this PR (function-level linking):
CMakeLists.txtnow emits/Gy /Gw(MSVC) /-ffunction-sections -fdata-sections(GCC/Clang) in both vendored and vcpkg modes. Previously the entire flags block was gated onNOT MATSDK_USE_VCPKG_DEPSand the MSVC branch 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+/OPT:ICF/--gc-sections/-dead_strip), matching the MSBuild Release projects. No source/ABI change.