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

add qcustomplot/2.1.0 #6029

Merged
merged 10 commits into from
Mar 18, 2022
Merged

Conversation

SpaceIm
Copy link
Contributor

@SpaceIm SpaceIm commented Jun 22, 2021

Specify library name and version: lib/1.0

closes #5983

Test package fails at runtime when qt is static (but not if shared, except on Linux). If someone knows how to make it work.


  • I've read the guidelines for contributing.
  • I've followed the PEP8 style guides for Python code in the recipes.
  • I've used the latest Conan client version.
  • I've tried at least one configuration locally with the
    conan-center hook activated.

@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

@xakod
Copy link
Contributor

xakod commented Jul 6, 2021

Test package fails at runtime when qt is static (but not if shared, except on Linux). If someone knows how to make it work.

If you try to make gui app with conan-qt-static it will fail with the same problem

@ericLemanissier
Copy link
Contributor

@Jihadist can you try to use Q_IMPORT_PLUGIN (QXcbIntegrationPlugin) in one of your cpp files ?

@xakod
Copy link
Contributor

xakod commented Jul 6, 2021

@ericLemanissier nope because generated cmake files don't provide direct path to plugin directory like this bin\archdatadir\plugins\styles so you cannot use Q_IMPORT_PLUGIN macro. I stucked on linkage errors with this macro.

@ericLemanissier
Copy link
Contributor

can you please send the full log withQ_IMPORT_PLUGIN (QXcbIntegrationPlugin), so that we have a chance to understand and fix the linkage errors?

@AndreyMlashkin
Copy link
Contributor

@Jihadist, did it work for you for the old bincrafters recipe?

@SpaceIm
Copy link
Contributor Author

SpaceIm commented Jul 6, 2021

@ericLemanissier nope because generated cmake files don't provide direct path to plugin directory like this bin\archdatadir\plugins\styles so you cannot use Q_IMPORT_PLUGIN macro. I stucked on linkage errors with this macro.

I've worked on this 2 weeks ago, but it makes qt recipe very complex: https://github.com/SpaceIm/conan-center-index/blob/fix/qt6-components/recipes/qt/6.x.x/conanfile.py
And I don't know if plugins (which are static when qt is static) must be linked with whole archive or not.

Honestly, it would be easier to turn shared option of qt recipe to True by default. Do we really want to create qwt or qcustomplot pre built binaries with qt static?

@AndreyMlashkin
Copy link
Contributor

@SpaceIm, there is no PR for it yet?
I am using a static build of qt, it is the last thing that is missing for me to start using conan-center version of qt recipe instead of bincrafters one

@ericLemanissier
Copy link
Contributor

Honestly, it would be easier to turn shared option of qt recipe to True by default

+10, I asked this for years. I acknowledge that it comes with its own set of problems because static dependencies are then embedded in the shared library, and they cannot be overridden without rebuilding qt, but that's less bad than the current situation

@AndreyMlashkin
Copy link
Contributor

I am getting an error

undefined reference to `qt_static_plugin_QXcbIntegrationPlugin()'

Should I add something to CMakeLists to make it work?

@xakod
Copy link
Contributor

xakod commented Jul 8, 2021

I am getting an error

undefined reference to `qt_static_plugin_QXcbIntegrationPlugin()'

Should I add something to CMakeLists to make it work?

I mentioned it before #6029 (comment)
Try to add target_link_directories with path something similar to path_to_conan_qt\bin\archdatadir\plugins

@xakod
Copy link
Contributor

xakod commented Jul 28, 2021

I found the solution but maybe qt5/6 recipe should be a bit expanded. You can found the QXcbIntegrationPlugin solution here https://github.com/Jihadist/StaticQtPlugin/blob/master/CMakeLists.txt#L58 but it's dirty solution.
Briefly, we need to link against
res/archdatadir/plugins/platforms/libqxcb.a and lib/libQt{$Qt_MAJOR_VERSION}XcbQpa.a and then use Q_IMPORT_PLUGIN (QXcbIntegrationPlugin)

Telegram-desktop uses custom linkage scheme against static qt so they solved same problem https://github.com/desktop-app/cmake_helpers/blob/6602ed1ea085901fc5b556c4df8f6e3a0b7e52b3/external/qt/CMakeLists.txt#L234

@stale
Copy link

stale bot commented Sep 4, 2021

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Sep 4, 2021
@ericLemanissier
Copy link
Contributor

@SpaceIm can you retrigger this PR, to see if the changes made in qt recipe this summer improved the situation ?

@stale stale bot removed the stale label Sep 22, 2021
@SpaceIm SpaceIm closed this Sep 22, 2021
@SpaceIm SpaceIm reopened this Sep 22, 2021
@conan-center-bot

This comment has been minimized.

@xakod xakod mentioned this pull request Sep 30, 2021
4 tasks
@stale
Copy link

stale bot commented Oct 22, 2021

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Oct 22, 2021
@SpaceIm SpaceIm closed this Nov 16, 2021
@SpaceIm
Copy link
Contributor Author

SpaceIm commented Nov 16, 2021

Let's try again, I don't know whether qt recipe is fixed.

@ericLemanissier
Copy link
Contributor

Isn't it because the CI is headless ?
qt.qpa.xcb: could not connect to display

@SpaceIm
Copy link
Contributor Author

SpaceIm commented Mar 14, 2022

Isn't it because the CI is headless ?
qt.qpa.xcb: could not connect to display

Maybe, but why does it work in macOS & Windows agents?

@conan-center-bot

This comment has been minimized.

@SpaceIm
Copy link
Contributor Author

SpaceIm commented Mar 14, 2022

ConanException: Error 3221225781 while executing bin\test_package

some missing dll in Debug build of qt?
https://c3i.jfrog.io/c3i/misc/summary.html?json=https://c3i.jfrog.io/c3i/misc/logs/pr/6029/5-configs/windows-visual_studio/qcustomplot/2.1.0//summary.json

@SpaceIm
Copy link
Contributor Author

SpaceIm commented Mar 14, 2022

Isn't it because the CI is headless ?
qt.qpa.xcb: could not connect to display

Maybe, but why does it work in macOS & Windows agents?

I've tested on Ubuntu, and indeed test package runs fine, so issue in CI likely due to headless.

@conan-center-bot

This comment has been minimized.

@ericLemanissier
Copy link
Contributor

ericLemanissier commented Mar 15, 2022

yep, glib-2.0-0.dll
so, qt6 currently requires glib/2.70.1 which is always shared due to https://github.com/conan-io/conan-center-index/blob/master/recipes/glib/all/conanfile.py#L66
When building qcustomplot, I can see that it uses glib/2.71.3, which defaults to static. it explains why qt wants to link to a dll which is not present when running the test. Is there a best solution than simply rebuilding qt with up to date dependencies ?

@SpaceIm
Copy link
Contributor Author

SpaceIm commented Mar 15, 2022

glib is disabled in qt if not Linux/FreeBSD, but it's not disabled in harfbuzz. So I guess that #9739 indirectly broke current expectation in qt binaries.
But I still don't understand why there are errors in test package of qcustomplot only for windows/debug.

@ericLemanissier
Copy link
Contributor

The existing shared build of qt have an old version of harfbuzz statically linked, which linked to shared glib/2.70.1 (on windows).
It only happens on windows because of https://github.com/conan-io/conan-center-index/blob/master/recipes/glib/all/conanfile.py#L63. Other platform always have a static glib. Maybe it happens only in debug because compiler/linker optimization put the loading of shared glib in a path which is not hit any more at run-time?

@SpaceIm
Copy link
Contributor Author

SpaceIm commented Mar 15, 2022

There is no other solution than generating new packages of qt recipe.

@ericLemanissier
Copy link
Contributor

#9782

@SSE4
Copy link
Contributor

SSE4 commented Mar 15, 2022

if you can upgrade to new glib version, it may allow to move things forward (latest glib releases support static builds on Windows)

@ericLemanissier
Copy link
Contributor

the fact that default builds of windows glib are now static is the cause of the failure: every existing Windows binaries of all recipes (requiring glib) currently on conan-center are linking dynamically to glib, but the default glib is now static on windows, so basically all binaries requiring glib have to be regenerated, or there will be an error as soon as glib is overridden to a version >= 2.71.1

@ericLemanissier
Copy link
Contributor

qt 6 binaries have been fixed with #9782. Can we retry ?

@SSE4
Copy link
Contributor

SSE4 commented Mar 16, 2022

it seems like package_id model is either too flawed, or too strict, and it's hard to find the balance
as I understand, what happens, more or less, is:

  • we compile glib:shared=True
  • compile Qt against glib:shared=True
  • at this point, glib's option glib:shared=True is erased from the Qt's package id, unless it uses something like full_package_mode
  • compile QCustomPlot overriding glib:shared=False
  • a final binary of QCustomPlot still require glib2.dll, but it's not there, as conan thinks glib is static
  • but actually, compiled Qt library references glib2.dll, because it was compiled against shared glib before
  • there is no way to distinguish these things, as Qt package ID is the same regardless of glib:shared option

might be also worth checking conan-io/conan#9712
TLDR: Qt might need to add self.info.requires["glib"].full_package_mode() to its package_id

@ericLemanissier
Copy link
Contributor

I agree with all your message, except your last sentence Qt might need to add self.info.requires["glib"].full_package_mode() to its package_id
The issue has nothing to do with glib or qt recipe. It showed up on this PR because there was a change in the default value of glib:shared option, but it can happen any time a user changes the :shared value of any package (A) in the dependency tree of any recipe, if said package (A) is a requirement of another package B which has B:shared=True

@SpaceIm
Copy link
Contributor Author

SpaceIm commented Mar 16, 2022

yes, this why #8987 would be a solution to avoid these kind of issues

@SSE4
Copy link
Contributor

SSE4 commented Mar 16, 2022

yes, it's a generic problem, that couldn't be fixed in conan 1.x unfortunately, and it affects all the recipes (with at least one dependency with shared option). basically, when you specify xxx:shared=True/False you don't get it applied to dependencies, because it was erased from the package_id. it's terrible, but nothing to do except rebuilding Qt and other packages once it happens...

@conan-center-bot
Copy link
Collaborator

All green in build 7 (7dbab2eb5205809be7b6707e4d7a0ba8523169dc):

  • qcustomplot/2.1.0@:
    All packages built successfully! (All logs)

@conan-center-bot conan-center-bot merged commit fe03f97 into conan-io:master Mar 18, 2022
@SpaceIm SpaceIm deleted the new/qcustomplot/2.1.0 branch March 18, 2022 12:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[request] qcustomplot/2.1.0
7 participants