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}

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).

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> 5.0.1-2
- BR: pkgconfig(zlib)
- -static subpkg

Got an error building this: https://gist.github.com/anonymous/5357548

(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.

some code is licensed under BSD 3, you didn't listed that

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

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> 5.0.2-1
- 5.0.2


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

- 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

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.

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

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.

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

We don't *have* to, but .desktop files would be nice for anything one would want to appear in menus.

(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.

Fair enough.  I'll put adding .desktop files on my todo list.

%changelog
* Fri Apr 19 2013 Rex Dieter <rdieter> 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

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

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..."

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

> 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.

%changelog
* Mon Apr 29 2013 Rex Dieter <rdieter> 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

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)

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.

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).

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

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?

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 ;)

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

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

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.

well I suppose you add a comment about the bundled clucene and we go on?

OK, I can agree with that tentative plan.  I'll add a similar note to qt4 packaging that is affected as well.

The bundled clucene is close to 0.9.21b, but it also uses non-public headers ...

Created attachment 787740 [details]
diff between bundled clucene and fedora clucene09

stay tuned, my unbundling efforts looks promising ...

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

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

(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

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.

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(){};

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 ...

can you try with -fpermissive ?

adding -fpermissive didn't help. :(

Created attachment 787847 [details]
proposed fix

seems we need a fix in clucene09

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.

(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.

> 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.

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

Filed bug #998477.

Can confirm build succeeds against the latest clucene09 patched per bug #998477


%changelog
* Mon Aug 19 2013 Rex Dieter <rdieter> 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

Gregor, ping?  Any other review blockers I've missed?

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?

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...

Created attachment 791114 [details]
rawhide mock build fail

no luck with your patched clucene

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?

confirmed clucene09-0.9.21b-7 didnt apply the patch. :(  fixed 0.9.21b-8 on the way.

(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.

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
========

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:

WARNING: Requested package name qt5-QtTool components doesn't match bug summary
qt5-qttools, please correct.

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:

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:

Git done (by process-git-requests).

imported

Package Change Request
======================
Package Name: qt5-qttools
New Branches: el6
Owners: hobbes1069 rdieter
InitialCC: 

Co-maintainers welcome!

Git done (by process-git-requests).