Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[qtkeychain] Fix the CMake export target on windows #25018

Merged
merged 2 commits into from
Jun 2, 2022

Conversation

daschuer
Copy link
Contributor

@daschuer daschuer commented May 31, 2022

It turns out that the fix from #24013 that allows to use CMake < 3.18 does not work on windows. This adds a patch that reintroduces the alias solution but under a cmake version guard.
I have propose this also upstream frankosterfeld/qtkeychain#212, but did not yet get a responds since May 8th.

  • What does your PR fix?

It fixes the exported CMake target on windows

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

No change

Yes

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

Yes

@JonLiu1993 JonLiu1993 added the category:port-bug The issue is with a library, which is something the port should already support label Jun 1, 2022
@JonLiu1993
Copy link
Member

I tested the usage successfull :

1> CMake generation started for configuration: 'x64-Debug'.
1> Command line: "C:\WINDOWS\system32\cmd.exe" /c "%SYSTEMROOT%\System32\chcp.com 65001 >NUL && "C:\PROGRAM FILES (X86)\MICROSOFT VISUAL STUDIO\2019\ENTERPRISE\COMMON7\IDE\COMMONEXTENSIONS\MICROSOFT\CMAKE\CMake\bin\cmake.exe"  -G "Visual Studio 16 2019" -A x64  -DCMAKE_CONFIGURATION_TYPES:STRING="Debug" -DCMAKE_INSTALL_PREFIX:PATH="C:\Users\test\source\repos\CMakeProject42\out\install\x64-Debug" -DCMAKE_TOOLCHAIN_FILE=F:/Feature-test/qtkeychain/vcpkg/scripts/buildsystems/vcpkg.cmake "C:\Users\test\source\repos\CMakeProject42" 2>&1"
1> Working directory: C:\Users\test\source\repos\CMakeProject42\out\build\x64-Debug
1> [CMake] -- Selecting Windows SDK version 10.0.19041.0 to target Windows 10.0.19042.
1> [CMake] -- The C compiler identification is MSVC 19.29.30145.0
1> [CMake] -- The CXX compiler identification is MSVC 19.29.30145.0
1> [CMake] -- Detecting C compiler ABI info
1> [CMake] -- Detecting C compiler ABI info - done
1> [CMake] -- Check for working C compiler: C:/Program Files (x86)/Microsoft Visual Studio/2019/Enterprise/VC/Tools/MSVC/14.29.30133/bin/Hostx64/x64/cl.exe - skipped
1> [CMake] -- Detecting C compile features
1> [CMake] -- Detecting C compile features - done
1> [CMake] -- Detecting CXX compiler ABI info
1> [CMake] -- Detecting CXX compiler ABI info - done
1> [CMake] -- Check for working CXX compiler: C:/Program Files (x86)/Microsoft Visual Studio/2019/Enterprise/VC/Tools/MSVC/14.29.30133/bin/Hostx64/x64/cl.exe - skipped
1> [CMake] -- Detecting CXX compile features
1> [CMake] -- Detecting CXX compile features - done
1> [CMake] -- Configuring done
1> [CMake] -- Generating done
1> [CMake] -- Build files have been written to: C:/Users/test/source/repos/CMakeProject42/out/build/x64-Debug
1> Extracted CMake variables.
1> Extracted source files and headers.
1> Extracted code model.
1> Extracted toolchain configurations.
1> Extracted includes paths.
1> CMake generation finished

@JonLiu1993 JonLiu1993 added the info:reviewed Pull Request changes follow basic guidelines label Jun 1, 2022
@daschuer
Copy link
Contributor Author

daschuer commented Jun 1, 2022

The upstream PR has just been merged.
I think I will pick the merge commit on master to avoid the patch. The resulting source is identical.

@JonLiu1993 JonLiu1993 added requires:author-response and removed info:reviewed Pull Request changes follow basic guidelines labels Jun 1, 2022
@daschuer
Copy link
Contributor Author

daschuer commented Jun 1, 2022

This PR is still needed to pull the upstream changes in. But we can point to the particular comit instead of using a patch with the same content. I will prepare this tonight.

@daschuer
Copy link
Contributor Author

daschuer commented Jun 1, 2022

Done. @JonLiu1993 can you have another look? I have also updated the qt6 version.

@JonLiu1993 JonLiu1993 added the info:reviewed Pull Request changes follow basic guidelines label Jun 2, 2022
@JonLiu1993
Copy link
Member

LGTM, thanks for your pr!

Copy link
Contributor

@ras0219-msft ras0219-msft left a comment

Choose a reason for hiding this comment

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

This LGTM, thanks for the fix!

@ras0219-msft ras0219-msft merged commit abb073f into microsoft:master Jun 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:port-bug The issue is with a library, which is something the port should already support info:reviewed Pull Request changes follow basic guidelines
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants