Skip to content

[giflib] Fix multi-config find_package(GIF)#27369

Merged
BillyONeal merged 4 commits intomicrosoft:masterfrom
dg0yt:giflib
Oct 28, 2022
Merged

[giflib] Fix multi-config find_package(GIF)#27369
BillyONeal merged 4 commits intomicrosoft:masterfrom
dg0yt:giflib

Conversation

@dg0yt
Copy link
Contributor

@dg0yt dg0yt commented Oct 21, 2022 •
edited
Loading

github-actions[bot]
github-actions bot previously approved these changes Oct 21, 2022
View reviewed changes
@LilyWangLL LilyWangLL self-assigned this Oct 21, 2022
@LilyWangLL LilyWangLL added the category:port-bug The issue is with a library, which is something the port should already support label Oct 21, 2022
github-actions[bot]
github-actions bot previously approved these changes Oct 21, 2022
View reviewed changes
@dg0yt dg0yt mentioned this pull request Oct 23, 2022
Draft
@LilyWangLL
Copy link
Contributor

LilyWangLL commented Oct 25, 2022

Usage test error:

1> [CMake] CMake Error at E:/Lily/giflib/installed/x64-windows/share/gif/vcpkg-cmake-wrapper.cmake:26 (_find_package):
1> [CMake]   Could not find a package configuration file provided by "gif" with any of
1> [CMake]   the following names:
1> [CMake] 
1> [CMake]     gifConfig.cmake
1> [CMake]     gif-config.cmake
1> [CMake] 
1> [CMake]   Add the installation prefix of "gif" to CMAKE_PREFIX_PATH or set "gif_DIR"
1> [CMake]   to a directory containing one of the above files.  If "gif" provides a
1> [CMake]   separate development package or SDK, be sure it has been installed.

@dg0yt
Copy link
Contributor Author

dg0yt commented Oct 25, 2022

Usage test error:

1> [CMake] CMake Error at E:/Lily/giflib/installed/x64-windows/share/gif/vcpkg-cmake-wrapper.cmake:26 (_find_package):
1> [CMake]   Could not find a package configuration file provided by "gif" with any of
1> [CMake]   the following names:
1> [CMake] 
1> [CMake]     gifConfig.cmake
1> [CMake]     gif-config.cmake
1> [CMake] 
1> [CMake]   Add the installation prefix of "gif" to CMAKE_PREFIX_PATH or set "gif_DIR"
1> [CMake]   to a directory containing one of the above files.  If "gif" provides a
1> [CMake]   separate development package or SDK, be sure it has been installed.

USER ERROR, NOT USAGE ERROR 😠

@Neumann-A
Copy link
Contributor

Neumann-A commented Oct 25, 2022

@LilyWangLL: It is find_package(GIF) not find_package(gif) ... case is important with find modules.

@dg0yt
Copy link
Contributor Author

dg0yt commented Oct 25, 2022 •
edited
Loading

Well, it is written in #26833 (comment), and in this PR's title.

@LilyWangLL
Copy link
Contributor

LilyWangLL commented Oct 25, 2022

@LilyWangLL: It is find_package(GIF) not find_package(gif) ... case is important with find modules.

I know, but I test this PR locally, after installing giflib, the output is:

Elapsed time to handle giflib:x64-windows: 10.37 s
Total elapsed time: 17.63 s

giflib provides CMake targets:

    # this is heuristically generated, and may not be correct
    find_package(gif CONFIG REQUIRED)
    target_link_libraries(main PRIVATE GIF::GIF)

@LilyWangLL
Copy link
Contributor

LilyWangLL commented Oct 25, 2022

I just clone the https://github.com/dg0yt/vcpkg.git and switch to branch dg0yt:giflib.

@LilyWangLL
Copy link
Contributor

LilyWangLL commented Oct 25, 2022 •
edited
Loading

And I also test the following usage in a new CMake project:

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

But the issue #26833 doesn't resolved, the link library still is debug lib.
image
Please let me know if there is a problem with my operation

@Neumann-A
Copy link
Contributor

Neumann-A commented Oct 25, 2022

Please let me know if there is a problem with my operation

Manifest mode?

@dg0yt
Copy link
Contributor Author

dg0yt commented Oct 25, 2022 

giflib provides CMake targets:

    # this is heuristically generated, and may not be correct
    find_package(gif CONFIG REQUIRED)
    target_link_libraries(main PRIVATE GIF::GIF)

#20190. There, I was increasing the size of the

Bad User Experience

headline until I run out of levels.

Would be fixed by #20190 (comment).

For now I can add a usage file to workaround the vpckg tool UX bug.

@LilyWangLL LilyWangLL added the depends:different-pr This PR or Issue depends on a PR which has been filed label Oct 25, 2022
@LilyWangLL
Copy link
Contributor

LilyWangLL commented Oct 25, 2022 •
edited
Loading

Please let me know if there is a problem with my operation

Manifest mode?

No, I test it in simple CMake project by integrate vcpkg. This property from the solution of CMake project.

@LilyWangLL LilyWangLL removed the depends:different-pr This PR or Issue depends on a PR which has been filed label Oct 25, 2022
@dg0yt
Copy link
Contributor Author

dg0yt commented Oct 25, 2022

Done.

@LilyWangLL
Copy link
Contributor

LilyWangLL commented Oct 26, 2022

And I also test the following usage in a new CMake project:

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

But the issue #26833 doesn't resolved, the link library still is debug lib. image Please let me know if there is a problem with my operation

This issue doesn't resolve.

@Neumann-A
Copy link
Contributor

Neumann-A commented Oct 26, 2022

This issue doesn't resolve.

delete your cache and reconfigure. Make sure the wrapper is actually called.

@dg0yt
Copy link
Contributor Author

dg0yt commented Oct 26, 2022

FTR this is the output from the multi-config testing in #27369:

-- Found GIF: optimized;D:/installed/x64-windows/lib/gif.lib;debug;D:/installed/x64-windows/debug/lib/gif.lib (found version "5.2.1")

@LilyWangLL
Copy link
Contributor

LilyWangLL commented Oct 27, 2022

I removed all caches and test again, issue #26833 resolved.

@LilyWangLL LilyWangLL added the info:needs-maintainer-attention Lets the current 'on rotation' vcpkg maintainer know they need to look at this. label Oct 27, 2022
@LilyWangLL LilyWangLL mentioned this pull request Oct 27, 2022
Closed
@BillyONeal BillyONeal merged commit acd359d into microsoft:master Oct 28, 2022
@BillyONeal
Copy link
Member

BillyONeal commented Oct 28, 2022

Thanks for the fix!

@dg0yt dg0yt deleted the giflib branch October 28, 2022 18:47
BillyONeal added a commit to BillyONeal/vcpkg that referenced this pull request Nov 1, 2022
@BillyONeal
When @BillyONeal started being the on-call vcpkg maintainer on 2022-1…
948a53d 
…0-17, he started applying use of the "embedded VERSION" feature microsoft/vcpkg-tool#717 to PRs on merge.

@dg0yt points out that this use should be accompanied by a call to vcpkg_minimum_required, in https://github.com/microsoft/vcpkg/pull/27594/files#r1010641672

This is an audit of everything merged in that time and whether it needs to gain that.

microsoft#27561 No VERSION
microsoft#27525 No VERSION
microsoft#27554 Already has vcpkg_minimum_required
microsoft#27536 No VERSION
microsoft#27562 No VERSION
microsoft#24914 Fixed here
microsoft#27547 No VERSION
microsoft#27502 No VERSION
microsoft#27497 No VERSION
microsoft#27317 No VERSION
microsoft#27288 No VERSION
microsoft#27509 No VERSION
microsoft#27504 No VERSION
microsoft#27514 No VERSION
microsoft#27501 No VERSION
microsoft#27495 No VERSION
microsoft#27488 No VERSION
microsoft#27499 No VERSION
microsoft#27378 No VERSION
microsoft#27376 Fixed here
microsoft#27373 No VERSION
microsoft#27045 No VERSION
microsoft#27521 No VERSION
microsoft#27453 No VERSION
microsoft#27481 No VERSION
microsoft#27511 No VERSION
microsoft#27490 No VERSION
microsoft#27510 No VERSION
microsoft#27496 No VERSION
microsoft#27503 No VERSION
microsoft#27485 No VERSION
microsoft#27484 No VERSION
microsoft#27483 No VERSION
microsoft#27459 No VERSION
microsoft#27369 No VERSION
microsoft#27489 No VERSION
microsoft#26594 No VERSION
microsoft#27465 No VERSION
microsoft#27456 No VERSION
microsoft#27425 No VERSION
microsoft#27464 Fixed here
microsoft#27406 No VERSION
microsoft#27398 No VERSION
microsoft#27240 No VERSION
microsoft#27450 No VERSION
microsoft#27463 No VERSION
microsoft#27462 No VERSION
microsoft#27448 No VERSION
microsoft#27440 No VERSION
microsoft#27435 No VERSION
microsoft#27424 No VERSION
microsoft#27414 No VERSION
microsoft#27412 No VERSION
microsoft#27380 No VERSION
microsoft#27343 No VERSION
microsoft#27342 No VERSION
microsoft#27367 No VERSION
microsoft#27226 No VERSION
microsoft#27320 No VERSION
microsoft#26923 No VERSION
microsoft#27284 No VERSION
microsoft#27433 No VERSION
microsoft#27314 VERSION got *removed*
microsoft#27335 No VERSION
microsoft#27370 No VERSION
microsoft#27324 No VERSION
microsoft#27391 No VERSION
microsoft#27388 No VERSION
microsoft#27396 No VERSION
microsoft#27404 No VERSION
microsoft#27413 No VERSION
microsoft#27417 No VERSION
microsoft#27427 No VERSION
microsoft#27428 No VERSION
microsoft#27368 No VERSION
microsoft#27307 No VERSION
microsoft#27415 Fixed here.
microsoft#27371 Fixed here.
microsoft#27323 No VERSION
microsoft#27352 No VERSION
microsoft#27347 No VERSION
microsoft#27366 No VERSION
microsoft#27361 No VERSION
microsoft#27359 No VERSION
microsoft#27358 No VERSION
microsoft#27355 No VERSION
microsoft#27331 No VERSION
microsoft#24615 No VERSION
microsoft#27325 No VERSION
microsoft#24861 No VERSION
microsoft#27354 No VERSION
microsoft#27346 No VERSION
microsoft#27345 No VERSION
microsoft#27218 No VERSION
microsoft#27329 No VERSION
microsoft#27326 No VERSION
microsoft#27321 No VERSION
microsoft#27312 No VERSION
microsoft#27297 No VERSION
microsoft#27336 No VERSION
microsoft#27225 No VERSION
microsoft#27339 No VERSION
microsoft#27302 No VERSION
microsoft#27295 No VERSION
microsoft#27233 No VERSION
microsoft#27313 No VERSION
microsoft#27237 No VERSION
microsoft#27250 No VERSION
microsoft#27263 No VERSION
microsoft#27266 No VERSION
microsoft#27272 No VERSION
microsoft#27287 No VERSION
microsoft#27282 No VERSION
microsoft#27294 No VERSION
microsoft#27228 No VERSION
microsoft#27163 No VERSION
microsoft#26817 No VERSION
microsoft#27286 No VERSION
microsoft#27274 No VERSION
microsoft#27276 No VERSION
microsoft#27232 No VERSION
microsoft#27221 No VERSION
microsoft#27215 No VERSION
microsoft#27166 No VERSION
microsoft#27239 No VERSION
microsoft#27246 No VERSION
microsoft#27268 No VERSION
microsoft#27259 No VERSION
microsoft#27238 No VERSION
microsoft#27224 No VERSION
microsoft#27203 No VERSION
microsoft#27124 No VERSION
@BillyONeal BillyONeal mentioned this pull request Nov 1, 2022
Merged
JavierMatosD pushed a commit that referenced this pull request Nov 8, 2022
@BillyONeal
Add vcpkg_minimum_required before using VERSION. (#27598)
e79c0d2 
* When @BillyONeal started being the on-call vcpkg maintainer on 2022-10-17, he started applying use of the "embedded VERSION" feature microsoft/vcpkg-tool#717 to PRs on merge.

@dg0yt points out that this use should be accompanied by a call to vcpkg_minimum_required, in https://github.com/microsoft/vcpkg/pull/27594/files#r1010641672

This is an audit of everything merged in that time and whether it needs to gain that.

#27561 No VERSION
#27525 No VERSION
#27554 Already has vcpkg_minimum_required
#27536 No VERSION
#27562 No VERSION
#24914 Fixed here
#27547 No VERSION
#27502 No VERSION
#27497 No VERSION
#27317 No VERSION
#27288 No VERSION
#27509 No VERSION
#27504 No VERSION
#27514 No VERSION
#27501 No VERSION
#27495 No VERSION
#27488 No VERSION
#27499 No VERSION
#27378 No VERSION
#27376 Fixed here
#27373 No VERSION
#27045 No VERSION
#27521 No VERSION
#27453 No VERSION
#27481 No VERSION
#27511 No VERSION
#27490 No VERSION
#27510 No VERSION
#27496 No VERSION
#27503 No VERSION
#27485 No VERSION
#27484 No VERSION
#27483 No VERSION
#27459 No VERSION
#27369 No VERSION
#27489 No VERSION
#26594 No VERSION
#27465 No VERSION
#27456 No VERSION
#27425 No VERSION
#27464 Fixed here
#27406 No VERSION
#27398 No VERSION
#27240 No VERSION
#27450 No VERSION
#27463 No VERSION
#27462 No VERSION
#27448 No VERSION
#27440 No VERSION
#27435 No VERSION
#27424 No VERSION
#27414 No VERSION
#27412 No VERSION
#27380 No VERSION
#27343 No VERSION
#27342 No VERSION
#27367 No VERSION
#27226 No VERSION
#27320 No VERSION
#26923 No VERSION
#27284 No VERSION
#27433 No VERSION
#27314 VERSION got *removed*
#27335 No VERSION
#27370 No VERSION
#27324 No VERSION
#27391 No VERSION
#27388 No VERSION
#27396 No VERSION
#27404 No VERSION
#27413 No VERSION
#27417 No VERSION
#27427 No VERSION
#27428 No VERSION
#27368 No VERSION
#27307 No VERSION
#27415 Fixed here.
#27371 Fixed here.
#27323 No VERSION
#27352 No VERSION
#27347 No VERSION
#27366 No VERSION
#27361 No VERSION
#27359 No VERSION
#27358 No VERSION
#27355 No VERSION
#27331 No VERSION
#24615 No VERSION
#27325 No VERSION
#24861 No VERSION
#27354 No VERSION
#27346 No VERSION
#27345 No VERSION
#27218 No VERSION
#27329 No VERSION
#27326 No VERSION
#27321 No VERSION
#27312 No VERSION
#27297 No VERSION
#27336 No VERSION
#27225 No VERSION
#27339 No VERSION
#27302 No VERSION
#27295 No VERSION
#27233 No VERSION
#27313 No VERSION
#27237 No VERSION
#27250 No VERSION
#27263 No VERSION
#27266 No VERSION
#27272 No VERSION
#27287 No VERSION
#27282 No VERSION
#27294 No VERSION
#27228 No VERSION
#27163 No VERSION
#26817 No VERSION
#27286 No VERSION
#27274 No VERSION
#27276 No VERSION
#27232 No VERSION
#27221 No VERSION
#27215 No VERSION
#27166 No VERSION
#27239 No VERSION
#27246 No VERSION
#27268 No VERSION
#27259 No VERSION
#27238 No VERSION
#27224 No VERSION
#27203 No VERSION
#27124 No VERSION

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

Reviewers

@github-actions github-actions[bot] github-actions[bot] approved these changes

+1 more reviewer

@LilyWangLL LilyWangLL LilyWangLL approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@LilyWangLL LilyWangLL

Labels

category:port-bug The issue is with a library, which is something the port should already support info:needs-maintainer-attention Lets the current 'on rotation' vcpkg maintainer know they need to look at this.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[giflib] Debug library linked in release configuration

4 participants

@dg0yt @LilyWangLL @Neumann-A @BillyONeal