Skip to content

[vcpkg-baseline][zxing-cpp] Fix build failure#11586

Merged
dan-shaw merged 4 commits intomicrosoft:masterfrom
JackBoosY:dev/jack/fix_zxing-cpp_build
Jun 2, 2020
Merged

[vcpkg-baseline][zxing-cpp] Fix build failure#11586
dan-shaw merged 4 commits intomicrosoft:masterfrom
JackBoosY:dev/jack/fix_zxing-cpp_build

Conversation

@JackBoosY
Copy link
Contributor

@JackBoosY JackBoosY commented May 26, 2020 •
edited
Loading

When building zxing-cpp, it links to libiconv, which depends on libcharset, but it is not added to the link list.
So add vcpkg-cmake-wrapper.cmake to libiconv and add the find libcharset code.
Set opencv and iconv as features.

Related: #11502.

@JackBoosY JackBoosY added category:port-bug The issue is with a library, which is something the port should already support info:internal labels May 26, 2020
@Neumann-A
Copy link
Contributor

Neumann-A commented May 26, 2020

Since FindIconv is a cmake module shipped by cmake itself the correct solution would be to install a vcpkg-cmake-wrapper.cmake for iconv.

@JackBoosY JackBoosY marked this pull request as ready for review May 26, 2020 07:40
@JackBoosY
Copy link
Contributor Author

JackBoosY commented May 26, 2020

@Neumann-A Since libiconv now exports cmake configuration files, why not use it?

@JackBoosY JackBoosY requested a review from PhoebeHui May 26, 2020 08:46
@Neumann-A
Copy link
Contributor

Neumann-A commented May 26, 2020

Because it is unofficial and other cmake ports also depending on it don't benefit from it without the wrapper. In the future iconv might also be changed to use its native buildsystem instead of the custom vcpkg one

@JackBoosY
Copy link
Contributor Author

JackBoosY commented May 26, 2020 •
edited
Loading

@Neumann-A The FindIconv.cmake didn't find libcharset, so I think we should fix it in vcpkg-cmake-wrapper.cmake.

Neumann-A
Neumann-A reviewed May 26, 2020
View reviewed changes
ports/zxing-cpp/0002-improve-features.patch
@@ -0,0 +1,39 @@
diff --git a/CMakeLists.txt b/CMakeLists.txt
Copy link
Contributor

@Neumann-A Neumann-A May 26, 2020

Choose a reason for hiding this comment

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

You dont need this patch. Use -DCMAKE_DISABLE_FIND_PACKAGE_<package>=ON in the vcpkg_configure_cmake call instead (https://cmake.org/cmake/help/v3.0/variable/CMAKE_DISABLE_FIND_PACKAGE_PackageName.html)

@JackBoosY
Copy link
Contributor Author

JackBoosY commented May 27, 2020

Needs to move glib tombstone later,

@JackBoosY JackBoosY changed the title [zxing-cpp] Fix build failure [vcpkg-baseline][zxing-cpp] Fix build failure May 29, 2020
@JackBoosY JackBoosY mentioned this pull request May 29, 2020
Closed
@dan-shaw
Copy link
Contributor

dan-shaw commented Jun 1, 2020

@JackBoosY Is this PR complete?

@JackBoosY
Copy link
Contributor Author

JackBoosY commented Jun 2, 2020

@dan-shaw Yes, merge this plz.

@PhoebeHui PhoebeHui added the info:needs-maintainer-attention Lets the current 'on rotation' vcpkg maintainer know they need to look at this. label Jun 2, 2020
@dan-shaw dan-shaw merged commit 5a80a8e into microsoft:master Jun 2, 2020
@JackBoosY JackBoosY deleted the dev/jack/fix_zxing-cpp_build branch June 2, 2020 01:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@PhoebeHui PhoebeHui PhoebeHui approved these changes

+1 more reviewer

@Neumann-A Neumann-A Neumann-A left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@PhoebeHui PhoebeHui

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.

4 participants

@JackBoosY @Neumann-A @dan-shaw @PhoebeHui