[drogon] Fix MySQL finding by kotori2 · Pull Request #22378 · microsoft/vcpkg

kotori2 · 2022-01-05T13:04:54Z

Describe the pull request
#21359 changed paths for libmariadb, which makes drogon fails to find them

What does your PR fix?

The Mysql is not supported by drogon, please install the development library first. drogonframework/drogon#1146
Which triplets are supported/not supported? Have you updated the CI baseline?

Not touched, No
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

If you are still working on the PR, open it as a Draft: https://github.blog/2019-02-14-introducing-draft-pull-requests/

github-actions

This is a new experimental fast check for PR issues. Please let us know if this bot is helpful!

PRs must add only one version and must not modify any published versions

When making any changes to a library, the version or port-version in vcpkg.json or CONTROL must be modified.

Error: Local changes detected for drogon but no changes to version or port version.
-- Version: 1.7.4
-- Old SHA: 275d55a78bb61a79f0d66cd4f71e6b5892566666
-- New SHA: d7a1f5f19430546a7029c491986f87f9dcd954be
-- Did you remember to update the version or port version?
-- Pass `--overwrite-version` to bypass this check.
***No files were updated.***

After committing all other changes, the version database must be updated

git add -u && git commit
git checkout b934807c799fd84c2e1e168b8d3472d4904a6f90 -- versions
./vcpkg x-add-version --all

Diff

diff --git a/versions/d-/drogon.json b/versions/d-/drogon.json
index 566ebb7..d929cf6 100644
--- a/versions/d-/drogon.json
+++ b/versions/d-/drogon.json
@@ -1,10 +1,5 @@
 {
   "versions": [
-    {
-      "git-tree": "d7a1f5f19430546a7029c491986f87f9dcd954be",
-      "version-semver": "1.7.4",
-      "port-version": 1
-    },
     {
       "git-tree": "275d55a78bb61a79f0d66cd4f71e6b5892566666",
       "version-semver": "1.7.4",

github-actions

This is a new experimental fast check for PR issues. Please let us know if this bot is helpful!

After committing all other changes, the version database must be updated

git add -u && git commit
git checkout b934807c799fd84c2e1e168b8d3472d4904a6f90 -- versions
./vcpkg x-add-version --all

Diff

diff --git a/versions/d-/drogon.json b/versions/d-/drogon.json
index 0f2a526..c38c515 100644
--- a/versions/d-/drogon.json
+++ b/versions/d-/drogon.json
@@ -1,7 +1,7 @@
 {
   "versions": [
     {
-      "git-tree": "554d8b686b5422c8e2489f4877f638c02acb0d5f",
+      "git-tree": "d9488b60c0805941903425f6dfac74195d4cb6fb",
       "version-semver": "1.7.4",
       "port-version": 1
     },

strega-nil-ms · 2022-01-07T21:52:41Z

I really don't like this; I'd much prefer if we fixed libmariadb to support find_package(MySql). Marking requires:discussion so we can talk about it at the thursday meeting.

dg0yt · 2022-01-08T10:10:27Z

ports/drogon/libmariadb.patch

+-    find_package(MySQL)
+-    if (MySQL_FOUND)
+    find_package(unofficial-libmariadb CONFIG REQUIRED)
+    if (unofficial-libmariadb_FOUND)


A minimal patch would just take

# Find mysql, only mariadb client liberary is supported - find_package(MySQL) + find_package(unofficial-libmariadb CONFIG REQUIRED) + set(MySQL_FOUND TRUE) + set(MySQL_lib unofficial::libmariadb) if (MySQL_FOUND)

The REQUIRED implies that the build stops if unofficial-libmariadb isn't found.

A minimal patch would just take

# Find mysql, only mariadb client liberary is supported - find_package(MySQL) + find_package(unofficial-libmariadb CONFIG REQUIRED) + set(MySQL_FOUND TRUE) + set(MySQL_lib unofficial::libmariadb) if (MySQL_FOUND)

The REQUIRED implies that the build stops if unofficial-libmariadb isn't found.

I've tried set(MySQL_FOUND TRUE) before, but it will throw an error when you are syncing your own project since MySQL can't be found. Does set(MySQL_lib unofficial::libmariadb) solve this issue?

I've tried set(MySQL_FOUND TRUE) before, but it will throw an error when you are syncing your own project since MySQL can't be found.

I see.

Does set(MySQL_lib unofficial::libmariadb) solve this issue?

I cannot answer that without know the error message. "syncing your own project " isn't clear.
But it seems either your project shouldn't directly depend on MySQL_FOUND when MySQL actually isn't used, or it is implicitly relying on drogon's FindMySQL.cmake, and then you need to patch that script instead.

"syncing your own project " isn't clear.

Actually I mean generating Makefile from CMake after all vcpkg packages installed.

I cannot answer that without know the error message.

CMake will generate DrogonConfig.cmake in vcpkg_installed/[triplet]/share/drogon. It will call find_dependency(MySQL), which will fail since mariadb changed its name.

Also when trying your patch, I got build error.

FAILED: CMakeFiles/drogon.dir/orm_lib/src/mysql_impl/MysqlConnection.cc.o /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/c++ -DUSE_BROTLI -DUSE_OSSP_UUID=0 -I/Users/kotori0/vcpkg/buildtrees/drogon/src/v1.7.4-3e6a1930a1.clean/lib/inc -I/Users/kotori0/vcpkg/buildtrees/drogon/src/v1.7.4-3e6a1930a1.clean/orm_lib/inc -I/Users/kotori0/vcpkg/buildtrees/drogon/src/v1.7.4-3e6a1930a1.clean/nosql_lib/redis/inc -I/Users/kotori0/vcpkg/buildtrees/drogon/x64-osx-dbg -I/Users/kotori0/vcpkg/buildtrees/drogon/src/v1.7.4-3e6a1930a1.clean/trantor -I/Users/kotori0/vcpkg/buildtrees/drogon/x64-osx-dbg/exports -isystem /Users/kotori0/Downloads/test/cmake-build-debug/vcpkg_installed/x64-osx/include -isystem /Users/kotori0/Downloads/test/cmake-build-debug/vcpkg_installed/x64-osx/include/uuid -isystem /Library/Frameworks/Mono.framework/Headers -fPIC -g -arch x86_64 -isysroot /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX12.1.sdk -std=c++17 -MD -MT CMakeFiles/drogon.dir/orm_lib/src/mysql_impl/MysqlConnection.cc.o -MF CMakeFiles/drogon.dir/orm_lib/src/mysql_impl/MysqlConnection.cc.o.d -o CMakeFiles/drogon.dir/orm_lib/src/mysql_impl/MysqlConnection.cc.o -c /Users/kotori0/vcpkg/buildtrees/drogon/src/v1.7.4-3e6a1930a1.clean/orm_lib/src/mysql_impl/MysqlConnection.cc In file included from /Users/kotori0/vcpkg/buildtrees/drogon/src/v1.7.4-3e6a1930a1.clean/orm_lib/src/mysql_impl/MysqlConnection.cc:15: /Users/kotori0/vcpkg/buildtrees/drogon/src/v1.7.4-3e6a1930a1.clean/orm_lib/src/mysql_impl/MysqlConnection.h:25:10: fatal error: 'mysql.h' file not found #include <mysql.h> ^~~~~~~~~ 1 error generated.

MySQL_lib is a target, so my proposed set(MySQL_lib ...) can't help - sorry for the confusion. Your change to the target_link_libraries is already the right thing at this point.

Variable or not, when linking unofficial::libmariadb while building, the exported config will link to unofficial::libmariadb, too. So you need to replace the find_dependency(MySQL) with find_dependency(unofficial-libmariadb).

(@strega-nil-ms proposed to only patch FindMySQL.cmake. It is possible to handle the redirection entirely in that module, but it might be even more confusing when something goes wrong.)

I wanted to attach a test patch now, but I just run into another build issue with drogon. Either you try yourself again, or you wait a moment.

dg0yt · 2022-01-08T10:20:43Z

I really don't like this; I'd much prefer if we fixed libmariadb to support find_package(MySql). Marking requires:discussion so we can talk about it at the thursday meeting.

Note that FindMySQL.cmake is part of drogon. It takes a patch anyway.

dg0yt · 2022-01-11T06:24:34Z

This PR is obsolete now with #22426 merged. But many thanks for the initial work and discussion @kotori2.

JackBoosY · 2022-01-11T07:58:39Z

Can you please resolve the file conflict?

github-actions bot requested changes Jan 5, 2022

View reviewed changes

kotori2 force-pushed the drogon-fix branch 2 times, most recently from 1821015 to 6f94d62 Compare January 5, 2022 13:09

github-actions bot requested changes Jan 5, 2022

View reviewed changes

kotori2 added 2 commits January 5, 2022 21:13

[drogon] fix mysql not found

dcfef1c

update version

7f97e41

kotori2 force-pushed the drogon-fix branch from 6f94d62 to 7f97e41 Compare January 5, 2022 13:14

github-actions bot approved these changes Jan 5, 2022

View reviewed changes

JackBoosY assigned FrankXie05 Jan 5, 2022

JackBoosY added the category:port-bug The issue is with a library, which is something the port should already support label Jan 5, 2022

JackBoosY self-assigned this Jan 5, 2022

JackBoosY requested a review from FrankXie05 January 5, 2022 16:49

FrankXie05 approved these changes Jan 7, 2022

View reviewed changes

FrankXie05 added the info:needs-maintainer-attention Lets the current 'on rotation' vcpkg maintainer know they need to look at this. label Jan 7, 2022

strega-nil-ms added requires:discussion and removed info:needs-maintainer-attention Lets the current 'on rotation' vcpkg maintainer know they need to look at this. labels Jan 7, 2022

dg0yt suggested changes Jan 8, 2022

View reviewed changes

dg0yt mentioned this pull request Jan 8, 2022

[drogon] Fix libmariadb import/export and usage #22426

Merged

JackBoosY added the requires:author-response label Jan 11, 2022

kotori2 closed this Jan 11, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[drogon] Fix MySQL finding#22378

[drogon] Fix MySQL finding#22378
kotori2 wants to merge 2 commits intomicrosoft:masterfrom
kotori2:drogon-fix

kotori2 commented Jan 5, 2022

Uh oh!

github-actions bot left a comment

Uh oh!

github-actions bot left a comment

Uh oh!

strega-nil-ms commented Jan 7, 2022

Uh oh!

dg0yt Jan 8, 2022

Uh oh!

kotori2 Jan 8, 2022

Uh oh!

dg0yt Jan 8, 2022

Uh oh!

kotori2 Jan 8, 2022

Uh oh!

dg0yt Jan 8, 2022

Uh oh!

dg0yt commented Jan 8, 2022

Uh oh!

dg0yt commented Jan 11, 2022

Uh oh!

JackBoosY commented Jan 11, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Conversation

kotori2 commented Jan 5, 2022

What does your PR fix?

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

Does your PR follow the maintainer guide?

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

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Uh oh!

strega-nil-ms commented Jan 7, 2022

Uh oh!

dg0yt Jan 8, 2022

Choose a reason for hiding this comment

Uh oh!

kotori2 Jan 8, 2022

Choose a reason for hiding this comment

Uh oh!

dg0yt Jan 8, 2022

Choose a reason for hiding this comment

Uh oh!

kotori2 Jan 8, 2022

Choose a reason for hiding this comment

Uh oh!

dg0yt Jan 8, 2022

Choose a reason for hiding this comment

Uh oh!

dg0yt commented Jan 8, 2022

Uh oh!

dg0yt commented Jan 11, 2022

Uh oh!

JackBoosY commented Jan 11, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

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