Skip to content

refactor: replace protobuf submodule with vcpkg#162

Open
Kybxd wants to merge 37 commits into
masterfrom
vcpkg
Open

refactor: replace protobuf submodule with vcpkg#162
Kybxd wants to merge 37 commits into
masterfrom
vcpkg

Conversation

@Kybxd
Copy link
Copy Markdown
Collaborator

@Kybxd Kybxd commented May 28, 2026

Summary

Drop the bundled third_party/_submodules/protobuf build pipeline and switch to vcpkg as the recommended way to provide protoc + libprotobuf. This removes a large per-machine bootstrap step (cloning + building protobuf from source via init.sh / init.bat) and lets developers consume a prebuilt protobuf toolchain through a single, cross-platform package manager. CI is migrated to the same model.

Motivation

  • The submodule build was slow on first checkout (full protobuf source compile) and fragile across compilers / CRT settings.
  • protobuf v22+ enforces a strict gencode/runtime version check, so protoc and libprotobuf must come from the same release. A package manager naturally guarantees this; a hand-rolled submodule does not.
  • vcpkg works on Linux / macOS / Windows with one set of instructions and produces a ready-to-use CMake config (find_package(Protobuf CONFIG)).

Changes

Build & toolchain

  • Remove third_party/_submodules/protobuf submodule and the init.sh / init.bat scripts that built it.
  • Update .gitmodules and .gitignore for the vcpkg manifest / install tree.
  • Rewrite prepare.bat to install Chocolatey, CMake, Ninja, MSVC build tools, buf, vcpkg, and protobuf:x64-windows-static; export VCPKG_ROOT and put the vcpkg-built protoc.exe on PATH for the current cmd session. Subsequent runs detect already-installed tools and skip them.
  • test/cpp-tableau-loader/CMakeLists.txt: drop the local-submodule lookup; rely on find_package(Protobuf CONFIG REQUIRED) and document the vcpkg / system-package / custom-prefix paths in comments.

CI

  • testing-cpp.yml: install protobuf via vcpkg manifest, matrix over modern (6.33.4) and legacy-v3 (3.21.12), cache vcpkg_installed/ with actions/cache@v4 keyed by triplet + vcpkg commit + vcpkg.json hash. Replaces the removed x-gha vcpkg binary-cache backend.
  • testing-csharp.yml: align matrix with cpp; use arduino/setup-protoc with the correct protoc tag mapping (x.y.zy.z).
  • testing-go.yml: align job naming with the others; protobuf version dimension dropped (Go uses google.golang.org/protobuf directly, host protoc version doesn't matter for the test).

Docs (README.md)

  • New "Install protobuf" section recommending vcpkg, with pointers to system-package / Homebrew / source alternatives.
  • Warn against dnf / yum protobuf-devel on RHEL-family distros (stuck on 3.5.x, predates the v22 / Abseil split).
  • Document the Windows static-CRT requirement and why -DCMAKE_BUILD_TYPE=Debug must match the protobuf you installed.
  • Clarify that -DCMAKE_TOOLCHAIN_FILE=... is the only flag strictly required when using vcpkg; -DVCPKG_TARGET_TRIPLET / -DCMAKE_PREFIX_PATH are only needed in specific scenarios.

Migration for existing checkouts

git submodule deinit -f third_party/_submodules/protobuf
rm -rf third_party/_submodules/protobuf .git/modules/third_party/_submodules/protobuf
# then install protobuf via vcpkg as documented in README.md

Test plan

  • Linux / macOS / Windows CI matrix green for both modern (protobuf 6.33.4) and legacy-v3 (protobuf 3.21.12).
  • Local cmake --build build && ctest on a fresh checkout where protobuf comes only from vcpkg (no submodule present).
  • Windows: prepare.bat from a clean machine installs everything and produces a working build with x64-windows-static.

Kybxd added 9 commits May 28, 2026 15:50
Switch from building the protobuf submodule via init.sh/init.bat to
installing protobuf through vcpkg. This removes the bundled protobuf
build, simplifying the setup process and significantly reducing initial
bootstrap time.

Update prepare.bat to install vcpkg and protobuf (x64-windows-static),
expose the vcpkg-built protoc on PATH, and export VCPKG_ROOT. Revise
README.md instructions to document vcpkg (recommended), system package,
Homebrew, and source-based protobuf installation paths. Adjust gitignore
rules for the vcpkg manifest and install tree accordingly.
@Kybxd Kybxd changed the title ci: migrate protobuf install to vcpkg in CI refactor: replace protobuf submodule with vcpkg May 28, 2026
wenchy and others added 20 commits May 29, 2026 21:27
- prepare.bat: switch to vcpkg manifest mode when PROTOBUF_VCPKG_VERSION
  is set. Classic-mode `--x-version` is silently a no-op, so the previous
  pin attempt would lie to users. Render a vcpkg.json under
  %LOCALAPPDATA%\loader\vcpkg-manifest\ with builtin-baseline + overrides,
  install via `vcpkg install --x-install-root=...`, and post-assert that
  the resolved port version matches the request. Pin the vcpkg checkout
  itself to VCPKG_BASELINE_COMMIT (mirroring testing-cpp.yml's VCPKG_COMMIT)
  so classic mode is also reproducible. Drop `--depth 1` so the commit pin
  is reachable. Export VCPKG_INSTALLED_DIR for downstream cmake.
- prepare.bat: add a `where cl.exe` preflight at the top of Step 5 so an
  unactivated MSVC environment fails fast with an actionable message
  instead of cryptic vcpkg compiler-detection errors.
- testing-cpp.yml: replace deprecated `version-string` with `version` in
  the rendered vcpkg.json.
- testing-csharp.yml: document inline that `protobuf-version` is the
  protoc release tag (e.g. 33.4) and explain how it maps to the C++
  libprotobuf SemVer used in testing-cpp.yml.
- README.md: add a "Migrating from the bundled-protobuf layout" callout
  with the submodule-deinit recipe so existing checkouts know how to
  clean up. Replace the misleading classic-mode `--x-version` snippet
  with a working manifest-mode example. Document the extra cmake flags
  (-DVCPKG_INSTALLED_DIR / -DVCPKG_MANIFEST_INSTALL=OFF) required when
  PROTOBUF_VCPKG_VERSION is used.
- .gitignore: ignore .claude/settings.local.json.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Captures the brainstormed design for adding a Dev Container under
.devcontainer/ to give contributors a one-command, reproducible
multi-language toolchain (C++17 + Go 1.24 + .NET 8 + Node 20 + buf
1.67.0 + protobuf 6.33.4 via vcpkg, all pinned to CI's exact versions).

Key decisions captured:
- Single all-in-one Ubuntu 24.04 image (covers all four languages)
- Dockerfile in repo, build on-demand (no ghcr.io publish in v1)
- Multi-arch native via TARGETARCH (amd64 + arm64; no QEMU on Apple Silicon)
- Pinnable protobuf version via LOADER_PROTOBUF_VERSION host env var,
  flowing through devcontainer.json build args into vcpkg manifest mode
- Devcontainer is recommended path; prepare.bat / per-language manual
  setup stays as fallback for contributors who can't run Docker
- CI keeps lukka/run-vcpkg directly (devcontainer is not used in CI)

Implementation plan to follow via writing-plans.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
12-task plan implementing the design at
docs/superpowers/specs/2026-05-29-devcontainer-design.md.
Each task is a single Dockerfile layer or config file with
build/verify/commit steps and concrete expected outputs.

Refs: docs/superpowers/specs/2026-05-29-devcontainer-design.md

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Bootstrap the devcontainer image with Microsoft's multi-arch
mcr.microsoft.com/devcontainers/cpp:1-ubuntu-24.04 base. Subsequent
commits layer Go, buf, vcpkg/protobuf, .NET, and Node on top.

Refs: docs/superpowers/specs/2026-05-29-devcontainer-design.md

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Resolves TARGETARCH (amd64 or arm64) into per-arch values
(Go tarball arch, buf release-asset arch, vcpkg triplet) and
writes them to /opt/buildargs.env for downstream RUN layers
to source. Unknown arches fail the build.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Install Go from the official multi-arch tarball into /usr/local/go.
PATH is exposed via ENV (not /etc/profile.d) so non-interactive shells
(postCreateCommand, downstream RUNs) see `go` without sourcing profile.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Single-binary release into /usr/local/bin/buf. Pinned to the same
version testing-cpp.yml / testing-csharp.yml use.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Pin vcpkg to commit dc8d75c…df932 (lock-step with prepare.bat and
testing-cpp.yml's VCPKG_COMMIT). Render a minimal vcpkg.json manifest
with the protobuf override + builtin-baseline, install via
manifest mode (the only mode where the version pin actually takes
effect), and post-assert that the resolved port version starts with
the requested PROTOBUF_VERSION. Default is 6.33.4; legacy v3 reachable
via --build-arg PROTOBUF_VERSION=3.21.12.

Symlink /opt/vcpkg/active → installed/<triplet> and /usr/local/bin/protoc
→ active/tools/protobuf/protoc so downstream ENV/PATH stays
arch-independent.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Microsoft apt repo for dotnet-sdk-8.0 (matches testing-csharp.yml's
target). NodeSource setup_20.x for Node 20 LTS (covers the
experimental _lab/ts/ workflow; not currently in CI). Both clean
their apt caches to keep the layer small.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
CMAKE_PREFIX_PATH=/opt/vcpkg/active lets the existing README cmake
recipe (-DCMAKE_BUILD_TYPE=Debug, no toolchain file) resolve protobuf
inside the container without any flag changes from the host workflow.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Wires the Dockerfile under build.args, mounts a named volume for the
Go module cache, declares the VS Code extension set, and prints a
one-line ready-banner via postCreateCommand. PROTOBUF_VERSION flows
from the host LOADER_PROTOBUF_VERSION env var (default 6.33.4) so
contributors can rebuild against the legacy v3 line via:
  LOADER_PROTOBUF_VERSION=3.21.12 code .

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
One-pager covering prerequisites (Docker Desktop / Engine + VS Code
Dev Containers extension), how to open the container, the
LOADER_PROTOBUF_VERSION knob, host-OS caveats (WSL2 workspace
location, Apple Silicon native arm64), and the layered Dockerfile
architecture. Points users at prepare.bat / per-language manual setup
as the explicit fallback path.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Add a new "Recommended: Dev Container (any host OS)" subsection at the
top of Prerequisites pointing contributors at .devcontainer/. Add a
"Skip this section if you're using the devcontainer" lead-in to the
existing "Install protobuf" and "Windows: bootstrap" blocks so the
manual paths are clearly the fallback, not the primary route.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributors with an existing host checkout can hit confusing build
errors when stale .pb.cc / .pb.cs files from a previous protoc-3.x
session linger in the workspace (gitignored, so git pull doesn't
remove them). The container's modern protoc 33.4 emits a different
file layout, and the leftovers shadow what's freshly generated. Add
a Troubleshooting section with the rm -rf recipe to wipe and retry.
A fresh clone doesn't hit this.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…TODO

The Troubleshooting recipe added in 8a583ba targeted directories that
don't actually hold stale generated files: src/tableau is empty (real
C++ stale files live at src/protoconf/tableau/), and protoconf/tableau
doesn't exist (C# protoc emits flat .cs files into protoconf/). Fix
the rm -rf paths so the recipe actually unblocks the failure mode it
documents.

Also drop the now-contradictory "> TODO: [devcontainer]" placeholder
from README.md line 7 — the devcontainer is implemented and recommended
in the section immediately below it.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…nfile cache

Docker creates named volumes as root:root by default, which leaves the
Go module cache mounted at /home/vscode/go unwritable for the `vscode`
user inside the devcontainer. Chown it to `vscode:vscode` in
postCreateCommand so `go mod download` / `go build` work out of the box.

Also ignore dotnet/runfile-discovery/, an auto-generated .NET SDK 10
file-based app discovery cache produced by the dotnet CLI / C# Dev Kit.
Update test/buf.yaml dependency pin and refresh test/buf.lock with the
new commit / digest.
wenchy and others added 8 commits June 1, 2026 16:09
…ain pins

Splits .devcontainer/ into linux/ (multi-arch, recommended) and windows/
(windows/amd64 only, native MSVC for Windows hosts that don't want WSL2),
plus a macos/ docs path explaining why no macOS container exists.

All toolchain versions (Go, buf, protobuf, vcpkg baseline, .NET, Node,
CMake) move into a single source of truth at .devcontainer/shared/versions.env,
consumed by both Dockerfiles, prepare.bat, and the CI workflows via a
new ./.github/actions/load-versions composite action. Bumping any
version is now a one-line change instead of touching 3-5 files.

Adds two path-gated smoke workflows (devcontainer-{linux,windows}-smoke.yml)
that build the devcontainer images on devcontainer-touching PRs so a
versions.env typo or Dockerfile regression won't ship. Linux smoke runs
on amd64 + arm64 to cover the multi-arch claim that testing-cpp.yml
doesn't exercise.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
GitHub Actions evaluates ${{ ... }} expressions in the description field
at parse time, and `env` is not in the allowed context for action
manifests. Rephrase to plain prose so the manifest loads.

Reported by the workflow run as:
  Unrecognized named-value: 'env'. Located at position 1 within
  expression: env.KEY

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…kerCli

Current windows-2022 GitHub runners ship Moby Engine instead of Docker
Desktop, so `$env:ProgramFiles\Docker\Docker\DockerCli.exe -SwitchDaemon`
no longer resolves. Re-register the `docker` Windows service directly
via dockerd.exe; probes both legacy and current install paths and
emits diagnostics if neither is present.

Reported by the previous workflow run as:
  The term 'C:\Program Files\Docker\Docker\DockerCli.exe' is not
  recognized as a name of a cmdlet, function, script file, or
  executable program.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…dio/buildtools

The plain windows/servercore:ltsc2022 base image cannot host the modern
VS 2022 Build Tools installer. vs_BuildTools.exe expects Windows
servicing components that servercore strips, so the chocolatey
visualstudio2022buildtools install fails with installer exit code
-2146232797 (COR_E_NOTSUPPORTED) deep into the install.

Microsoft publishes mcr.microsoft.com/visualstudio/buildtools:ltsc2022
specifically to solve this — same servercore base but with VS 2022
Build Tools (VC++ workload) pre-installed, exposing
C:\BuildTools\Common7\Tools\VsDevCmd.bat at a fixed path.

Drops the ~6 GB chocolatey Build Tools install layer (now in the base
image) and lets vcpkg-install.ps1 skip the vswhere probe and go
directly to C:\BuildTools\... with vswhere kept as a defensive
fallback for future base-image layout changes.

Discovered while testing the Windows devcontainer locally:
  ERROR: The installation of visualstudio2022buildtools failed
  (installer exit code: -2146232797).

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
mcr.microsoft.com/visualstudio/buildtools doesn't exist as a published
image (I invented the path from memory). Use Microsoft's actually
documented pattern: base on dotnet/framework/runtime:4.8-windowsservercore-ltsc2022
and download vs_BuildTools.exe from aka.ms/vs/17/release/vs_buildtools.exe.

Microsoft's official VS Build Tools container guidance
(https://learn.microsoft.com/en-us/visualstudio/install/build-tools-container)
explicitly recommends the dotnet-framework base over plain
windows/servercore, because the modern vs_BuildTools.exe installer
needs .NET Framework runtime support that servercore strips.

Side benefit: dotnet/framework/runtime:4.8-windowsservercore-ltsc2022
is pre-cached on GitHub's windows-2022 runners (per the runner image
manifest), so the CI smoke job's image pull is free.

Also pass --memory 2GB to the smoke job's docker build — Microsoft
documents this as required for vs_BuildTools (default 1 GB silently
fails partway through).

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…pattern

Mirrors linux/Dockerfile's approach of COPYing versions.env into the
image and reading it per-RUN. PowerShell has no POSIX `source`, so we
provide one as Import-LoaderVersions.ps1 and bake it into the SHELL
prefix; every RUN's body executes with $env:GO_VERSION etc. populated
from C:\loader\versions.env.

Three real bugs fixed:

1. The previous version's `Get-Content $path | ConvertFrom-StringData`
   pipeline was a trap: ConvertFrom-StringData consumed input
   line-by-line and produced an array of 7 single-key hashtables
   instead of one merged hashtable. Looping over `$vars.Keys` was
   therefore a no-op and $env:GO_VERSION came up empty. The CI run
   on 4e35924 hit this directly:
       https://go.dev/dl/go${env:GO_VERSION}.windows-amd64.msi
       -> resolved to https://go.dev/dl/go.windows-amd64.msi  (404)
   Fixed by joining the lines into one string and calling
   `ConvertFrom-StringData -StringData`.

2. The previous version relied on
   `[Environment]::SetEnvironmentVariable(... 'Machine')` persisting
   across RUN layer boundaries via registry. ARG/ENV is the documented
   portable mechanism; Machine-scope env in Windows containers is
   unreliable across layers. The new pattern doesn't depend on it.

3. The previous version had a `start /w vs_buildtools.exe ...` cmd-shell
   sequence that swallowed the actual exit code. Replaced with a
   Start-Process + ExitCode check that fails the build with the real
   error code.

Symmetry win: the smoke workflow no longer needs to know the list of
pinned versions — it just runs `docker build`, exactly like the Linux
smoke job. Single source of truth in versions.env; build wrappers
stay version-agnostic.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…probes

The previous version probed three hardcoded Program Files paths for
dockerd.exe. None of them match the path Microsoft's MobyOnWindowsRunner
installer chose on current actions/runner-images windows-2022 builds —
the upstream installer doesn't commit to a stable location.

Resolve dockerd via the registered `docker` Windows service's ImagePath
instead. That works regardless of where the runner image installed
Docker, and degrades to a useful diagnostic ("docker service is not
registered") if the runner doesn't have Docker at all.

Reported by the previous workflow run as:
  dockerd.exe not found in any of:
    C:\Program Files\docker\dockerd.exe;
    C:\Program Files\Docker\Docker\resources\dockerd.exe;
    C:\Program Files\Docker\dockerd.exe

Also fixed: under $ErrorActionPreference='Stop', the previous Write-Error
fallback aborted before the diagnostic Get-ChildItem could run, so
nothing useful was logged. Throw with explicit diagnostic capture
beforehand.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The Windows-container ecosystem (HNS, WinNAT, NDIS filter drivers,
Hyper-V virtual switches) is too sensitive to host config to be
reliable for daily dev. Real failures hit during validation included:

- WinNAT half-state where Get-NetNat returned empty but
  Get-NetNatExternalAddress still showed stale bindings, blocking
  New-NetNat with "Windows System Error 52: duplicate name".
- Ghost vEthernet (nat) interfaces registered to the IP stack with no
  associated Get-NetAdapter (Description=<no adapter>).
- Container outbound TCP fully blocked at the host NAT layer despite
  ICMP working and per-interface forwarding being enabled. WFP audit
  logging found zero drop events for the container's source IP, so
  the issue was in the NAT translation path, not in any filter callout.
- HNS service restart broke dockerd's Windows engine API with persistent
  500 Internal Server Error survivable across factory reset and reboot.

For Windows hosts the alternatives are:
  1. Linux container under WSL2 - the documented recommendation.
     Multi-arch, multi-OS, none of the issues above.
  2. Bare-metal native dev via prepare.bat (C++ toolchain) plus winget
     for Go / .NET / Node. Documented in the repo root README's new
     "Windows (bare-metal)" section.

Removes:
- .devcontainer/windows/ (Dockerfile, devcontainer.json, README,
  vcpkg-install.ps1, Import-LoaderVersions.ps1)
- .github/workflows/devcontainer-windows-smoke.yml
- .devcontainer/shared/postcreate-banner.ps1 (orphaned without the
  Windows devcontainer)

Updates docs to reflect two devcontainer subfolders (linux/, macos/)
instead of three; macOS already follows this docs-only pattern, and
the new Windows path is parallel - winget one-liners + prepare.bat
plus a pointer at the Linux container under WSL2.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants