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

Use qGuiApp instead of qApp when working with a QGuiApplication #51

Merged
merged 1 commit into from
Feb 18, 2017
Merged

Use qGuiApp instead of qApp when working with a QGuiApplication #51

merged 1 commit into from
Feb 18, 2017

Conversation

Thra11
Copy link
Contributor

@Thra11 Thra11 commented Feb 18, 2017

qApp casts to QApplication, so it is only really valid when we're using
QApplication. In addition, QApplication and qApp are part of QtWidgets,
so using qApp adds an unnecessary dependency on QtWidgets to
QtQuick-only applications.

qApp casts to QApplication, so it is only really valid when we're using
QApplication. In addition, QApplication and qApp are part of QtWidgets,
so using qApp adds an unnecessary dependency on QtWidgets to
QtQuick-only applications.
@White-Oak
Copy link

we somehow need to fix a travis build

@filcuc
Copy link
Owner

filcuc commented Feb 18, 2017

I'm taking a look at it, but the patch is fine i cani merge It anyway

@filcuc
Copy link
Owner

filcuc commented Feb 18, 2017

@White-Oak @Thra11

Travis CI makes encrypted variables and data available only to pull requests coming from the same repository. These are considered trustworthy, as only members with write access to the repository can send them.

Pull requests sent from forked repositories do not have access to encrypted variables or data.

I'll look how can i expose securely the COVERALLS toke

@filcuc filcuc merged commit 2402c43 into filcuc:master Feb 18, 2017
@filcuc
Copy link
Owner

filcuc commented Feb 18, 2017

@White-Oak @Thra11 done there's no need to have a token when using coveralls from a public repository (ie. on github) :)

@filcuc
Copy link
Owner

filcuc commented Feb 19, 2017

Keep in mind that this doesn't break the QWidgets dependency because it's still linked during compilation. For solving this there're a couple of solutions. The neat one should be split the dotherside library in multiple shared libraries and let the bindings load them through dlopen/dlclose

@Thra11
Copy link
Contributor Author

Thra11 commented Feb 19, 2017

Yes, that's true. However, we have broken one dependency. If building a QtQuick application using a binding based on DOtherside (such as qml-rust), you still need to have QtWidgets present when you build DOtherSide, but the application binary itself won't depend on QtWidgets.

This could be handy, as you can e.g. build an application on a build machine which has QtWidgets, then run it on an embedded target which doesn't have QtWidgets. Of course in the long term it would be nice to split DOtherSide so it only builds against the Qt modules you have installed or are interested in using.

I originally came across this because I was trying to build a QtQuick application using qml-rust. I was getting linker errors because my application didn't include QtWidgets in its dependencies. With these changes, it now links without any problems, and running ldd on the resultant binary doesn't list QtWidgets as a library requirement.

@filcuc
Copy link
Owner

filcuc commented Feb 19, 2017

@Thra11 well more or less..the binary itself doesn't depends on QtWidgets but it depends on DOtherSide that depends on the QtWidgets..

This could be handy, as you can e.g. build an application on a build machine which has QtWidgets, then run it on an embedded target which doesn't have QtWidgets

Not really for the point i've already explained. On the machine you execute your project you still need to have the DOtherSide shared library thus you need to have QtWidgets.

Of course in the long term it would be nice to split DOtherSide so it only builds against the Qt modules you have installed or are interested in using.

Yeah i agree. @White-Oak how much work would be for you to split the binding in order to support multiple dll loaded through dlopen/LoadLibrary? Nevertheless i can split the DOtherSide library in multiple shared libraries DOtrherSideCore, DOtherSideWidgets etc. After this split the bindings authors have three choices:

  1. Dynamic link at compile time to all the libraries
  2. Lazy load the dll with dlopen, dlclose
  3. Split their bindings in multiple modules qml-rust-core qml-rust-widgets etc.

I think that option (2) is the neat one because it fails gradually and doesn't need you to link to all libraries and not split the bindings.

@filcuc
Copy link
Owner

filcuc commented Feb 19, 2017

Opened #52 for keeping track of this split

@Thra11
Copy link
Contributor Author

Thra11 commented Feb 19, 2017

@filcuc It looks like rust/qml-rust is statically linking DOtherSide, so presumably any unused symbols get stripped at link time, leaving the application free from QtWidgets dependencies as far as I can tell.

@filcuc
Copy link
Owner

filcuc commented Feb 19, 2017

@Thra11 Ah that makes sense

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.

None yet

3 participants