Bug 1234209 - RFE: Support for Qt5
Summary: RFE: Support for Qt5
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: jdns
Version: rawhide
Hardware: All
OS: Linux
unspecified
medium
Target Milestone: ---
Assignee: Ivan Romanov
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: psi-plus 1253041
TreeView+ depends on / blocked
 
Reported: 2015-06-22 03:45 UTC by Raphael Groner
Modified: 2015-12-16 22:25 UTC (History)
2 users (show)

Fixed In Version:
Doc Type: Enhancement
Doc Text:
Clone Of:
Environment:
Last Closed: 2015-12-16 22:25:40 UTC


Attachments (Terms of Use)
cointstalling qt4 and qt5. api/abi is not break (7.32 KB, text/plain)
2015-07-08 04:44 UTC, Ivan Romanov
no flags Details
set QT_MULTI=OFF for Qt4 build, maintain api/abi (1.24 KB, patch)
2015-07-08 16:39 UTC, Rex Dieter
no flags Details | Diff


Links
System ID Priority Status Summary Last Updated
Red Hat Bugzilla 1292107 None None None Never

Internal Links: 1292107

Description Raphael Groner 2015-06-22 03:45:20 UTC
Description of problem:
Please provide support for Qt5.

Version-Release number of selected component (if applicable):
-

How reproducible:
-

Steps to Reproduce:
1. dnf install qjdns
2.
3.

Actual results:
Not possible to build for Qt5 with qjdns as a depedency.

Expected results:
Package builds fine with Qt5 and qjdns dependencies.

Additional info:
Possible name for a subpackage would be qjdns-qt5 as the usual convention suggests.

Comment 1 Ivan Romanov 2015-06-22 05:30:49 UTC
By fact now qjdns can be compiled with Qt5 need only to solve suffix troubles. I remember about qca...

Comment 3 Rex Dieter 2015-06-22 23:37:16 UTC
For now, I just recycle the same include dir for -qt5 build, since jdns didn't need suffixing.

Comment 4 Ivan Romanov 2015-06-23 04:05:02 UTC
Rex, you make me sad.

Comment 5 Ivan Romanov 2015-06-23 04:43:45 UTC
Seems here I can't use my way again? So?

Comment 6 Raphael Groner 2015-06-23 04:59:20 UTC
Did you file this patch to upstream?

Comment 7 Rex Dieter 2015-06-23 11:08:02 UTC
Ivan, what is "your way"?  We certainly can use any implementation, especially if you have better ideas.

Comment 8 Ivan Romanov 2015-07-06 03:51:31 UTC
Have a look at jdns-2.0.3 https://github.com/psi-im/jdns. Rion and me did such way and it upstreamable. 

There is MULTI_QT cmake cache variable. On Linux it's ON by default on other systems is OFF. With MULTI_QT enabled will be used '-qt4' or '-qt5'suffixes for library, .pc file and cmake config files and dir. Now I have 

$ rpm -ql qjdns-qt4 qjdns-qt4-devel qjdns-qt5 qjdns-qt5-devel
/usr/bin/jdns
/usr/lib64/libqjdns-qt4.so.2
/usr/lib64/libqjdns-qt4.so.2.0.3
/usr/include/jdns/qjdns.h
/usr/include/jdns/qjdnsshared.h
/usr/lib64/cmake/qjdns
/usr/lib64/cmake/qjdns-qt4
/usr/lib64/cmake/qjdns-qt4/QJDns-qt4Config.cmake
/usr/lib64/cmake/qjdns-qt4/QJDns-qt4ConfigVersion.cmake
/usr/lib64/cmake/qjdns-qt4/QJDns-qt4Targets-release.cmake
/usr/lib64/cmake/qjdns-qt4/QJDns-qt4Targets.cmake
/usr/lib64/cmake/qjdns/QJDnsConfig.cmake
/usr/lib64/cmake/qjdns/QJDnsConfigVersion.cmake
/usr/lib64/libqjdns-qt4.so
/usr/lib64/pkgconfig/qjdns-qt4.pc
/usr/lib64/libqjdns-qt5.so.2
/usr/lib64/libqjdns-qt5.so.2.0.3
/usr/include/jdns/qjdns.h
/usr/include/jdns/qjdnsshared.h
/usr/lib64/cmake/qjdns
/usr/lib64/cmake/qjdns-qt5
/usr/lib64/cmake/qjdns-qt5/QJDns-qt5Config.cmake
/usr/lib64/cmake/qjdns-qt5/QJDns-qt5ConfigVersion.cmake
/usr/lib64/cmake/qjdns-qt5/QJDns-qt5Targets-release.cmake
/usr/lib64/cmake/qjdns-qt5/QJDns-qt5Targets.cmake
/usr/lib64/cmake/qjdns/QJDnsConfig.cmake
/usr/lib64/cmake/qjdns/QJDnsConfigVersion.cmake
/usr/lib64/libqjdns-qt5.so
/usr/lib64/pkgconfig/qjdns-qt5.pc

Also pay attention that there is just QJDnsConfig.cmake (without suffix) which automagically choose correct version of qjdns. Without MULTI_QT any suffixes will not be used at all. For developers who want to use qjdns and who use cmake it's mean that they will do 

find_package(Qt4 REQUIRED QtCore QtNetwork)
find_package(QJdns REQUIRED)

or

find_package(Qt5Core REQUIRED)
find_package(Qt5Network REQUIRED)
find_package(QJdns REQUIRED)

in any case. No need to care about suffixes anymore.

I want to believe that thats will be good for all. Only one thing. Now I don't know how to correctly set Provides: tag for the new qjdns-qt4 package. 

I want to have qjdns-qt4 with 
/usr/lib64/libqjdns.so.2 -> /usr/lib64/libqjdns-qt4.so.2
and Provides:
libqjdns.so.2()(64bit)

Comment 9 Rex Dieter 2015-07-06 03:54:20 UTC
We'll probably want to maintain backward compatiblity for Qt4 builds, so only need to enable that renaming feature for Qt5 builds.

Comment 10 Ivan Romanov 2015-07-06 05:17:42 UTC
It will be good to have backwards compatibility. MULTI_QT should be enabled in both cases (Qt4 and Qt5) as I mentioned above. So how to just add symbolic link to new library name (library fully compatible with old one, only name was changed). I tried to do 'ln -s libqjdns-qt4.so.2 %{buildroot}/libqjdns.so.2' and add libqjdns.so.2 to %files section. But such library was not added to package provides. Psi requires libqjdns.so.2()(64bit) not libqjdns. So it is not enouch to add use Provides: libqjdns = ${version}-${release}

Comment 11 Rex Dieter 2015-07-06 11:43:12 UTC
Symlink is not good enough, the library soname would still be different and break abi.  API would break too.

Really, really, really, the Qt4 build should stay exactly the same, you only want changes made for the Qt5 build.

Comment 12 Rex Dieter 2015-07-06 11:49:09 UTC
To be more verbose, we want to maintain both api and abi compatibility for the Qt4 build.  That means preserving the same cmake/pkgconfig naming (api) as well as keeping the same library soname (abi).  This means we won't need to patch (or rebuild) any applications that currently use qjdns.  Otherwise... we will.  

Do you really want to impose api/abi breaks?  (I assume not)

Comment 13 Ivan Romanov 2015-07-06 13:05:15 UTC
What exactly means api and abi here? As I understand api and abi won't be changed.

Comment 14 Rex Dieter 2015-07-06 13:20:18 UTC
API => header names, pkgconfig names, cmake names.  Changing anything there changes/breaks api, and requires applications to adapt.

ABI => changing library soname, e.g. libqjdns.so.2 => libqjdns-qt4.so.2 , is a binary incompatible change.

Both are changing in your MULTI_QT scenario.

Comment 15 Ivan Romanov 2015-07-08 04:44:01 UTC
Created attachment 1049704 [details]
cointstalling qt4 and qt5. api/abi is not break

Comment 16 Ivan Romanov 2015-07-08 04:52:24 UTC
Here is used upstream source tarball of version 2.0.3. For build I use MULTI_QT scenario. To fix api/abi breaking I do:

1. Symbolink link qjdns.pc => qjdns-qt4.pc
2. Provides: libqjdns.so.2 for 32-bit architectures (i686 and arm). 
libqjdns.so.2()(64bit) for 32-bit architectures (x86_64 and ppc64).
3. Symbolink link libqjdns.so.2 => libqjdns-qt4.so.2

