[qt5-base] Fix for feature postgresqlplugin on Windows #32880
Conversation
|
@microsoft-github-policy-service agree |
|
It seems I didn't really understand the version stuff correctly, because the git-tree SHA seems to be wrong in versions/q-/qt5-base.json, despite the hash being set by vcpkg x-add-version --all. It is btw. the same value as |
|
@jvbsl try |
|
@Osyotr nope, unfortunately that gave me exactly the same hash Edit:
|
|
I don't see how my change caused the arm64-android build to fail. Could someone perhaps restart that? To me it seems that is caused by some CI runner problems... |
FrankXie05
added
the
category:port-feature
|
/azp run |
|
Re-run CI. |
|
Azure Pipelines successfully started running 1 pipeline(s). |
| list(APPEND RELEASE_OPTIONS "PSQL_LIBS=${PSQL_RELEASE} ${PSQL_PORT_RELEASE} ${PSQL_COMMON_RELEASE} ${SSL_RELEASE} ${EAY_RELEASE} ${ADDITIONAL_WINDOWS_LIBS} -lwldap32") | ||
| list(APPEND DEBUG_OPTIONS "PSQL_LIBS=${PSQL_DEBUG} ${PSQL_PORT_DEBUG} ${PSQL_COMMON_DEBUG} ${SSL_DEBUG} ${EAY_DEBUG} ${ADDITIONAL_WINDOWS_LIBS} -lwldap32") |
There was a problem hiding this comment.
If this is a fix for Windows, shouldn't the option go to ADDITIONAL_WINDOWS_LIBS instead?
There was a problem hiding this comment.
I thought about that as well, but then it would be part of MYSQL_LIBS, despite MySQL not needing to be linked with that library, and I don't think it should then be part of the MYSQL_LIBS, and even less if it is used for the linking of the sqldrivers dlls of QT(Edit: which I think it is used for in the QT build, but I'm not sure), as they are independent of each other.
That is the reason why I did it kinda analogous to linux and mac:
vcpkg/ports/qt5-base/portfile.cmake
Line 291 in 60c7efb
Of course one could argue, that it isn't really the same, because there is no
ADDITIONAL_LINUX_LIBS variable, but it wasn't refactored to be like that yet. But I still think the reason above applies.
Does my reasoning make sense to you, or do you still think it should be part of ADDITIONAL_WINDOWS_LIBS, or perhaps even somewhere completely different?
There was a problem hiding this comment.
Sorry, I didn't notice that this is already guarded with VCPKG_TARGET_IS_WINDOWS. So it is okay as it is.
(I do think this should be done differently, based on pkgconfig, but this is too much for this simple PR.)
| list(APPEND RELEASE_OPTIONS "PSQL_LIBS=${PSQL_RELEASE} ${PSQL_PORT_RELEASE} ${PSQL_COMMON_RELEASE} ${SSL_RELEASE} ${EAY_RELEASE} ${ADDITIONAL_WINDOWS_LIBS} -lwldap32") | ||
| list(APPEND DEBUG_OPTIONS "PSQL_LIBS=${PSQL_DEBUG} ${PSQL_PORT_DEBUG} ${PSQL_COMMON_DEBUG} ${SSL_DEBUG} ${EAY_DEBUG} ${ADDITIONAL_WINDOWS_LIBS} -lwldap32") |
There was a problem hiding this comment.
Sorry, I didn't notice that this is already guarded with VCPKG_TARGET_IS_WINDOWS. So it is okay as it is.
(I do think this should be done differently, based on pkgconfig, but this is too much for this simple PR.)
|
Btw. should I squash the last two commits and force push? As the SHA hash change could've been done in one commit, at least I hope that wouldn't influence the hash, but as I don't exactly get where the hash comes from anyway I'm not sure. Apart from that do you think it is ready to be changed into a non-draft PR? |
|
Just mark as ready for review. |
|
Just wanted to thank you all for the help, the fast reviewing and merging. |
Fixes #30086.
SHA512s are updated for each updated downloadThe "supports" clause reflects platforms that may be fixed by this new versionAny fixed CI baseline entries are removed from that file.Any patches that are no longer applied are deleted from the port's directory../vcpkg x-add-version --alland committing the result.I hope the way I did my changes is correct. But after these changes it did build for me in a VM and in a github action.