Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[protobuf] protobuf v3.12.0 #11397

Merged
merged 2 commits into from
May 19, 2020
Merged

Conversation

jozefizso
Copy link
Contributor

Update protobuf package to release v3.12.0

@LilyWangL
Copy link
Contributor

@jozefizso Thanks for your PR. Now ignition-msgs5 build failed with the following error, it build depends on protobuf.
buildtrees\ignition-msgs5\src\sgs5_5.1.0-e593042925\src\Utility.cc : fatal error C1128: number of sections exceeded object file format limit: compile with /bigobj

@jozefizso
Copy link
Contributor Author

The /bigobj switch is set in CMakeLists.txt already.

  # Allow big object
  add_definitions(/bigobj)

Source: https:/protocolbuffers/protobuf/blob/3.12.x/cmake/CMakeLists.txt

@jozefizso
Copy link
Contributor Author

I added patch for ignition-msg5 to use /bigobj switch. This should make it compatible with protobuf 3.12.

@jozefizso
Copy link
Contributor Author

The microsoft.vcpkg.pr (x64_osx) failed because of git issue:

From https:/microsoft/vcpkg
 * [new ref]             refs/pull/11397/merge -> pull/11397/merge
git checkout --progress --force refs/remotes/pull/11397/merge
Previous HEAD position was 35a71e008 Merge 6248f7cc6dd2a25fe8b8f31d8f8ef1c76eec5ed9 into 8a583e80da3b72141105da9003175679af2fcb92
fatal: update_ref failed for ref 'HEAD': cannot lock ref 'HEAD': Unable to create '/Volumes/data/work/1/s/.git/HEAD.lock': File exists.

Another git process seems to be running in this repository, e.g.
an editor opened by 'git commit'. Please make sure all processes
are terminated then try again. If it still fails, a git process
may have crashed in this repository earlier:
remove the file manually to continue.
##[error]Git checkout failed with exit code: 128

Source: https://dev.azure.com/vcpkg/public/_build/results?buildId=37113&view=logs&jobId=7b75bd19-17d3-53d4-00fd-23f1a49a8ba4&j=7b75bd19-17d3-53d4-00fd-23f1a49a8ba4&t=6c1a7a3b-b385-5a17-556f-78ddf167924d

@LilyWangL LilyWangL added the info:reviewed Pull Request changes follow basic guidelines label May 19, 2020
@strega-nil
Copy link
Contributor

Cool, thanks @jozefizso :)

@strega-nil strega-nil merged commit e6c6d96 into microsoft:master May 19, 2020
@BillyONeal
Copy link
Member

Looks like this might have caused regressions when merged with other changes: https://dev.azure.com/vcpkg/public/_build/results?buildId=37252&view=artifacts&type=publishedArtifacts

-- Downloading https:/protocolbuffers/protobuf/archive/v3.12.0.tar.gz...
-- Extracting source C:/agent/_work/1/s/downloads/protocolbuffers-protobuf-v3.12.0.tar.gz
-- Applying patch fix-uwp.patch
-- Applying patch fix-android-log.patch
-- Using source at C:/agent/_work/1/s/buildtrees/protobuf/src/v3.12.0-8ba83cbbdb
CMake Error at ports/protobuf/portfile.cmake:21 (message):
  Cross-targetting protobuf requires the x86-windows protoc to be available.
  Please install protobuf:x86-windows first.
Call Stack (most recent call first):
  scripts/ports.cmake:90 (include)

@LilyWangL
Copy link
Contributor

LilyWangL commented May 20, 2020

Due to the following code, protobuf arm64-windows arm-uwp x64-uwp build failed on CI pipelines.

if(CMAKE_HOST_WIN32 AND NOT VCPKG_TARGET_ARCHITECTURE MATCHES "x64" AND NOT VCPKG_TARGET_ARCHITECTURE MATCHES "x86")
    set(protobuf_BUILD_PROTOC_BINARIES OFF)
elseif(CMAKE_HOST_WIN32 AND VCPKG_CMAKE_SYSTEM_NAME)
    set(protobuf_BUILD_PROTOC_BINARIES OFF)
else()
    set(protobuf_BUILD_PROTOC_BINARIES ON)
endif()

if(NOT protobuf_BUILD_PROTOC_BINARIES AND NOT EXISTS ${CURRENT_INSTALLED_DIR}/../x86-windows/tools/protobuf)
    message(FATAL_ERROR "Cross-targetting protobuf requires the x86-windows protoc to be available. Please install protobuf:x86-windows first.")
endif()

This error was not detected due to the cache on the CI, I will modify the ci.baseline.txt to fix this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
info:reviewed Pull Request changes follow basic guidelines
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants