Skip to content

Use qtpaths in build.go to locate Qt plugins. #56

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

dwarfcrank
Copy link
Contributor

Depending on the system, Qt plugins may not be at ../../plugins relative to qmake. This is the case on Arch Linux at least, where qmake is at /usr/bin/qmake but Qt plugins are at /usr/lib/qt/plugins. This patch uses the qtpaths tool to find the right plugin directory.

qtpaths doesn't seem to be included in the Qt5 base packages but in an additional tools package instead (e.g. qttools5-dev-tools on Ubuntu, qt5-tools on Arch). I'm not sure about the situation on Windows, so I limited the use of qtpaths to Linux builds.

A minor issue is that the chrpath command fails for me on Arch due to libqxcb.so lacking an RPATH entry. This causes the run command fail on the first run, but re-running it works as the chrpath invocation is skipped.

Another build-related issue on Arch is caused by the hardcoded versions of libicu*.so (the script uses .56 but I only have .59). I'm not sure of a proper way to find the paths for those libraries, so I've just crudely replaced those in my local copy of the build script.

@cretz
Copy link
Owner

cretz commented Sep 22, 2017

This is for distro packaged Qt, right? I based my build on the Qt installer from their website because I always build with the latest (5.9.1 right now, installed at ~/Qt-5.9.1 IIRC). I specifically do this because most distro packages are behind. I also copy and ship all Qt resources I need out of the common install instead of using the system level libs (at runtime AND compile time). Because I use nothing security critical and want the latest, I think this kind of "vendoring" is worth it.

Fair? Should I make it more clear in my build instructions that this should not use the latest distro packaging but instead the latest install from qt.io?

@dwarfcrank
Copy link
Contributor Author

Yeah, this is for distro packaged Qt. In my case, 5.9.1 on Arch.

I pretty much only use Arch which usually has the latest version of everything, so I kinda forgot that most distros are indeed quite a bit behind. But yeah, would be good to have a note in the build instructions regarding Qt versions.

@terinjokes
Copy link

On Arch (where I am as well) I have a preference to use the distro packages if possible. I understand the situation on other distros.

@cretz
Copy link
Owner

cretz commented Sep 22, 2017

I'd accept changes to the PR that made it conditional. Either fall back to this way, use an env var (for either a boolean aying you are using system lib, or a Qt plugin path override), or intelligent detection (but not just detection that the system package is installed).

Otherwise, I was just gonna update the build docs to say regardless of distro, use qt.io installer.

@dwarfcrank
Copy link
Contributor Author

I'd probably go for the environment variable, making it a bit more explicit than a fallback.

So if something like USE_SYSTEM_QT is set to

  • 1: use qtpaths (and possibly skip chrpath or just ignore if it fails)
  • 0 or not set: use <qmake>/../../plugins.

Does that sound good?

@cretz
Copy link
Owner

cretz commented Sep 23, 2017

Wouldn't want to skip chrpath because that's for runtime. Unless you want runtime to use system libs which is a different conversation because we wouldn't copy any of the so files or set the plugin path at app start.

@terinjokes
Copy link

terinjokes commented Sep 24, 2018

For the ArchLinux PKGBUILD I've got locally (haven't published it, pending this conversation).

My patches to build.go were getting pretty ridiculous, so I've stopped using it in favor of calling qmake and make directly.

When packaging, I've created symlinks to the system installations of Qt5 and CEF components. It looks to me like the symlinks for Qt5 wouldn't be needed if we weren't calling QCoreApplication::setLibraryPaths (I'm not a Qt expert, but I suspect that then means we'll have to create a QCoreApplication).


Edit: It's not clear from the above, but I'm fine with making CEF part of the package for doogie, since your releases are highly coupled to them (I'm using system versions mostly because of a mistake I made). I do believe, at least on Arch, that we should use system versions for Qt.

@cretz
Copy link
Owner

cretz commented Sep 25, 2018

@terinjokes - Sounds good. I'm down with making locally-referenced Qt optional and instead using system Qt. How about we take the approach in this PR and just make it conditional like I mentioned above with an env var or a flag?

As for runtime, such as symlinks and QCoreApplication::setLibraryPaths and what not, I think we can just easily reference the same Qt it was built with assuming it was built on the same system it's running on. To distribute a no-Qt-included binary is a bit more difficult due to the shared libs being in different places on each system. I think just asking people to set LD_LIBRARY_PATH before running may be the best approach as mentioned at http://doc.qt.io/qt-5/linux-deployment.html#creating-the-application-package. QCoreApplication::setLibraryPaths is more for plugins and extras (you have to have loaded the .so to even be able to call QCoreApplication). As a first step, I say we just make it build/run w/out copying libs or changing rpath for people wanting system libs. Then we can look to making a distribution w/ a shell script or something.

I am doing some other things, so I won't get to it for a little bit.

@terinjokes
Copy link

No worries, I know OSS is a labor of love.

I'll test out the changes from this PR, and see if it improves the situation.

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.

3 participants