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

[qt5-base] no absolute paths #21568

Merged
merged 1 commit into from
Nov 26, 2021

Conversation

autoantwort
Copy link
Contributor

For #20469

@LilyWangLL LilyWangLL added category:port-feature The issue is with a library, which is requesting new capabilities that didn’t exist info:reviewed Pull Request changes follow basic guidelines labels Nov 22, 2021
@BillyONeal
Copy link
Member

@Neumann-A Are you OK with this?

@Neumann-A
Copy link
Contributor

@Neumann-A Are you OK with this?

yeah but I don't guarantee that the pc files will be useable with that. Especially for static builds the pc files might be wrong or missing stuff.

@Neumann-A
Copy link
Contributor

also there are probably still absolute paths to the winsdk in the qt files.

@BillyONeal
Copy link
Member

yeah but I don't guarantee that the pc files will be useable with that. Especially for static builds the pc files might be wrong or missing stuff.

Do you think it's worth a shot anyway?

also there are probably still absolute paths to the winsdk in the qt files.

Probably :(

@Neumann-A
Copy link
Contributor

Do you think it's worth a shot anyway?

You can try but I there is currently nothing testing qt pc files. qmake uses the prl stuff and cmake uses the cmake configs. Only meson/autotools would possibly use pc files for qt.

@wrobelda
Copy link
Contributor

wrobelda commented Nov 23, 2021

You can try but I there is currently nothing testing qt pc files. qmake uses the prl stuff and cmake uses the cmake configs. Only meson/autotools would possibly use pc files for qt.

I assume the #14225 issue still stands, so they will remain unusable anyway.

@Neumann-A
Copy link
Contributor

I assume the #14225 issue still stands, so they will remain unusable anyway.

unless you use a single configuration triplet on osx?

@wrobelda
Copy link
Contributor

wrobelda commented Nov 23, 2021

unless you use a single configuration triplet on osx?

Well, the only Qt .pc files being installed are those with _debug suffix; there aren't any release copies of them.

EDIT: also, is this specific to macOS only?

@Neumann-A
Copy link
Contributor

Well, the only Qt .pc files being installed are those with _debug suffix; there aren't any release copies of them.

filelist for osx disagrees lib/pkgconfig has qt stuff.

EDIT: also, is this specific to macOS only?

windows: no pkgconfig files
linux: normal pkgconfig installation no _debug suffixes.

qt6 doesn't seem to install pkgconfig files at all

@wrobelda
Copy link
Contributor

Well, the only Qt .pc files being installed are those with _debug suffix; there aren't any release copies of them.

filelist for osx disagrees lib/pkgconfig has qt stuff.

That's what I have here:

Qt5Concurrent_debug.pc
Qt5Core_debug.pc
Qt5DBus_debug.pc
Qt5Designer_debug.pc
Qt5Gui_debug.pc
Qt5Help_debug.pc
Qt5MacExtras_debug.pc
Qt5Network_debug.pc
Qt5OpenGLExtensions_debug.pc
Qt5OpenGL_debug.pc
Qt5PrintSupport_debug.pc
Qt5QmlModels_debug.pc
Qt5QmlWorkerScript_debug.pc
Qt5Qml_debug.pc
Qt5QuickTest_debug.pc
Qt5QuickWidgets_debug.pc
Qt5Quick_debug.pc
Qt5Sql_debug.pc
Qt5Svg_debug.pc
Qt5Test_debug.pc
Qt5UiPlugin_debug.pc
Qt5UiTools_debug.pc
Qt5Widgets_debug.pc
Qt5XmlPatterns_debug.pc
Qt5Xml_debug.pc

@wrobelda
Copy link
Contributor

wrobelda commented Nov 23, 2021

@wrobelda: CI filelist here: https://dev.azure.com/vcpkg/public/_build/results?buildId=63427&view=artifacts&pathAsName=false&type=publishedArtifacts

Ah, my bad, had my find limited to /debug 🤦🏻‍♂️
Yeah, so in this case, single configuration triplet would work, but that's still suboptimal. Not to mention the fact that Windows build doesn't install any .pc at all, as you mentioned, so enabling Qt in non-CMake ports (like GpgMe) would be platform-dependent.

@autoantwort
Copy link
Contributor Author

I did this mainly so that microsoft/vcpkg-tool#172 can be merged. Imho this PR is a small improvement over the status quo. My intention was not to fix all pkgconfig issues of Qt

@wrobelda
Copy link
Contributor

I did this mainly so that microsoft/vcpkg-tool#172 can be merged. Imho this PR is a small improvement over the status quo. My intention was not to fix all pkgconfig issues of Qt

Yes, that's fair. We stole your thunder here a bit, our discussion belongs to #14225, sorry for that!

@BillyONeal
Copy link
Member

@Neumann-A @wrobelda I was mainly making sure that this wouldn't cause things to break horribly, not necessarily to make sure all pkgconfig ills of qt are fixed. :P From the feedback above this seems safe to merge.

@BillyONeal BillyONeal merged commit b722bcf into microsoft:master Nov 26, 2021
@BillyONeal
Copy link
Member

@autoantwort Thanks for the fix!
@Neumann-A @wrobelda Thanks for the review feedback!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:port-feature The issue is with a library, which is requesting new capabilities that didn’t exist info:reviewed Pull Request changes follow basic guidelines
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants