Skip to content

[fmt] Add usage file#13815

Merged
BillyONeal merged 1 commit intomicrosoft:masterfrom
dgellow:add-fmt-usage
Oct 9, 2020
Merged

[fmt] Add usage file#13815
BillyONeal merged 1 commit intomicrosoft:masterfrom
dgellow:add-fmt-usage

Conversation

@dgellow
Copy link
Contributor

@dgellow dgellow commented Sep 29, 2020

Describe the pull request

What does your PR fix?

When fmt is installed from vcpkg, you get the output:

The package fmt:x64-windows provides CMake targets:

    find_package(fmt CONFIG REQUIRED)
    target_link_libraries(main PRIVATE fmt::fmt fmt::fmt-header-only)

Unfortunately that's not correct, we shouldn't link both fmt::fmt and the header only version, fmt::fmt-header-only.

This PR adds a usage file to instead show to the user the CMake statements from fmt own documentation: https://fmt.dev/latest/usage.html#usage-with-cmake.

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

I don't think anything has to be updated. Just tell me if I have to change something.

Does your PR follow the maintainer guide?

I believe so, yes.

Copy link
Contributor

@PhoebeHui PhoebeHui left a comment

Choose a reason for hiding this comment

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

Could you also update the 'Port-Version: 2' to ''Port-Version: 3' in CONTROL file?

@PhoebeHui PhoebeHui added category:port-bug The issue is with a library, which is something the port should already support requires:author-response labels Sep 30, 2020
@dgellow dgellow requested a review from PhoebeHui October 1, 2020 13:54
Copy link
Contributor

@ras0219-msft ras0219-msft left a comment

Choose a reason for hiding this comment

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

Hi @dgellow, thanks for the PR!

Did you verify that this actually achieves the change you wanted? The PR does not appear to deploy the usage file, so it will not affect the actual printed message in vcpkg.

The file needs to be deployed to share/fmt/usage.

@dgellow
Copy link
Contributor Author

dgellow commented Oct 7, 2020

Thanks for the review. The usage file should be copied to the correct location now, though I have issues testing it locally because of network timeouts errors when fetching mingw from MSYS2. They seem to face some problems with their servers.

I will try again later, hopefully that will be fixed.

image

image

@BillyONeal BillyONeal merged commit 54dbd5c into microsoft:master Oct 9, 2020
@BillyONeal
Copy link
Member

Thanks for your contribution!

@dgellow dgellow deleted the add-fmt-usage branch October 9, 2020 08:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

category:port-bug The issue is with a library, which is something the port should already support

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants