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

[fmt] Update to 10.0.0 #31378

Merged
merged 8 commits into from
May 22, 2023
Merged

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.

@FtZPetruska
Copy link
Contributor Author

coolprop and seacas failures come from a breaking change with 10.0 where enums require a custom formatter.

spdlog 1.11 has known issues with the latest fmt version. It has already been fixed upstream but no release contain this fix. (gabime/spdlog@0ca574a)

@Cheney-W Cheney-W added the category:port-update The issue is with a library, which is requesting update new revision label May 11, 2023
@Cheney-W
Copy link
Contributor

Are the upstream projects of coolprop and seacas aware of this issue? If they are not aware, could you report this issue to the upstream projects to seek a solution?
As for spdlog, could you use a patch to apply the upstream fix?

@Cheney-W Cheney-W marked this pull request as draft May 11, 2023 10:00
@Cheney-W
Copy link
Contributor

If updating a port causes issues with downstream ports, it is necessary to fix those downstream ports as well.

@Cheney-W
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@FtZPetruska FtZPetruska force-pushed the bump-fmt-10.0.0 branch 2 times, most recently from 83ef8ef to 03cc3d8 Compare May 14, 2023 15:04
@FtZPetruska FtZPetruska marked this pull request as ready for review May 14, 2023 19:20
Cheney-W
Cheney-W previously approved these changes May 15, 2023
@Cheney-W Cheney-W added the info:reviewed Pull Request changes follow basic guidelines label May 15, 2023
Copy link
Contributor

@JavierMatosD JavierMatosD left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Has upstream been notified of this issue/ are they planning on fixing it? We would prefer to avoid patching whenever possible and some of these patches are very large. If upstream is aware of the issue and working on the solution, it might be best to wait for their update.

Additionally, I see this patch being added to the coolprop port, but I don't see it being applied in the portfile.

@JavierMatosD JavierMatosD marked this pull request as draft May 15, 2023 23:05
@FtZPetruska
Copy link
Contributor Author

FtZPetruska commented May 16, 2023

Has upstream been notified of this issue/ are they planning on fixing it? We would prefer to avoid patching whenever possible and some of these patches are very large. If upstream is aware of the issue and working on the solution, it might be best to wait for their update.

Additionally, I see this patch being added to the coolprop port, but I don't see it being applied in the portfile.

I hear your concerns, regarding upstream:

Regarding Coolprop's patch not being added to the portfile, there already was a fmt-fix.patch that was updated to include fmt 10 support.

Hopefully this addresses your concerns, let me know if there is any change you wish to be made.

@FtZPetruska
Copy link
Contributor Author

It seems that updating openimageio to the latest release fixes fmt 10 issues, but has other build issues specifically on MSVC unrelated to fmt.

I went ahead and opened a standalone PR to update the port since it's no longer related to this PR. (#31489)

@Cheney-W Cheney-W added depends:different-pr This PR or Issue depends on a PR which has been filed and removed info:reviewed Pull Request changes follow basic guidelines labels May 18, 2023
- Remove dead code. It is no longer necessary to manually move DLLs and
edit CMake configs.
- Use vcpkg_install_copyright.
* Update to 2023.05.08.00.
* Add patch to support fmt 10.
* Update baseline.
* Add upstream patch to support fmt 10.
* Update portfile.
* Add usage.
* Update baseline.
* Add patch to support fmt 10.0.0.
* Update baseline.
* Update patch to support fmt 10.0.0. See CoolProp/CoolProp#2252
* Update baseline.
* Add patch to support fmt 10
* Update baseline
@Cheney-W Cheney-W removed the depends:different-pr This PR or Issue depends on a PR which has been filed label May 19, 2023
@FtZPetruska FtZPetruska marked this pull request as ready for review May 19, 2023 07:37
@Cheney-W Cheney-W added the info:reviewed Pull Request changes follow basic guidelines label May 19, 2023
@JavierMatosD JavierMatosD merged commit 656fcc6 into microsoft:master May 22, 2023
@FtZPetruska FtZPetruska deleted the bump-fmt-10.0.0 branch May 22, 2023 14:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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
Development

Successfully merging this pull request may close these issues.

3 participants