Skip to content

[libpng] Add file usage.txt to describe the usage of port libpng#23379

Closed
RookieTars wants to merge 1 commit intomicrosoft:masterfrom
RookieTars:patch-1
Closed

[libpng] Add file usage.txt to describe the usage of port libpng#23379
RookieTars wants to merge 1 commit intomicrosoft:masterfrom
RookieTars:patch-1

Conversation

@RookieTars
Copy link
Copy Markdown

Describe the pull request

  • What does your PR fix?

    Add usage.txt in directory ports/libpng to describe the usage of package libpng in CMake targets.

  • Have you updated the CI baseline?

    No

  • Does your PR follow the maintainer guide?

    Yes

Describe the usage of package libpng in CMake targets.
Comment on lines +4 to +5
target_link_libraries(main PRIVATE ${PNG_LIBRARY})
target_include_directories(main PRIVATE ${PNG_INCLUDE_DIR})
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

why? use targets? Isn't the automatic usage not good enough?

there is also cmakes FindPNG which can be used.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Do you mean find_package(PNG REQUIRED) ? It doesn't work on my machine.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

find_package(libpng REQUIRED)
target_link_libraries(${target_name} PRIVATE png)

This is how it's used in my project. I think this is also what vcpkg prints after install.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

for static builds it is png_static

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

@Osyotr You're right.I didn't pay attention to the info after install and couldn't find any information from this repo.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

@Neumann-A Sorry, I can't understand that. 😢 I'm still very new to CMake and vcpkg. But I found

find_package(PNG REQUIRED)
target_link_libraries(main PRIVATE PNG::PNG)

works on my machine. Is that the correct usage of FindPNG?

@Hoikas
Copy link
Copy Markdown
Contributor

Hoikas commented Mar 4, 2022

This port seems to already have a usage message

The package libpng provides CMake targets:

    find_package(libpng CONFIG REQUIRED)
    target_link_libraries(main PRIVATE png)

is this incorrect?

@RookieTars
Copy link
Copy Markdown
Author

@Hoikas That's right! But I couldn't find it.

@dg0yt
Copy link
Copy Markdown
Contributor

dg0yt commented Mar 5, 2022

The port doesn't have a usage file. The usage information is generated automatically (but seems to work).

I would still recommend the CMake find module if you don't want to depend on either vpckg or a recent enough libpng.

find_package(PNG REQUIRED)
target_link_libraries(main PRIVATE PNG::PNG) # since CMake 3.5

If this doesn't work, it is either wrong usage of vcpkg, or a bug.

@dg0yt
Copy link
Copy Markdown
Contributor

dg0yt commented Mar 5, 2022

    target_link_libraries(main PRIVATE png)

is this incorrect?

As pointed out by @Neumann-A, this is incorrect for static builds. Adding to #20190.

@JackBoosY JackBoosY added category:port-feature The issue is with a library, which is requesting new capabilities that didn’t exist invalid labels Mar 7, 2022
@PhoebeHui PhoebeHui changed the title Add file usage.txt to describe the usage of port libpng [libpng] Add file usage.txt to describe the usage of port libpng Apr 1, 2022
@dg0yt dg0yt mentioned this pull request Apr 17, 2022
@dg0yt
Copy link
Copy Markdown
Contributor

dg0yt commented Apr 19, 2022

There is now a usage file based on PNG.

@JackBoosY
Copy link
Copy Markdown
Contributor

There is now a usage file based on PNG.

Correct. So I'm going to close this PR since it's invalid.

@JackBoosY JackBoosY closed this Apr 19, 2022
@RookieTars
Copy link
Copy Markdown
Author

There is now a usage file based on PNG.

Appreciate that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

category:port-feature The issue is with a library, which is requesting new capabilities that didn’t exist invalid

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants