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 9.0.0 #25658

Merged
merged 4 commits into from
Aug 4, 2022
Merged

[fmt] Update to 9.0.0 #25658

merged 4 commits into from
Aug 4, 2022

Conversation

spnda
Copy link
Contributor

@spnda spnda commented Jul 9, 2022

Describe the pull request
Updates the fmtlib port to 9.0.0.

  • What does your PR fix?

    Fixes [fmt] update to 9.0.0 #25642.

  • Which triplets are supported/not supported? Have you updated the CI baseline?

    All, No, not required

  • Does your PR follow the maintainer guide?

    Yes

  • If you have added/updated a port: Have you run ./vcpkg x-add-version --all and committed the result?

    Yes

github-actions[bot]
github-actions bot previously approved these changes Jul 9, 2022
@spnda
Copy link
Contributor Author

spnda commented Jul 9, 2022

It seems like the openimageio port depends on fmt and fails to build with fmt 9.0.0. Is there perhaps some functionality to restrict building openimageio with fmt 9.0.0? I personally can't test out builds or see where and why it fails because the bundled meson doesn't seem to like macOS 13 and Xcode 14.

@dg0yt
Copy link
Contributor

dg0yt commented Jul 10, 2022

In #23918, I arrived at updating openimageio. But the PR isn't ready yet.

@FrankXie05 FrankXie05 added the category:port-update The issue is with a library, which is requesting update new revision label Jul 11, 2022
@FrankXie05
Copy link
Contributor

Log from x86-windows

D:\buildtrees\openimageio\x86-windows-dbg\include\OpenImageIO\detail\fmt\core.h(1733): error C2338: static_assert failed: 'Cannot format an argument. To make type T formattable provide a formatter<T> specialization: https://fmt.dev/latest/api.html#udt'
D:\buildtrees\openimageio\x86-windows-dbg\include\OpenImageIO\detail\fmt\core.h(1753): note: see reference to function template instantiation 'fmt::v9::detail::value<Context> fmt::v9::detail::make_value<Context,const T&>(const OpenImageIO_v2_3::string_view&)' being compiled
        with
        [
            Context=context,
            T=OpenImageIO_v2_3::string_view
        ]

install-x86-windows-dbg-out.log

@FrankXie05 FrankXie05 added the depends:different-pr This PR or Issue depends on a PR which has been filed label Jul 14, 2022
@BullyWiiPlaza
Copy link
Contributor

BullyWiiPlaza commented Jul 24, 2022

I'm hoping for a merge soon since I was also getting these annoying warnings lately in Visual Studio 2022:
C:\vcpkg\installed\x64-windows-static\include\fmt\format.h(1812,18): warning C4189: 'zero': local variable is initialized but not referenced
It seems like fmt version 9.0.0 fixes it.

@spnda
Copy link
Contributor Author

spnda commented Jul 24, 2022

@BullyWiiPlaza I'm also eager to get this merged. Though fmt 9 introduced 2 new warnings that have since been fixed but have not been released with a 9.1 or 9.0.1 update yet.

The CI failures currently only come from the openimageio port being severely outdated and it currently will not compile with fmt v9. Though, as I'm not too experienced with how vcpkg handles port dependencies, I would like to ask the vcpkg team if there was any way in telling the CI that that port simply won't compile with fmt 9, and have the compile failures be expected? That would get this PR merged, while having openimageio users still be able to use it with the previous version of the fmt port.

@FrankXie05
Copy link
Contributor

@spnda @BullyWiiPlaza Sorry, we have to wait for Openimageio to update before we can update fmt to 9.0.0. However, you can temporarily clone the PR #23918 changes to self-update Openimageiolo locally to support fmt 9.0.0. :)

@walge-laine
Copy link
Contributor

Now that #23918 has been merged, is this PR being worked on?

@spnda
Copy link
Contributor Author

spnda commented Aug 3, 2022

@walge-laine There's nothing for this PR to work on. I'll just trigger the CI to re-run with the changes on master and all of that should succeed. Thanks for reminding me though, I was not subscribed to the other PR.

github-actions[bot]
github-actions bot previously approved these changes Aug 3, 2022
github-actions[bot]
github-actions bot previously approved these changes Aug 4, 2022
@FrankXie05 FrankXie05 removed the depends:different-pr This PR or Issue depends on a PR which has been filed label Aug 4, 2022
FrankXie05
FrankXie05 previously approved these changes Aug 4, 2022
@FrankXie05
Copy link
Contributor

@spnda @walge-laine I merged the branch of this pr into the master. Now this PR is working fine, I think we can merge it now. :)

@FrankXie05 FrankXie05 added the info:reviewed Pull Request changes follow basic guidelines label Aug 4, 2022
ports/fmt/vcpkg.json Outdated Show resolved Hide resolved
@BillyONeal BillyONeal merged commit e2a7148 into microsoft:master Aug 4, 2022
@BillyONeal
Copy link
Member

Thanks for the update!

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.

[fmt] update to 9.0.0
7 participants