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.

Sign up for GitHub

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

Jump to bottom

[yaml-cpp] Update to 0.8.0 #33223

Merged
merged 3 commits into from
Nov 1, 2023
Merged

[yaml-cpp] Update to 0.8.0 #33223

merged 3 commits into from
Nov 1, 2023

Conversation

FtZPetruska
Copy link
Contributor

  • Changes comply with the maintainer guide
  • SHA512s are updated for each updated download
  • The "supports" clause reflects platforms that may be fixed by this new version
  • Any fixed CI baseline entries are removed from that file.
  • Any patches that are no longer applied are deleted from the port's directory.
  • The version database is fixed by rerunning ./vcpkg x-add-version --all and committing the result.
  • Only one version is added to each modified port's versions file.

A few additional tweaks were made to the portfile, notably:

  • The install location for the CMake configs and pkg-config files were moved from share to lib, so moving the pc files manually is no longer needed.
  • YAML_CPP_DLL is no longer used in dll.h, instead replaced with YAML_CPP_STATIC_DEFINE. The inline patch was updated as such. Moreover, it was extended to non-Windows platforms as it also applies to them.

@FtZPetruska
Copy link
Contributor Author

Three ports are failing:

  • opencolorio
  • jaeger-client-cpp
  • rtabmap

@FtZPetruska FtZPetruska force-pushed the yaml-cpp-bump-0.8.0 branch 2 times, most recently from a2b0370 to 77f2213 Compare August 16, 2023 23:48
@FtZPetruska
Copy link
Contributor Author

The first issue from upgrading to 0.8.0 was broken CMake configs, PATH_VARS were used to load the targets, breaking when vcpkg_cmake_config_fixup was used.

The patch added in 7900bdf is a copy of what was submitted upstream in jbeder/yaml-cpp#1212.

The remaining build failures come from a change in the CMake target name from yaml-cpp to yaml-cpp::yaml-cpp.

@Cheney-W Cheney-W added the category:port-update The issue is with a library, which is requesting update new revision label Aug 17, 2023
@FtZPetruska FtZPetruska marked this pull request as draft August 17, 2023 14:44
@FtZPetruska
Copy link
Contributor Author

The patches to fix the new target name were relatively trivial:

  • jaeger-tracing-client-cpp and opencolorio needed their dependency patches to be updated.
  • rtabmap needed a new patch to fix the target used for yaml-cpp.

@FtZPetruska FtZPetruska marked this pull request as ready for review August 17, 2023 16:31
@FtZPetruska
Copy link
Contributor Author

OpenColorIO and rtabmap have been notified and provided a patch.
On the other hand, the Jaeger Tracing Client repo is archived.

@Cheney-W
Copy link
Contributor

Cheney-W commented Aug 18, 2023 •
edited
Loading

Please do not change the line breaks in the ports/jaeger-client-cpp/fix-CMakeLists.patch file. We want to keep the modifications as concise as possible.

Moving the PR to draft. Please set it to "ready for review" once changes have been applied.

@Cheney-W Cheney-W marked this pull request as draft August 18, 2023 02:30
@FtZPetruska
Copy link
Contributor Author

Please do not change the line breaks in the ports/jaeger-client-cpp/fix-CMakeLists.patch file. We want to keep the modifications as concise as possible.

My apologies, vscode did not show the change of newline character in the diff. It should be fixed now.

@FtZPetruska FtZPetruska marked this pull request as ready for review August 18, 2023 02:40
Cheney-W
Cheney-W previously approved these changes Aug 18, 2023
View reviewed changes
@Cheney-W
Copy link
Contributor

Thank you very much for your prompt response and for contributing to vcpkg.

@Cheney-W Cheney-W added the info:reviewed Pull Request changes follow basic guidelines label Aug 18, 2023
@dan-shaw dan-shaw added the depends:upstream-changes Waiting on a change to the upstream project label Aug 18, 2023
@dan-shaw dan-shaw removed the info:reviewed Pull Request changes follow basic guidelines label Sep 1, 2023
@Arech
Copy link

Arech commented Sep 21, 2023

Hey, guys, thanks for working on this! Any forecasts when v0.8.0 will be merged?

@FtZPetruska FtZPetruska marked this pull request as draft September 25, 2023 11:23
@FtZPetruska
Copy link
Contributor Author

Looks like there are some merge conflicts with recent updates, I switched this PR to draft until I find time to resolve them.

@liuchibing
Copy link

Can you finish this work please? @FtZPetruska

@FtZPetruska
[yaml-cpp] Update to 0.8.0.
103af54 
- The config path has changed to `lib/cmake/yaml-cpp`.
- pkg-config files are installed to `lib/pkgconfig`.
- dll.h uses a new variable for control and should be patched outside
of Windows.
@FtZPetruska
[yaml-cpp] Backport CMake fixes from upstream.
80f3576 
The patch is a shortened version from:
jbeder/yaml-cpp#1212

It fixes issues with downstream projects using the old target, and makes
the CMake config more easily relocatable.

The patch was rebased on the 0.8.0 release, and the CI and tests bits
were omitted.
@FtZPetruska
[yaml-cpp] Update baseline.
c8148c7
@FtZPetruska FtZPetruska marked this pull request as ready for review November 1, 2023 07:19
@Cheney-W
Copy link
Contributor

Cheney-W commented Nov 1, 2023

Usage test passed with x64-windows.

@Cheney-W Cheney-W added info:reviewed Pull Request changes follow basic guidelines and removed depends:upstream-changes Waiting on a change to the upstream project labels Nov 1, 2023
@JavierMatosD JavierMatosD merged commit e9fba6a into microsoft:master Nov 1, 2023
15 checks passed
@autoantwort
Copy link
Contributor

This resulted in #35041

@FtZPetruska FtZPetruska deleted the yaml-cpp-bump-0.8.0 branch November 11, 2023 18:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dg0yt dg0yt dg0yt left review comments

@dan-shaw dan-shaw dan-shaw approved these changes

@Cheney-W Cheney-W Cheney-W approved these changes

Assignees

@Cheney-W Cheney-W

Labels
category:port-update The issue is with a library, which is requesting update new revision info:reviewed Pull Request changes follow basic guidelines
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@FtZPetruska @Cheney-W @Arech @liuchibing @autoantwort @dg0yt @dan-shaw @JavierMatosD