[protobuf] Make target libprotoc opt-in for non-native builds#51545
[protobuf] Make target libprotoc opt-in for non-native builds#51545dudantas wants to merge 11 commits intomicrosoft:masterfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates the protobuf port to skip building/installing target libprotoc (and related libupb) when doing non-native builds (TARGET_TRIPLET != HOST_TRIPLET), reusing the existing protobuf_BUILD_PROTOC_BINARIES decision to reduce build time and package size for cross-compilation scenarios.
Changes:
- Tie
protobuf_BUILD_LIBPROTOCtoprotobuf_BUILD_PROTOC_BINARIES(so it’s disabled for non-native target builds, and still disabled for UWP). - Bump the
protobufport-version to2. - Update the versions database and baseline to reflect the new port-version.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| ports/protobuf/portfile.cmake | Disables building libprotoc (and libupb) for non-native target builds by reusing protobuf_BUILD_PROTOC_BINARIES. |
| ports/protobuf/vcpkg.json | Bumps port-version to 2. |
| versions/baseline.json | Updates baseline protobuf port-version to 2. |
| versions/p-/protobuf.json | Adds a new entry for protobuf 6.33.4 port-version 2. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Add a libprotoc feature to the protobuf port and make it a default feature. Map the feature to protobuf_BUILD_TARGET_LIBPROTOC in portfile.cmake and adjust logic to enable libprotoc when building protoc binaries or when the libprotoc feature is requested; keep it off by default for non-UWP unless enabled. Also update the recorded git-tree for protobuf to the new commit hash.
|
@microsoft-github-policy-service agree |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
BillyONeal
left a comment
There was a problem hiding this comment.
"Request changes" is for adding the default-feature, the comment is just a nitpick.
| if(VCPKG_TARGET_IS_UWP) | ||
| set(protobuf_BUILD_LIBPROTOC OFF) | ||
| else() | ||
| elseif(protobuf_BUILD_PROTOC_BINARIES OR protobuf_BUILD_TARGET_LIBPROTOC) | ||
| set(protobuf_BUILD_LIBPROTOC ON) | ||
| else() | ||
| set(protobuf_BUILD_LIBPROTOC OFF) | ||
| endif() |
There was a problem hiding this comment.
I think this would be better as just the expression rather than mixing mutually exclusive if/elseif chains flipping between off and on. Consider:
if(VCPKG_TARGET_IS_UWP OR NOT (protobuf_BUILD_PROTOC_BINARIES OR protobuf_BUILD_TARGET_LIBPROTOC))
set(protobuf_BUILD_LIBPROTOC OFF)
else()
set(protobuf_BUILD_LIBPROTOC ON)
endif()or
set(protobuf_BUILD_LIBPROTOC OFF)
if(NOT VCPKG_TARGET_IS_UWP AND (protobuf_BUILD_PROTOC_BINARIES OR protobuf_BUILD_TARGET_LIBPROTOC))
set(protobuf_BUILD_LIBPROTOC ON)
endif()(or possibly even)
set(protobuf_BUILD_LIBPROTOC OFF)
if(NOT VCPKG_TARGET_IS_UWP)
if(protobuf_BUILD_PROTOC_BINARIES OR protobuf_BUILD_TARGET_LIBPROTOC)
set(protobuf_BUILD_LIBPROTOC ON)
endif()
endif()Make libprotoc an opt-in feature (non-UWP) and update ports that require it. Changes: protobuf portfile and vcpkg.json now treat libprotoc as a feature that supports !uwp and only builds libprotoc on non-UWP when requested; removed libprotoc from protobuf default-features. brpc and offscale-libetcd-cpp ports now request the protobuf feature libprotoc and have their port-version incremented. Updated corresponding versions entries and baseline to reflect the new port-versions and updated git-tree for protobuf.
cbd5a41 to
b58bc0a
Compare
Thanks, @BillyONeal, I've updated. I removed
|
Remove libprotoc mapping from vcpkg_check_features and instead explicitly initialize protobuf_BUILD_TARGET_LIBPROTOC to OFF, then set it ON if "libprotoc" is present in the FEATURES list. This ensures the variable is always defined and robustly reflects whether the libprotoc feature was enabled before later logic that may enable building libprotoc.
BillyONeal
left a comment
There was a problem hiding this comment.
dudantas#2 fixes the version database and has 1 nitpick.
Use vcpkg_check_features and fix version database.
Can you check again, please? I've updated this. |
Summary
This PR makes target
libprotocan explicit opt-in feature of theprotobufport.For non-native builds where
TARGET_TRIPLET != HOST_TRIPLET, the port already uses the hostprotobufpackage to provide theprotocbuild-time executable. In that scenario, this PR stops building/installing targetlibprotocandlibupbunless the newlibprotocfeature is requested.Native builds keep the existing behavior: when
TARGET_TRIPLET == HOST_TRIPLET,protocand targetlibprotocare still built together.Motivation
Currently, non-native target builds use host
protobuffor theprotocexecutable, but still build and install targetlibprotocandlibupb.Many consumers only need the protobuf runtime libraries, such as
libprotobuforlibprotobuf-lite, plus the hostprotoctool for code generation. They do not need to link targetprotobuf::libprotoc.libprotocis a public library target, so it should be requested explicitly by consumers that need it, rather than modeled as a default feature.Changes
This PR introduces an opt-in
libprotocfeature to theprotobufport.protobuf[core]/ default non-native target builds no longer build targetlibprotocor targetlibupb.protobuf[libprotoc]builds targetlibprotocand targetlibupbfor non-UWP target builds.protocandlibprotocbecauseprotobuf_BUILD_PROTOC_BINARIESremains enabled whenTARGET_TRIPLET == HOST_TRIPLET.protobuf::libprotocnow requestprotobuf[libprotoc]explicitly:brpcoffscale-libetcd-cppBehavior after this change
protocbinarylibprotocTARGET_TRIPLET == HOST_TRIPLETTARGET_TRIPLET != HOST_TRIPLETcoreTARGET_TRIPLET != HOST_TRIPLETlibprotocValidation
Validated locally with separate install/build/package roots.
Non-native target build without
libprotocCommand shape used:
Expected result:
x64-windows-staticinstallslibprotobufx64-windows-staticinstallslibprotobuf-litex64-windows-staticdoes not install targetlibprotocx64-windows-staticdoes not install targetlibupbx64-windowsprovidesprotoc.exeNon-native target build with
libprotocCommand shape used:
Expected result:
x64-windows-staticinstallslibprotobufx64-windows-staticinstallslibprotobuf-litex64-windows-staticinstalls targetlibprotocx64-windows-staticinstalls targetlibupbx64-windowsprovidesprotoc.exeNative build behavior
The change preserves native behavior because
protobuf_BUILD_LIBPROTOCremains enabled whenprotobuf_BUILD_PROTOC_BINARIESis enabled.Port update checklist
./vcpkg x-add-version protobuf brpc offscale-libetcd-cppand committing the result.