Note: This is a beta release of Red Hat Bugzilla 5.0. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Also email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback here.
Bug 915005 (qt5-qttools) - Review Request: qt5-qttools - Qt5 - QtTool components
Summary: Review Request: qt5-qttools - Qt5 - QtTool components
Keywords:
Status: CLOSED RAWHIDE
Alias: qt5-qttools
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Gregor Tätzner
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On: qt5-qtbase, qt5-qtbase-review 998477
Blocks: kde-reviews qt-reviews sigil-07 qt5-qtquick1
TreeView+ depends on / blocked
 
Reported: 2013-02-24 01:00 UTC by Rex Dieter
Modified: 2013-09-10 17:54 UTC (History)
5 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2013-09-03 12:07:03 UTC
gregor: fedora-review+
gwync: fedora-cvs+


Attachments (Terms of Use)
pre-review (deleted)
2013-04-21 09:04 UTC, Gregor Tätzner
no flags Details
diff between bundled clucene and fedora clucene09 (deleted)
2013-08-18 08:37 UTC, Dan Horák
no flags Details | Diff
use system clucene (deleted)
2013-08-18 11:37 UTC, Dan Horák
no flags Details | Diff
proposed fix (deleted)
2013-08-18 21:08 UTC, Dan Horák
no flags Details | Diff
rawhide mock build fail (deleted)
2013-08-27 17:28 UTC, Gregor Tätzner
no flags Details
review (deleted)
2013-08-27 19:33 UTC, Gregor Tätzner
no flags Details

Description Rex Dieter 2013-02-24 01:00:52 UTC
Spec URL: http://rdieter.fedorapeople.org/rpms/qt5/qt5-qttools.spec
SRPM URL: http://rdieter.fedorapeople.org/rpms/qt5/qt5-qttools-5.0.1-1.fc18.src.rpm
Description: Qt5 - QtTool components
Fedora Account System Username: rdieter

Comment 1 Gregor Tätzner 2013-02-26 10:16:16 UTC
Missing BuildReq: mesa-libGL-devel, zlib-devel

static lib in devel package: /usr/lib64/libQt5UiTools.a
make it shared or move it to -static
http://fedoraproject.org/wiki/Packaging:Guidelines#Packaging_Static_Libraries

lots of hidden files in debuginfo (.moc and .rcc), please check that

SHOULD:

for loop in %install:
please replace those ../ cascades with buildroot/bindir, it's not very readable

macro in comment:
# put non-conflicting binaries with -qt5 postfix in %{_bindir}

Comment 2 Rex Dieter 2013-02-26 14:43:27 UTC
1.  missing BuildReq ...

actually, I'm adding those as deps to qt5-qtbase-devel and qt5-qtbase-static, but adding them here in the meantime won't hurt.


2.  lots of hidden files in debuginfo (.moc and .rcc), please check that

harmless, have bigger things to worry about. :)


3.  please replace those ../ cascades with buildroot/bindir, it's not very readable

I'd rather not, relative links are much nicer than absolute ones for various reasons.


4.  macro in comment

again, this one's mostly harmless, and valid as-is imho.  (%configure and ones like it are the ones to worry about).

Comment 3 Rex Dieter 2013-02-26 17:06:19 UTC
Spec URL: http://rdieter.fedorapeople.org/rpms/qt5/qt5-qttools.spec
SRPM URL: http://rdieter.fedorapeople.org/rpms/qt5/qt5-qttools-5.0.1-2.fc18.src.rpm

%changelog
* Mon Feb 25 2013 Rex Dieter <rdieter@fedoraproject.org> 5.0.1-2
- BR: pkgconfig(zlib)
- -static subpkg

Comment 4 Ismael Olea 2013-04-10 19:11:19 UTC
Got an error building this: https://gist.github.com/anonymous/5357548

Comment 5 Gregor Tätzner 2013-04-11 20:23:16 UTC
(In reply to comment #4)
> Got an error building this: https://gist.github.com/anonymous/5357548

qttools 5.0.2 builds fine against qtbase 5.0.2

rdieter can you update the spec and srpm references.

rpmlint output:
qt5-qttools.src:51: W: macro-in-comment %{_bindir}
qt5-qttools.x86_64: W: no-documentation
qt5-qttools.x86_64: W: no-manual-page-for-binary assistant-qt5
qt5-qttools.x86_64: W: no-manual-page-for-binary qdbusviewer-qt5
qt5-qttools.x86_64: W: no-manual-page-for-binary qdbus-qt5
qt5-qttools-debuginfo.x86_64: W: hidden-file-or-dir /usr/src/debug/qttools-opensource-src-5.0.2/src/assistant/assistant/.uic
qt5-qttools-debuginfo.x86_64: W: hidden-file-or-dir /usr/src/debug/qttools-opensource-src-5.0.2/src/assistant/assistant/.uic
qt5-qttools-debuginfo.x86_64: W: hidden-file-or-dir /usr/src/debug/qttools-opensource-src-5.0.2/src/designer/src/components/lib/.rcc
qt5-qttools-debuginfo.x86_64: W: hidden-file-or-dir /usr/src/debug/qttools-opensource-src-5.0.2/src/designer/src/components/lib/.rcc
qt5-qttools-debuginfo.x86_64: W: hidden-file-or-dir /usr/src/debug/qttools-opensource-src-5.0.2/src/designer/src/designer/.uic
qt5-qttools-debuginfo.x86_64: W: hidden-file-or-dir /usr/src/debug/qttools-opensource-src-5.0.2/src/designer/src/designer/.uic
qt5-qttools-debuginfo.x86_64: W: hidden-file-or-dir /usr/src/debug/qttools-opensource-src-5.0.2/src/assistant/qcollectiongenerator/.moc
qt5-qttools-debuginfo.x86_64: W: hidden-file-or-dir /usr/src/debug/qttools-opensource-src-5.0.2/src/assistant/qcollectiongenerator/.moc
qt5-qttools-debuginfo.x86_64: W: hidden-file-or-dir /usr/src/debug/qttools-opensource-src-5.0.2/src/designer/src/designer/.moc
qt5-qttools-debuginfo.x86_64: W: hidden-file-or-dir /usr/src/debug/qttools-opensource-src-5.0.2/src/designer/src/designer/.moc
qt5-qttools-debuginfo.x86_64: W: hidden-file-or-dir /usr/src/debug/qttools-opensource-src-5.0.2/src/assistant/qhelpgenerator/.moc
qt5-qttools-debuginfo.x86_64: W: hidden-file-or-dir /usr/src/debug/qttools-opensource-src-5.0.2/src/assistant/qhelpgenerator/.moc
qt5-qttools-debuginfo.x86_64: W: hidden-file-or-dir /usr/src/debug/qttools-opensource-src-5.0.2/src/designer/src/components/lib/.moc
qt5-qttools-debuginfo.x86_64: W: hidden-file-or-dir /usr/src/debug/qttools-opensource-src-5.0.2/src/designer/src/components/lib/.moc
qt5-qttools-debuginfo.x86_64: W: hidden-file-or-dir /usr/src/debug/qttools-opensource-src-5.0.2/src/linguist/linguist/.rcc
qt5-qttools-debuginfo.x86_64: W: hidden-file-or-dir /usr/src/debug/qttools-opensource-src-5.0.2/src/linguist/linguist/.rcc
qt5-qttools-debuginfo.x86_64: W: hidden-file-or-dir /usr/src/debug/qttools-opensource-src-5.0.2/src/assistant/help/.moc
qt5-qttools-debuginfo.x86_64: W: hidden-file-or-dir /usr/src/debug/qttools-opensource-src-5.0.2/src/assistant/help/.moc
qt5-qttools-debuginfo.x86_64: W: hidden-file-or-dir /usr/src/debug/qttools-opensource-src-5.0.2/src/designer/src/lib/.uic
qt5-qttools-debuginfo.x86_64: W: hidden-file-or-dir /usr/src/debug/qttools-opensource-src-5.0.2/src/designer/src/lib/.uic
qt5-qttools-debuginfo.x86_64: W: hidden-file-or-dir /usr/src/debug/qttools-opensource-src-5.0.2/src/linguist/linguist/.moc
qt5-qttools-debuginfo.x86_64: W: hidden-file-or-dir /usr/src/debug/qttools-opensource-src-5.0.2/src/linguist/linguist/.moc
qt5-qttools-debuginfo.x86_64: W: hidden-file-or-dir /usr/src/debug/qttools-opensource-src-5.0.2/src/designer/src/components/lib/.uic
qt5-qttools-debuginfo.x86_64: W: hidden-file-or-dir /usr/src/debug/qttools-opensource-src-5.0.2/src/designer/src/components/lib/.uic
qt5-qttools-debuginfo.x86_64: W: hidden-file-or-dir /usr/src/debug/qttools-opensource-src-5.0.2/src/assistant/qhelpconverter/.moc
qt5-qttools-debuginfo.x86_64: W: hidden-file-or-dir /usr/src/debug/qttools-opensource-src-5.0.2/src/assistant/qhelpconverter/.moc
qt5-qttools-debuginfo.x86_64: W: hidden-file-or-dir /usr/src/debug/qttools-opensource-src-5.0.2/src/assistant/qhelpconverter/.rcc
qt5-qttools-debuginfo.x86_64: W: hidden-file-or-dir /usr/src/debug/qttools-opensource-src-5.0.2/src/assistant/qhelpconverter/.rcc
qt5-qttools-debuginfo.x86_64: W: hidden-file-or-dir /usr/src/debug/qttools-opensource-src-5.0.2/src/qdbus/qdbusviewer/.moc
qt5-qttools-debuginfo.x86_64: W: hidden-file-or-dir /usr/src/debug/qttools-opensource-src-5.0.2/src/qdbus/qdbusviewer/.moc
qt5-qttools-debuginfo.x86_64: W: hidden-file-or-dir /usr/src/debug/qttools-opensource-src-5.0.2/src/qdbus/qdbusviewer/.rcc
qt5-qttools-debuginfo.x86_64: W: hidden-file-or-dir /usr/src/debug/qttools-opensource-src-5.0.2/src/qdbus/qdbusviewer/.rcc
qt5-qttools-debuginfo.x86_64: W: hidden-file-or-dir /usr/src/debug/qttools-opensource-src-5.0.2/src/assistant/assistant/.rcc
qt5-qttools-debuginfo.x86_64: W: hidden-file-or-dir /usr/src/debug/qttools-opensource-src-5.0.2/src/assistant/assistant/.rcc
qt5-qttools-debuginfo.x86_64: W: hidden-file-or-dir /usr/src/debug/qttools-opensource-src-5.0.2/src/assistant/help/.rcc
qt5-qttools-debuginfo.x86_64: W: hidden-file-or-dir /usr/src/debug/qttools-opensource-src-5.0.2/src/assistant/help/.rcc
qt5-qttools-debuginfo.x86_64: W: hidden-file-or-dir /usr/src/debug/qttools-opensource-src-5.0.2/src/linguist/linguist/.uic
qt5-qttools-debuginfo.x86_64: W: hidden-file-or-dir /usr/src/debug/qttools-opensource-src-5.0.2/src/linguist/linguist/.uic
qt5-qttools-debuginfo.x86_64: W: hidden-file-or-dir /usr/src/debug/qttools-opensource-src-5.0.2/src/designer/src/lib/.moc
qt5-qttools-debuginfo.x86_64: W: hidden-file-or-dir /usr/src/debug/qttools-opensource-src-5.0.2/src/designer/src/lib/.moc
qt5-qttools-debuginfo.x86_64: W: hidden-file-or-dir /usr/src/debug/qttools-opensource-src-5.0.2/src/designer/src/lib/.rcc
qt5-qttools-debuginfo.x86_64: W: hidden-file-or-dir /usr/src/debug/qttools-opensource-src-5.0.2/src/designer/src/lib/.rcc
qt5-qttools-debuginfo.x86_64: W: hidden-file-or-dir /usr/src/debug/qttools-opensource-src-5.0.2/src/pixeltool/.moc
qt5-qttools-debuginfo.x86_64: W: hidden-file-or-dir /usr/src/debug/qttools-opensource-src-5.0.2/src/pixeltool/.moc
qt5-qttools-debuginfo.x86_64: W: hidden-file-or-dir /usr/src/debug/qttools-opensource-src-5.0.2/src/assistant/assistant/.moc
qt5-qttools-debuginfo.x86_64: W: hidden-file-or-dir /usr/src/debug/qttools-opensource-src-5.0.2/src/assistant/assistant/.moc
qt5-qttools-debuginfo.x86_64: W: hidden-file-or-dir /usr/src/debug/qttools-opensource-src-5.0.2/src/assistant/qhelpconverter/.uic
qt5-qttools-debuginfo.x86_64: W: hidden-file-or-dir /usr/src/debug/qttools-opensource-src-5.0.2/src/assistant/qhelpconverter/.uic
qt5-qttools-debuginfo.x86_64: W: hidden-file-or-dir /usr/src/debug/qttools-opensource-src-5.0.2/src/designer/src/designer/.rcc
qt5-qttools-debuginfo.x86_64: W: hidden-file-or-dir /usr/src/debug/qttools-opensource-src-5.0.2/src/designer/src/designer/.rcc
qt5-qttools-debuginfo.x86_64: W: hidden-file-or-dir /usr/src/debug/qttools-opensource-src-5.0.2/src/designer/src/uitools/.moc
qt5-qttools-debuginfo.x86_64: W: hidden-file-or-dir /usr/src/debug/qttools-opensource-src-5.0.2/src/designer/src/uitools/.moc
qt5-qttools-devel.x86_64: W: no-documentation
qt5-qttools-devel.x86_64: W: no-manual-page-for-binary lrelease-qt5
qt5-qttools-devel.x86_64: W: no-manual-page-for-binary linguist-qt5
qt5-qttools-devel.x86_64: W: no-manual-page-for-binary pixeltool-qt5
qt5-qttools-devel.x86_64: W: no-manual-page-for-binary qhelpgenerator-qt5
qt5-qttools-devel.x86_64: W: no-manual-page-for-binary lupdate-qt5
qt5-qttools-devel.x86_64: W: no-manual-page-for-binary lconvert-qt5
qt5-qttools-devel.x86_64: W: no-manual-page-for-binary designer-qt5
qt5-qttools-devel.x86_64: W: no-manual-page-for-binary qhelpconverter-qt5
qt5-qttools-devel.x86_64: W: no-manual-page-for-binary qcollectiongenerator-qt5
qt5-qttools-static.x86_64: W: no-documentation
5 packages and 0 specfiles checked; 0 errors, 68 warnings.

Comment 6 Gregor Tätzner 2013-04-11 20:28:22 UTC
some code is licensed under BSD 3, you didn't listed that

examples/uitools/multipleinheritance/calculatorform.cpp: BSD (3 clause)
...

Comment 7 Rex Dieter 2013-04-11 23:35:57 UTC
1.  I don't think that file even built/packaged
2.  even if it were, combined with the GPL stuff, the resulting license is GPL anyway


Spec URL: http://rdieter.fedorapeople.org/rpms/qt5/qt5-qttools.spec
SRPM URL: http://rdieter.fedorapeople.org/rpms/qt5/qt5-qttools-5.0.2-1.fc18.src.rpm

%changelog
* Thu Apr 11 2013 Rex Dieter <rdieter@fedoraproject.org> 5.0.2-1
- 5.0.2


scratch build:
http://koji.fedoraproject.org/koji/taskinfo?taskID=5243968

Comment 8 Gregor Tätzner 2013-04-12 06:22:28 UTC
- if possible remove libtools archive from qtools-static:
-rw-r--r--  /usr/lib64/libQt5UiTools.la

- I think you should restict the buildreq to qtbase-devel further. We saw what happens if you use the wrong version

Comment 9 Rex Dieter 2013-04-14 12:21:41 UTC
Re: removing libQt5UiTools.la

No, per
http://fedoraproject.org/wiki/Packaging:Guidelines#Packaging_Static_Libraries

which implies, but unfortunately does not say explicitly, .la files can only be safely removed in the non-static linking case.

Comment 10 Gregor Tätzner 2013-04-14 20:37:31 UTC
very well, another thing:
the source includes quite a lot of example code. How do we handle api doc and example code in qt5? Didn't spot a qt5-doc in your package reviews. Imho it would be nice to move those samples into a *-doc subpkg

Comment 11 Rex Dieter 2013-04-15 01:57:09 UTC
Good point, probably worth consulting upstream about (since these items are not installed by default).  That said, I'd prefer to deal with this post-review, as it isn't a MUST/blocker item.

Comment 12 Gregor Tätzner 2013-04-15 06:32:32 UTC
ah, last point: you are installing quite some gui applications (assistant, dbusviewer) but there are no signs of desktop files. I'm rather puzzled that these are not shipped in the tarball. I suppose we have to inject our own, as the suse team did

Comment 13 Rex Dieter 2013-04-15 10:17:04 UTC
We don't *have* to, but .desktop files would be nice for anything one would want to appear in menus.

Comment 14 Gregor Tätzner 2013-04-15 11:37:42 UTC
(In reply to comment #13)
> We don't *have* to, but .desktop files would be nice for anything one would
> want to appear in menus.

no, it wouldn't be nice - it's necessary...
http://fedoraproject.org/wiki/Packaging:Guidelines#Desktop_files

Since we don't present qdbusviewer to the user in qt4 packaging, we can get away with that. But for assistant, a menu entry really has to be provided (for designer and linguist too). Even if there is not much to see yet ;)

Now if I think about it, the package splitting feels *very* strange. What are doing all those tools in devel? Imho all binaries should be moved to qttools since the target group are devs anyway. Or even better: move every tool into his own subpkg. But again no MUST, so your choice.

Comment 15 Rex Dieter 2013-04-15 11:57:46 UTC
Fair enough.  I'll put adding .desktop files on my todo list.

Comment 16 Rex Dieter 2013-04-20 02:53:52 UTC
%changelog
* Fri Apr 19 2013 Rex Dieter <rdieter@fedoraproject.org> 5.0.2-2
- add .desktop/icons for assistant, designer, linguist, qdbusviewer

Spec URL: http://rdieter.fedorapeople.org/rpms/qt5/qt5-qttools.spec
SRPM URL: http://rdieter.fedorapeople.org/rpms/qt5/qt5-qttools-5.0.2-2.fc18.src.rpm

Comment 17 Gregor Tätzner 2013-04-21 09:04:45 UTC
Created attachment 738214 [details]
pre-review

thanks for the quick update.

When browsing the code I missed some spots:

- assistant bundles clucene (src/assistant/3rdparty)

- desktop-file-install: we're not allowed to use the vendor option
http://fedoraproject.org/wiki/Packaging:Guidelines#desktop-file-install_usage

- include licenses and changelog (dist/*) in %doc

Comment 18 Rex Dieter 2013-04-21 12:33:30 UTC
Looks like we made the guideline update too strict.   What we wanted to avoid was packages uselessly adding --vendor (like --vendor=fedora).  One *valid* purpose of .desktop vendor was to avoid conflicts (with qt4), which is precisely why I'm using it here.

See
http://standards.freedesktop.org/menu-spec/menu-spec-latest.html

"To prevent that a desktop entry from one party inadvertently cancels out the desktop entry from another party because both happen to get the same desktop-file id it is recommended that providers of desktop-files ensure that all desktop-file ids start with a vendor prefix..."

Comment 19 Gregor Tätzner 2013-04-24 11:50:53 UTC
Does that mean you can achieve the same result by renaming the desktop files to qt5-<component>.desktop?

btw:
desktop-file-validate: *.desktop: warning: key "Encoding" in group "Desktop Entry" is deprecated

Comment 20 Rex Dieter 2013-04-24 12:54:10 UTC
> Does that mean you can achieve the same result by renaming the 
> desktop files to qt5-<component>.desktop?

Yes, that's precisely what adding a vendor prefix means.

Comment 21 Rex Dieter 2013-04-29 16:32:25 UTC
%changelog
* Mon Apr 29 2013 Rex Dieter <rdieter@fedoraproject.org> 5.0.2-3
- drop deprecated Encoding= key from .desktop files
- add justification for desktop vendor usage

Spec URL: http://rdieter.fedorapeople.org/rpms/qt5/qt5-qttools.spec
SRPM URL: http://rdieter.fedorapeople.org/rpms/qt5/qt5-qttools-5.0.2-3.fc18.src.rpm

Comment 22 Panos Polychronis 2013-05-24 18:10:18 UTC
last lines from rpmbuild (qt5-qttools-5.0.2-3.fc18.src.rpm @ fedora 19) output.... 
g++ -c -m64 -pipe -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fstack-protector --param=ssp-buffer-size=4 -m64 -mtune=generic -O2 -fno-exceptions -Wall -W -D_REENTRANT -fPIE -DQT_NO_LIBUDEV -DQT_NO_CAST_FROM_ASCII -DQT_NO_CAST_TO_ASCII -DQFORMINTERNAL_NAMESPACE -DQT_KEYWORDS -DQT_NO_EXCEPTIONS -D_LARGEFILE64_SOURCE -D_LARGEFILE_SOURCE -DQT_NO_DEBUG -DQT_PRINTSUPPORT_LIB -DQT_WIDGETS_LIB -DQT_UITOOLS_LIB -DQT_XML_LIB -DQT_GUI_LIB -DQT_CORE_LIB -I/usr/lib64/qt5/mkspecs/linux-g++-64 -I. -I../shared -I/usr/include/qt5 -I/usr/include/qt5/QtPrintSupport -I/usr/include/qt5/QtWidgets -I../../../include -I../../../include/QtUiTools -I../../../include/QtUiTools/5.0.2 -I../../../include/QtUiTools/5.0.2/QtUiTools -I/usr/include/qt5/QtXml -I/usr/include/qt5/QtGui -I/usr/include/qt5/QtCore -I/usr/include/qt5/QtCore/5.0.2 -I/usr/include/qt5/QtCore/5.0.2/QtCore -I.moc/release-shared -I.uic/release-shared -o .obj/release-shared/moc_translatedialog.o .moc/release-shared/moc_translatedialog.cpp
g++ -c -m64 -pipe -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fstack-protector --param=ssp-buffer-size=4 -m64 -mtune=generic -O2 -fno-exceptions -Wall -W -D_REENTRANT -fPIE -DQT_NO_LIBUDEV -DQT_NO_CAST_FROM_ASCII -DQT_NO_CAST_TO_ASCII -DQFORMINTERNAL_NAMESPACE -DQT_KEYWORDS -DQT_NO_EXCEPTIONS -D_LARGEFILE64_SOURCE -D_LARGEFILE_SOURCE -DQT_NO_DEBUG -DQT_PRINTSUPPORT_LIB -DQT_WIDGETS_LIB -DQT_UITOOLS_LIB -DQT_XML_LIB -DQT_GUI_LIB -DQT_CORE_LIB -I/usr/lib64/qt5/mkspecs/linux-g++-64 -I. -I../shared -I/usr/include/qt5 -I/usr/include/qt5/QtPrintSupport -I/usr/include/qt5/QtWidgets -I../../../include -I../../../include/QtUiTools -I../../../include/QtUiTools/5.0.2 -I../../../include/QtUiTools/5.0.2/QtUiTools -I/usr/include/qt5/QtXml -I/usr/include/qt5/QtGui -I/usr/include/qt5/QtCore -I/usr/include/qt5/QtCore/5.0.2 -I/usr/include/qt5/QtCore/5.0.2/QtCore -I.moc/release-shared -I.uic/release-shared -o .obj/release-shared/moc_translationsettingsdialog.o .moc/release-shared/moc_translationsettingsdialog.cpp
g++ -m64 -Wl,-O1 -Wl,-z,relro -o ../../../bin/linguist .obj/release-shared/numerus.o .obj/release-shared/translator.o .obj/release-shared/translatormessage.o .obj/release-shared/qm.o .obj/release-shared/qph.o .obj/release-shared/po.o .obj/release-shared/ts.o .obj/release-shared/xliff.o .obj/release-shared/batchtranslationdialog.o .obj/release-shared/errorsview.o .obj/release-shared/finddialog.o .obj/release-shared/formpreviewview.o .obj/release-shared/globals.o .obj/release-shared/main.o .obj/release-shared/mainwindow.o .obj/release-shared/messageeditor.o .obj/release-shared/messageeditorwidgets.o .obj/release-shared/messagehighlighter.o .obj/release-shared/messagemodel.o .obj/release-shared/phrasebookbox.o .obj/release-shared/phrase.o .obj/release-shared/phrasemodel.o .obj/release-shared/phraseview.o .obj/release-shared/printout.o .obj/release-shared/recentfiles.o .obj/release-shared/sourcecodeview.o .obj/release-shared/statistics.o .obj/release-shared/translatedialog.o .obj/release-shared/translationsettingsdialog.o .obj/release-shared/simtexth.o .obj/release-shared/qrc_linguist.o .obj/release-shared/moc_batchtranslationdialog.o .obj/release-shared/moc_errorsview.o .obj/release-shared/moc_finddialog.o .obj/release-shared/moc_formpreviewview.o .obj/release-shared/moc_mainwindow.o .obj/release-shared/moc_messageeditor.o .obj/release-shared/moc_messageeditorwidgets.o .obj/release-shared/moc_messagehighlighter.o .obj/release-shared/moc_messagemodel.o .obj/release-shared/moc_phrasebookbox.o .obj/release-shared/moc_phrase.o .obj/release-shared/moc_phrasemodel.o .obj/release-shared/moc_phraseview.o .obj/release-shared/moc_recentfiles.o .obj/release-shared/moc_sourcecodeview.o .obj/release-shared/moc_statistics.o .obj/release-shared/moc_translatedialog.o .obj/release-shared/moc_translationsettingsdialog.o   -L/usr/X11R6/lib64 -lQt5PrintSupport -L/root/rpmbuild/BUILD/qttools-opensource-src-5.0.2/lib -lQt5UiTools -lQt5Widgets -lQt5Xml -lQt5Gui -lQt5Core -lGL -lpthread 
make[3]: Leaving directory `/root/rpmbuild/BUILD/qttools-opensource-src-5.0.2/src/linguist/linguist'
make[2]: Leaving directory `/root/rpmbuild/BUILD/qttools-opensource-src-5.0.2/src/linguist'
make[1]: *** [sub-linguist-make_first-ordered] Error 2
make[1]: Leaving directory `/root/rpmbuild/BUILD/qttools-opensource-src-5.0.2/src'
make: *** [sub-src-make_first] Error 2
error: Bad exit status from /var/tmp/rpm-tmp.Dj8qER (%build)

Comment 23 Rex Dieter 2013-05-24 18:24:04 UTC
Hard to say, builds ok in mock for me:

http://kdeforge.unl.edu/apt/kde-redhat/mock/fedora-18-x86_64-kde/qt5-qttools/

once we get all it's dependencies reviewed, we can do official koji (scratch) builds too for review purposes.

Comment 24 Rex Dieter 2013-06-12 01:43:43 UTC
ping, you still interested in finishing this review?  (I see the review flag isn't set, if you want to continue, please set to ?, thanks).

Comment 25 Gregor Tätzner 2013-06-12 06:39:12 UTC
sure, but you haven't responded to my bundling note regarding clucene.

(In reply to Rex Dieter from comment #24)
> (I see the review flag
> isn't set, if you want to continue, please set to ?, thanks).

ah, I always forget that

Comment 26 Rex Dieter 2013-06-13 19:00:38 UTC
My naive attempt to use system clucene-core is a bust, applying this:

http://rdieter.fedorapeople.org/rpms/qt5/qttools-opensource-src-5.0.2-system_clucene.patch

build fails pretty quickly in src/assistant/clucene/

...
-o .obj/release-shared/qanalyzer.o qanalyzer.cpp
In file included from /usr/include/CLucene/index/IndexWriter.h:19:0,
                 from /usr/include/CLucene.h:13,
                 from qanalyzer.cpp:22:
/usr/include/CLucene/index/MergePolicy.h:238:35: error: ‘DEFAULT_MERGE_FACTOR’ has not been declared
   LUCENE_STATIC_CONSTANT(int32_t, DEFAULT_MERGE_FACTOR = 10);
                                   ^
In file included from /usr/include/CLucene/index/IndexWriter.h:19:0,
                 from /usr/include/CLucene.h:13,
                 from qanalyzer.cpp:22:
/usr/include/CLucene/index/MergePolicy.h:351:35: error: ‘DEFAULT_MIN_MERGE_DOCS’ has not been declared
   LUCENE_STATIC_CONSTANT(int32_t, DEFAULT_MIN_MERGE_DOCS = 1000);
                                   ^
In file included from /usr/include/CLucene.h:13:0,
                 from qanalyzer.cpp:22:
/usr/include/CLucene/index/IndexWriter.h:290:34: error: ‘DEFAULT_MAX_FIELD_LENGTH’ has not been declared
  LUCENE_STATIC_CONSTANT(int32_t, DEFAULT_MAX_FIELD_LENGTH = 10000);

not sure why exactly.  LUCENE_STATIC_CONSTANT should be defined from CLucene/clucene-config.h

Any ideas?


Any ideas?

Comment 27 Gregor Tätzner 2013-06-14 07:38:57 UTC
He can't find 'DEFAULT_MERGE_FACTOR', 'LUCENE_STATIC_CONSTANT' is fine

in general assistant is stuck on an old 0.9.x version of clucene. no wonder it doesn't take our current system version. question is: who wants to port it ;)

Comment 28 Gregor Tätzner 2013-06-14 08:01:05 UTC
actually it doesn't build against clucene09-core-devel either.

qmake:
INCLUDEPATH +=  /usr/include/clucene09 /usr/lib64/clucene09
LIBS += -L/usr/lib64/clucene09 -lclucene-core -lclucene-shared

Result:

g++ -c -m64 -pipe -fexceptions -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fstack-protector --param=ssp-buffer-size=4 -m64 -mtune=generic -O2 -fvisibility=hidden -fvisibility-inlines-hidden -w -std=c++0x -D_REENTRANT -fPIC -DQT_BUILD_CLUCENE_LIB -DQT_BUILDING_QT -DQT_NO_CAST_TO_ASCII -DQT_ASCII_CAST_WARNINGS -DQT_MOC_COMPAT -DQT_USE_QSTRINGBUILDER -DQT_DEPRECATED_WARNINGS -DQT_DISABLE_DEPRECATED_BEFORE=0x050000 -D_BUILD_FOR_QT_ -DLUCENE_DISABLE_MEMTRACKING -D_GLIBCXX_EXTERN_TEMPLATE=0 -D_LARGEFILE64_SOURCE -D_LARGEFILE_SOURCE -DQT_NO_DEBUG -DQT_CORE_LIB -I/usr/lib64/qt5/mkspecs/linux-g++-64 -I. -I../../../include -I../../../include/QtCLucene -I../../../include/QtCLucene/5.0.2 -I../../../include/QtCLucene/5.0.2/QtCLucene -I/usr/include/clucene09 -I/usr/lib64/clucene09 -I/usr/include/qt5 -I/usr/include/qt5/QtCore -I.moc/release-shared -o .obj/release-shared/qanalyzer.o qanalyzer.cpp
qanalyzer.cpp: In copy constructor ‘QCLuceneAnalyzerPrivate::QCLuceneAnalyzerPrivate(const QCLuceneAnalyzerPrivate&)’:
qanalyzer.cpp:37:64: error: ‘class lucene::analysis::Analyzer’ has no member named ‘__cl_addref’
qanalyzer.cpp: In destructor ‘QCLuceneAnalyzerPrivate::~QCLuceneAnalyzerPrivate()’:
qanalyzer.cpp:44:50: error: ‘class lucene::analysis::Analyzer’ has no member named ‘__cl_decref’
make[3]: *** [.obj/release-shared/qanalyzer.o] Error 1
make[3]: Leaving directory `/home/greg/rpmbuild/BUILD/qttools-opensource-src-5.0.2/src/assistant/clucene'
make[2]: *** [sub-clucene-make_first-ordered] Error 2
make[2]: Leaving directory `/home/greg/rpmbuild/BUILD/qttools-opensource-src-5.0.2/src/assistant'
make[1]: *** [sub-assistant-make_first-ordered] Error 2
make[1]: Leaving directory `/home/greg/rpmbuild/BUILD/qttools-opensource-src-5.0.2/src'
make: *** [sub-src-make_first] Error 2

not sure if that qmake conf is correct though

Comment 29 Rex Dieter 2013-06-14 13:54:26 UTC
I could've sworn diff'ing the bundle to the latest clucene git and finding only minimal differences, but maybe I did that wrong.

Anyway, here's my query upstream about the topic,
http://lists.qt-project.org/pipermail/development/2013-June/011306.html

Comment 30 Rex Dieter 2013-08-17 19:42:08 UTC
upstream hasn't responded to my query. :(  and, given our simple and failed attempts to unbundle it, I'm not sure what else to do.

Comment 31 Gregor Tätzner 2013-08-17 21:01:13 UTC
well I suppose you add a comment about the bundled clucene and we go on?

Comment 32 Rex Dieter 2013-08-17 21:52:06 UTC
OK, I can agree with that tentative plan.  I'll add a similar note to qt4 packaging that is affected as well.

Comment 33 Dan Horák 2013-08-18 08:25:05 UTC
The bundled clucene is close to 0.9.21b, but it also uses non-public headers ...

Comment 34 Dan Horák 2013-08-18 08:37:14 UTC
Created attachment 787740 [details]
diff between bundled clucene and fedora clucene09

Comment 35 Dan Horák 2013-08-18 10:42:37 UTC
stay tuned, my unbundling efforts looks promising ...

Comment 36 Dan Horák 2013-08-18 11:37:55 UTC
Created attachment 787760 [details]
use system clucene

and finally we are there

what remains is to switch to some variable instead of hard-coded /usr/lib64

Comment 37 Dan Horák 2013-08-18 14:51:54 UTC
there is probably a wrong path somewhere in the language tools setup, I get the following error when building sigil-0.7.1 

...
[ 11%] Generating sigil_ja.qm
cd /home/dan/projects/fedora/sigil/sigil-0.7.1/build/src/Sigil && /usr/lib64/cmake/Qt5LinguistTools/bin/lrelease /home/dan/projects/fedora/sigil/sigil-0.7.1/src/Sigil/Resource_Files/ts/sigil_ja.ts -qm /home/dan/projects/fedora/sigil/sigil-0.7.1/build/src/Sigil/sigil_ja.qm
/bin/sh: /usr/lib64/cmake/Qt5LinguistTools/bin/lrelease: No such file or directory
make[2]: *** [src/Sigil/sigil_ja.qm] Error 127

Comment 38 Dan Horák 2013-08-18 15:43:50 UTC
(In reply to Dan Horák from comment #37)
> there is probably a wrong path somewhere in the language tools setup, I get
> the following error when building sigil-0.7.1 
> 
> ...
> [ 11%] Generating sigil_ja.qm
> cd /home/dan/projects/fedora/sigil/sigil-0.7.1/build/src/Sigil &&
> /usr/lib64/cmake/Qt5LinguistTools/bin/lrelease
> /home/dan/projects/fedora/sigil/sigil-0.7.1/src/Sigil/Resource_Files/ts/
> sigil_ja.ts -qm
> /home/dan/projects/fedora/sigil/sigil-0.7.1/build/src/Sigil/sigil_ja.qm
> /bin/sh: /usr/lib64/cmake/Qt5LinguistTools/bin/lrelease: No such file or
> directory
> make[2]: *** [src/Sigil/sigil_ja.qm] Error 127

the path seems to be defined in ./src/linguist/Qt5LinguistToolsConfig.cmake.in

Comment 39 Rex Dieter 2013-08-18 17:22:50 UTC
Thanks for the clucene love!

That cmake bug you see looks very similar to one we'd fixed previously in qtbase.  Let's finish the review, then update to latest 5.1.x, to see if it's fixed upstream yet or not.

Comment 40 Rex Dieter 2013-08-18 18:00:48 UTC
Hrm, that patch doesn't seem to work for me, I'm seeing these all over:

In file included from /usr/include/clucene09/CLucene/index/IndexReader.h:15:0,
                 from /usr/include/clucene09/CLucene.h:14,
                 from qtokenstream.cpp:20:
/usr/include/clucene09/CLucene/store/FSDirectory.h:92:5: error: looser throw specifier for ‘virtual lucene::store::FSDirectory::FSIndexInput::SharedHandle::~SharedHandle() throw (CLuceneError&)’
     ~SharedHandle() throw(CLuceneError&);
     ^
In file included from /usr/include/clucene09/CLucene/debug/mem.h:14:0,
                 from /usr/include/clucene09/CLucene/StdHeader.h:440,
                 from /usr/include/clucene09/CLucene.h:11,
                 from qtokenstream.cpp:20:
/usr/include/clucene09/CLucene/debug/lucenebase.h:61:13: error:   overriding ‘virtual lucene::debug::LuceneBase::~LuceneBase() noexcept (true)’
     virtual ~LuceneBase(){};

Comment 41 Dan Horák 2013-08-18 18:13:42 UTC
I've successfully rebuilt
http://fedora.danny.cz/tmp/qt5-qttools-5.0.2-3.fc18.dh.1.src.rpm on F-18, your error should be caused by g++ 4.8 if you are on F-19, I'll look on it ...

Comment 42 Dan Horák 2013-08-18 18:19:09 UTC
can you try with -fpermissive ?

Comment 43 Rex Dieter 2013-08-18 18:51:04 UTC
adding -fpermissive didn't help. :(

Comment 44 Dan Horák 2013-08-18 21:08:55 UTC
Created attachment 787847 [details]
proposed fix

seems we need a fix in clucene09

Comment 45 Kevin Kofler 2013-08-19 11:36:54 UTC
I don't think that fix is right. The problem is that the virtual ~LuceneBase() destructor must not be noexcept because at least 1 subclass does throw exceptions in the destructor. So I think that noexcept in lucenebase.h needs to go away. But it is definitely a clucene09 bug.

Comment 46 Dan Horák 2013-08-19 11:59:56 UTC
(In reply to Kevin Kofler from comment #45)
> I don't think that fix is right. The problem is that the virtual
> ~LuceneBase() destructor must not be noexcept because at least 1 subclass
> does throw exceptions in the destructor. So I think that noexcept in
> lucenebase.h needs to go away. But it is definitely a clucene09 bug.

Kevin, could you please open new bug against clucene09 with this information? Thanks.

Comment 47 Kevin Kofler 2013-08-19 12:00:47 UTC
> what remains is to switch to some variable instead of hard-coded /usr/lib64

You probably want to do something with QMAKE_LIBDIR, but beware, that can be a whole list of directories, unfortunately.

Comment 48 Kevin Kofler 2013-08-19 12:11:22 UTC
> Kevin, could you please open new bug against clucene09 with this information?
> Thanks.

Filed bug #998477.

Comment 49 Rex Dieter 2013-08-19 13:57:14 UTC
Can confirm build succeeds against the latest clucene09 patched per bug #998477


%changelog
* Mon Aug 19 2013 Rex Dieter <rdieter@fedoraproject.org> 5.0.2-4
- use system clucene09-core

Spec URL: http://rdieter.fedorapeople.org/rpms/qt5/qt5-qttools.spec
SRPM URL: http://rdieter.fedorapeople.org/rpms/qt5/qt5-qttools-5.0.2-4.src.rpm

Comment 50 Rex Dieter 2013-08-27 15:35:45 UTC
Gregor, ping?  Any other review blockers I've missed?

Comment 51 Gregor Tätzner 2013-08-27 16:17:34 UTC
uhm it doesn't build. thats actually a blocker - I checked it :). afaik there is no proper clucene09 in the pipeline, so whats the use of pushing this package forward?

Comment 52 Rex Dieter 2013-08-27 16:30:51 UTC
clucene09 : http://koji.fedoraproject.org/koji/packageinfo?packageID=12830

and clucene09 bug tracking required fix(es) in order for dependant packages (like this one) to build using recent gcc: 
https://bugzilla.redhat.com/show_bug.cgi?id=998477

If you're going to insist clucene09 get fixed before approving this review, I guess I can use provenpackager to make the required fixes...

Comment 53 Gregor Tätzner 2013-08-27 17:28:25 UTC
Created attachment 791114 [details]
rawhide mock build fail

no luck with your patched clucene

Comment 54 Rex Dieter 2013-08-27 17:34:48 UTC
Grr, looks like the clucene09 build didn't patch properly, taking a closer look.

Just to be clear, you *are* going to consider a bug in clucene09 that causes a build failure here to be a review blocker?

Comment 55 Rex Dieter 2013-08-27 17:38:27 UTC
confirmed clucene09-0.9.21b-7 didnt apply the patch. :(  fixed 0.9.21b-8 on the way.

Comment 56 Gregor Tätzner 2013-08-27 18:09:55 UTC
(In reply to Rex Dieter from comment #54)
> Just to be clear, you *are* going to consider a bug in clucene09 that causes
> a build failure here to be a review blocker?

I just want to be able to build this package so fedora-review can do its magic. I don't care how or why the build fails.

Comment 57 Gregor Tätzner 2013-08-27 19:33:27 UTC
Created attachment 791155 [details]
review

minor issues:

- add license files to %doc (LICENSE.FDL LICENSE.GPL LICENSE.LGPL and exceptions)
- also the changelogs from dist/*

========
APPROVED
========

Comment 58 Rex Dieter 2013-08-28 17:29:25 UTC
New Package SCM Request
=======================
Package Name: qt5-QtTool components
Short Description: Qt5 - QtTool components
Owners: than rdieter jreznik kkofler ltinkl rnovacek
Branches: f18 f19 f20
InitialCC:

Comment 59 Gwyn Ciesla 2013-08-28 18:21:28 UTC
WARNING: Requested package name qt5-QtTool components doesn't match bug summary
qt5-qttools, please correct.

Comment 60 Rex Dieter 2013-08-28 18:34:18 UTC
New Package SCM Request
=======================
Package Name: qt5 - QtTool components
Short Description: Qt5 - QtTool components
Owners: than rdieter jreznik kkofler ltinkl rnovacek
Branches: f18 f19 f20
InitialCC:

Comment 61 Rex Dieter 2013-08-28 18:42:09 UTC
Let's try one more time with feeling,

New Package SCM Request
=======================
Package Name: qt5-qttools
Short Description: Qt5 - QtTool components
Owners: than rdieter jreznik kkofler ltinkl rnovacek
Branches: f18 f19 f20
InitialCC:

Comment 62 Gwyn Ciesla 2013-08-28 19:16:06 UTC
Git done (by process-git-requests).

Comment 63 Rex Dieter 2013-09-03 12:07:03 UTC
imported

Comment 64 Rex Dieter 2013-09-10 17:03:38 UTC
Package Change Request
======================
Package Name: qt5-qttools
New Branches: el6
Owners: hobbes1069 rdieter
InitialCC: 

Co-maintainers welcome!

Comment 65 Gwyn Ciesla 2013-09-10 17:54:46 UTC
Git done (by process-git-requests).


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