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.

Sign up for GitHub

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

Jump to bottom

[qhttpengine] no absolute paths #22328

Merged
merged 1 commit into from
Mar 21, 2022
Merged

[qhttpengine] no absolute paths #22328

merged 1 commit into from
Mar 21, 2022

Conversation

autoantwort
Copy link
Contributor

For #20469

@autoantwort autoantwort force-pushed the qhttpengine-no-absolute branch from b0eaf45 to 6cedf8e Compare January 4, 2022 12:36
@JackBoosY JackBoosY added the category:port-bug The issue is with a library, which is something the port should already support label Jan 4, 2022
@JackBoosY JackBoosY requested a review from FrankXie05 January 5, 2022 16:49
FrankXie05
FrankXie05 reviewed Jan 7, 2022
View reviewed changes
ports/qhttpengine/portfile.cmake
@@ -17,6 +17,11 @@ vcpkg_cmake_configure(
vcpkg_cmake_install()
vcpkg_copy_pdbs()
vcpkg_cmake_config_fixup(CONFIG_PATH lib/cmake)
if(VCPKG_TARGET_IS_WINDOWS)
Copy link
Contributor

Choose a reason for hiding this comment

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

Why specify windows？

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because qt does not provide pkgconfig files on windows.

Copy link
Contributor

Choose a reason for hiding this comment

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

clearly they do, since you're deleting the directories? this seems very wrong...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No official QT5 package provides pkgconfig files on windows. This port does it (not a official qt package), but the ports this ports depends on does not provide pkgconfig files.

Copy link
Contributor

Choose a reason for hiding this comment

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

I need @Neumann-A double check this point.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@JackBoosY Here you find the content of the vcpkg_installed after installing qt5. You can search for qt5 pkgconfig files but there are none on windows.

Copy link
Contributor

Choose a reason for hiding this comment

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

@strega-nil-ms We have to wait for qtbase to support exporting pkgconfig before we can completely fix the pc file provided by this port. I think this change is acceptable to us now. What's your opinion?

Copy link
Contributor

Choose a reason for hiding this comment

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

Ping @strega-nil-ms for reply.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ping @strega-nil-ms for review again

Copy link
Member

Choose a reason for hiding this comment

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

Confirmed:

Total elapsed time: 27.57 min

The package qhttpengine provides CMake targets:

    find_package(qhttpengine CONFIG REQUIRED)
    target_link_libraries(main PRIVATE qhttpengine)

PS C:\Dev\vcpkg> cmd /c dir /s .\installed\x86-windows\*.pc
 Volume in drive C has no label.
 Volume Serial Number is 5EE8-E2C2

 Directory of C:\Dev\vcpkg\installed\x86-windows\debug\lib\pkgconfig

03/21/2022  11:41 AM               225 bzip2.pc
03/21/2022  11:41 AM               374 freetype2.pc
03/21/2022  11:43 AM               267 harfbuzz-subset.pc
03/21/2022  11:43 AM               259 harfbuzz.pc
03/21/2022  11:51 AM             1,391 icu-i18n.pc
03/21/2022  11:51 AM             1,385 icu-io.pc
03/21/2022  11:51 AM             1,418 icu-uc.pc
03/21/2022  11:41 AM               297 libbrotlicommon.pc
03/21/2022  11:41 AM               325 libbrotlidec.pc
03/21/2022  11:41 AM               325 libbrotlienc.pc
03/21/2022  11:53 AM               293 libcrypto.pc
03/21/2022  11:52 AM               269 libjpeg.pc
03/21/2022  11:54 AM               373 libpcre2-16.pc
03/21/2022  11:54 AM               373 libpcre2-32.pc
03/21/2022  11:54 AM               370 libpcre2-8.pc
03/21/2022  11:54 AM               339 libpcre2-posix.pc
03/21/2022  11:41 AM               288 libpng.pc
03/21/2022  11:41 AM               288 libpng16.pc
03/21/2022  11:53 AM               293 libssl.pc
03/21/2022  11:52 AM               281 libturbojpeg.pc
03/21/2022  12:01 PM               472 libzstd.pc
03/21/2022  11:53 AM               246 openssl.pc
03/21/2022  12:01 PM               301 sqlite3.pc
03/21/2022  11:41 AM               294 zlib.pc
              24 File(s)         10,746 bytes

 Directory of C:\Dev\vcpkg\installed\x86-windows\lib\pkgconfig

03/21/2022  11:41 AM               221 bzip2.pc
03/21/2022  11:41 AM               370 freetype2.pc
03/21/2022  11:43 AM               264 harfbuzz-subset.pc
03/21/2022  11:43 AM               256 harfbuzz.pc
03/21/2022  11:51 AM             1,367 icu-i18n.pc
03/21/2022  11:51 AM             1,361 icu-io.pc
03/21/2022  11:51 AM             1,393 icu-uc.pc
03/21/2022  11:41 AM               294 libbrotlicommon.pc
03/21/2022  11:41 AM               322 libbrotlidec.pc
03/21/2022  11:41 AM               322 libbrotlienc.pc
03/21/2022  11:53 AM               290 libcrypto.pc
03/21/2022  11:52 AM               266 libjpeg.pc
03/21/2022  11:54 AM               369 libpcre2-16.pc
03/21/2022  11:54 AM               369 libpcre2-32.pc
03/21/2022  11:54 AM               366 libpcre2-8.pc
03/21/2022  11:54 AM               335 libpcre2-posix.pc
03/21/2022  11:41 AM               283 libpng.pc
03/21/2022  11:41 AM               283 libpng16.pc
03/21/2022  11:53 AM               290 libssl.pc
03/21/2022  11:52 AM               278 libturbojpeg.pc
03/21/2022  12:01 PM               469 libzstd.pc
03/21/2022  11:53 AM               243 openssl.pc
03/21/2022  12:01 PM               298 sqlite3.pc
03/21/2022  11:41 AM               290 zlib.pc
              24 File(s)         10,599 bytes

     Total Files Listed:
              48 File(s)         21,345 bytes
               0 Dir(s)  3,095,669,248,000 bytes free
PS C:\Dev\vcpkg>

@FrankXie05 FrankXie05 added the info:reviewed Pull Request changes follow basic guidelines label Jan 7, 2022
@strega-nil-ms strega-nil-ms added requires:author-response and removed info:reviewed Pull Request changes follow basic guidelines labels Jan 7, 2022
@JackBoosY JackBoosY added the info:reviewed Pull Request changes follow basic guidelines label Mar 18, 2022
@FrankXie05 FrankXie05 added the requires:vcpkg-team-review This PR or issue requires someone on the vcpkg team to take a further look. label Mar 18, 2022
@BillyONeal BillyONeal merged commit bca99c4 into microsoft:master Mar 21, 2022
@BillyONeal
Copy link
Member

Thanks for your contribution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@BillyONeal BillyONeal BillyONeal left review comments

@JackBoosY JackBoosY JackBoosY left review comments

@FrankXie05 FrankXie05 FrankXie05 approved these changes

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

@strega-nil-ms strega-nil-ms Awaiting requested review from strega-nil-ms

Assignees

@JackBoosY JackBoosY

@FrankXie05 FrankXie05

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 requires:vcpkg-team-review This PR or issue requires someone on the vcpkg team to take a further look.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@autoantwort @BillyONeal @JackBoosY @FrankXie05 @strega-nil-ms