Rex, in Comment 11 you wrote:
Symlink is not good enough, the library soname would still be different and break abi.  API would break too.

I tested my 2.0.3 package. So with this package I cat start psi application and build psi package. All works fine. I don't see any problems.

Comment 17 Rex Dieter 2015-07-08 16:32:19 UTC
Hacks like this won't work reliably, I would recommend against it strongly.

# avoid abi breaking
%if %{__isa_bits} == 64
Provides:       libqjdns.so.2()(64bit)
%else
Provides:       libqjdns.so.2
%endif

I'll say it again, the simplest, most robust approach would be to not change the Qt4 build *at all*

Comment 18 Rex Dieter 2015-07-08 16:35:29 UTC
specifically, for Qt4 build, set MULTI_QT=OFF

Comment 19 Rex Dieter 2015-07-08 16:39:09 UTC
Created attachment 1049922 [details]
set QT_MULTI=OFF for Qt4 build, maintain api/abi

Minimalistic approach for 2.0.3 update against current fedora git.

If you insist, we can implement symlinks to provide compatibility with applications that expect QT_MULTI=ON

Comment 20 Rex Dieter 2015-07-08 16:50:44 UTC
My strongest recommendation to you as upstream:
* effectively implement QT_MULTI=ON unconditionally for Qt5 builds, and QT_MULTI=OFF unconditionally for Qt4 builds.

Keep it as an option if you insist, but seting that as the default should minimize church/breakage, reduce any possible confusion.

Comment 21 Ivan Romanov 2015-07-08 17:01:07 UTC
(In reply to Rex Dieter from comment #17)
> Hacks like this won't work reliably, I would recommend against it strongly.
> 
> # avoid abi breaking
> %if %{__isa_bits} == 64
> Provides:       libqjdns.so.2()(64bit)
> %else
> Provides:       libqjdns.so.2
> %endif
> 
> I'll say it again, the simplest, most robust approach would be to not change
> the Qt4 build *at all*

In which cases this won't work?

Comment 22 Rex Dieter 2015-07-08 17:08:10 UTC
It certainly works for some definition of "works".  

I only said it's unreliable and not robust: it relies on rpm internals not changing, and only handles (existing) known-values of %%__isa_bits

Why introduce such complications, when it can be handled simply with only minimal changes (see how small the .spec patch is in comment #19) ?

Comment 23 Ivan Romanov 2015-07-08 17:59:46 UTC
Sometimes needs to do some changes. Even if it's introduce some breakings. As I understand linuxes now provides two version of Qt (Qt4 and Qt5). So it will be good to have such names which allow to know which version Qt was linked to. Old libqjdns.so.2 is not allow this. So decision was made to rename it. Now we have libqjdns-qt4.so.2 and libqjdns-qt5.so.2. This is the best decision from that point of view. Also there is /usr/lib64/cmake/qjdns/QJDnsConfig.cmake in MULTI_QT which will conflict with the same name file when no MULTI_QT. It's mean that just use MULTI_QT=OFF is not enough.

Anyway how I mentioned above. MULTI_QT=OFF is not recommended for systems where both versions of Qt can be installed. 

I agree that my old sulution in QCA was not ideally. Furthemore I can said it was really bad idea to try use many suffixes for any cases. In that time when I only wanted to use always (with Qt4 or Qt5) find_package(Qca) in cmake build rules. So my goals:

1. When it's possible have nice-looking library names (qjdns.dll for example).
2. If not, have clear library names (libqjdns-qt4.so.2 and libqjdns-qt5.so.2).
3. Keep compatibilty with qjdns related soft in Fedora (now is jreen, iris and psi packages).
4. Use single name for cmake package (now QJDns).

It was my failing that I initially choose wrong name for qjdns library. But now is not too late to fix this. Then need to do only changes in 3 packages. It's not big work.

Comment 24 Rex Dieter 2015-07-08 18:36:45 UTC
I humbly disagree... but it's your package, so ultimately your decision.

go ahead and commit comment #15, if that's still your preference.

Comment 25 Fedora Update System 2015-07-09 06:24:54 UTC
jdns-2.0.3-1.fc22 has been submitted as an update for Fedora 22.
https://admin.fedoraproject.org/updates/jdns-2.0.3-1.fc22

Comment 26 Ivan Romanov 2015-07-09 06:29:01 UTC
I updated jdns.
Rex, Raphael please check that no any problems with new qjdns-qt4 and qjdns-qt4-devel packages and they fully compatible with old qjdns and qjdns-devel.

Comment 27 Fedora Update System 2015-07-13 19:12:49 UTC
Package jdns-2.0.3-1.fc22:
* should fix your issue,
* was pushed to the Fedora 22 testing repository,
* should be available at your local mirror within two days.
Update it with:
# su -c 'yum update --enablerepo=updates-testing jdns-2.0.3-1.fc22'
as soon as you are able to.
Please go to the following url:
https://admin.fedoraproject.org/updates/FEDORA-2015-11418/jdns-2.0.3-1.fc22
then log in and leave karma (feedback).

Comment 28 Raphael Groner 2015-07-19 22:42:29 UTC
(In reply to Ivan Romanov from comment #26)
> I updated jdns.
> Rex, Raphael please check that no any problems with new qjdns-qt4 and
> qjdns-qt4-devel packages and they fully compatible with old qjdns and
> qjdns-devel.

Build of psi for rawhide (fc24) works.

(In reply to Fedora Update System from comment #27)
Please do not push to f22 stable as we do not need an official Qt5 package currently and because of the discussion here about risky ABI/API breakage in Qt4 that will imply needed rebuilds of stable (and so far for good tested) packages. 

This request was an intention to go further with the separate psi-plus review that is senseful for Qt5 only but lacks also some other dependencies at the moment.

Comment 29 Raphael Groner 2015-07-29 12:16:50 UTC
As discussed again and agreed in IRC, the move of upstream to qt5 as default does not imply an update to F22 and 21. 

I am totally fine if this bug goes for F23+ only and it was reported for rawhide anyways. Backported Qt5 builds of qjdns are something for Copr if such are needed for both stable F22 and hardened F21, so we're without the risk to break any ABI, also with that (temporarily meant) symlinks solution.

Other thoughts? We would have to rebuild and test all depending packages in any other case to ensure there's no ABI breakage. That could propably be against Fedora's policy.

Comment 30 Ivan Romanov 2015-07-29 19:31:00 UTC
Ok. I unpushed update for F22 as you want.

Comment 31 Raphael Groner 2015-08-01 13:11:59 UTC
Thanks. I'll close this now to skip optionally additional work in QA. Please feel free to reopen if you think there have to be updates as well for both f22 and f21 with the general intention to have stable ABI.

Comment 32 Rex Dieter 2015-08-03 14:50:34 UTC
If we're doing this for f23+ only, I'd much prefer we drop the compatibility hacks

Any objections to doing that?

Comment 33 Rex Dieter 2015-08-03 14:51:34 UTC
On second thought, nevermind (by then, nothing in fedora should be affected, it'll only help 3rd-party software)

Comment 34 Rex Dieter 2015-11-02 17:26:09 UTC
see also bug #1253041  , possiblly something I warned may happen back in comment #17

Comment 35 Fedora Update System 2015-11-29 02:11:50 UTC
jdns-2.0.3-1.fc22.1 has been submitted as an update to Fedora 22. https://bodhi.fedoraproject.org/updates/FEDORA-2015-45dbf4003c

Comment 36 Raphael Groner 2015-11-29 10:30:25 UTC
We have to test this update for f22, see comment #35, with both psi-plus and psi packages.

Comment 37 Fedora Update System 2015-11-29 22:20:45 UTC
jdns-2.0.3-1.fc22.1 has been pushed to the Fedora 22 testing repository. If problems still persist, please make note of it in this bug report.
If you want to test the update, you can install it with
$ su -c 'dnf --enablerepo=updates-testing update jdns'
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2015-45dbf4003c

Comment 38 Fedora Update System 2015-12-12 01:54:46 UTC
jdns-2.0.3-1.fc22.1 has been pushed to the Fedora 22 stable repository. If problems still persist, please make note of it in this bug report.


Note You need to log in before you can comment on or make changes to this bug.