Bug 846913

Summary:	Review Request: libcommuni - Communi is a cross-platform IRC client library written with Qt 4
Product:	[Fedora] Fedora	Reporter:	Jan Kaluža <jkaluza>
Component:	Package Review	Assignee:	Matěj Cepl <mcepl>
Status:	CLOSED NEXTRELEASE	QA Contact:	Fedora Extras Quality Assurance <extras-qa>
Severity:	medium	Docs Contact:
Priority:	medium
Version:	rawhide	CC:	dan.mashal, dwmw2, jpnurmi, mcepl, mcepl, notting
Target Milestone:	---	Flags:	mcepl: fedora-review+ gwync: fedora-cvs+
Target Release:	---
Hardware:	All
OS:	Linux
Whiteboard:
Fixed In Version:		Doc Type:	Bug Fix
Doc Text:		Story Points:	---
Clone Of:		Environment:
Last Closed:	2012-10-29 13:43:26 UTC	Type:	---
Regression:	---	Mount Type:	---
Documentation:	---	CRM:
Verified Versions:		Category:	---
oVirt Team:	---	RHEL 7.3 requirements from Atomic Host:
Cloudforms Team:	---	Target Upstream Version:
Embargoed:
Bug Depends On:
Bug Blocks:	1006556

Description Jan Kaluža 2012-08-09 06:14:05 UTC

Spec URL: http://jkaluza.fedorapeople.org/communi.spec
SRPM URL: http://jkaluza.fedorapeople.org/communi-1.1.0-1.fc17.src.rpm
Description: Communi is a cross-platform IRC client library written with Qt 4.
Fedora Account System Username: jkaluza

libircclient-qt is renamed to communi. This is a re-review request for a package rename.

rpmlint shows following warning, but this should be OK according to guidelines:
communi.x86_64: W: obsolete-not-provided libircclient-qt

[1] says: "If a package supersedes/replaces an existing package without being a compatible enough replacement as defined in above, use only the Obsoletes from above."

[1] http://fedoraproject.org/wiki/Packaging:Guidelines#Renaming.2FReplacing_Existing_Packages

Comment 1 Matěj Cepl 2012-08-09 10:35:14 UTC

First of all ... it would be prudent to do something about bug 707617 and not just letting it hang there.

Legend: + = PASSED, - = FAILED, 0 = Not Applicable

- MUST: rpmlint must be run on every package. The output should be posted in
the review

rpmlint on src.rpm is silent

HOWEVER

I was not able to rebuilt binary packages neither on my computer or in koji (http://koji.fedoraproject.org/koji/taskinfo?taskID=4371336), so I couldn't test binary packages. I got

error: Installed (but unpackaged) file(s) found:
   /usr/lib64/qt4/imports/Communi/libcommuniplugin.so
   /usr/lib64/qt4/imports/Communi/qmldir

+ MUST: package named according to the Package Naming Guidelines
+ MUST: The spec file name must match the base package %{name}
+ MUST: The package must meet the Packaging Guidelines .
+ MUST: The package licensed with a Fedora approved license and meets the
Licensing Guidelines
+ MUST: The License field in the package spec file matches the actual
license
LGPLv2+
+ MUST: If (and only if) the source package includes the text of the license(s)
in its own file, then that file, containing the text of the license(s) for the
package must be included in %doc.
License file is not included in the upstream package.
+ MUST: The spec file must be written in American English.
+ MUST: The spec file for the package MUST be legible.
+ MUST: The sources used to build the package must match the upstream
source, as provided in the spec URL. Reviewers should use md5sum for this task
MD5: 1e6d736d5f212a91fe9f1a6d0926fef5
- MUST: The package successfully compiles and builds into binary rpms on at
least one primary architecture - SEE ABOVE
0 MUST: If the package does not successfully compile, build or work on an
architecture, then those architectures should be listed in the spec in
ExcludeArch
+ MUST: All build dependencies must be listed in BuildRequires, except for any
that are listed in the exceptions section of the Packaging Guidelines
%build part goes on smoothly.
0 MUST: The spec file handles locales properly. This is done by using the
%find_lang macro
No locales are present.
+ MUST: Every binary RPM package (or subpackage) which stores shared library
files (not just symlinks) in any of the dynamic linker's default paths, must
call ldconfig in %post and %postun.
+ MUST: Packages must NOT bundle copies of system libraries
0 MUST: If the package is designed to be relocatable, the packager must state
this fact in the request for review, along with the rationalization for
relocation of that specific package. Without this, use of Prefix: /usr is
considered a blocker
+ MUST: Package must own all directories that it creates. If it does not create
a directory that it uses, then it should require a package which does create
that directory
+ MUST: Package must not list a file more than once in the spec file's %files
listings
+ MUST: Each package must consistently use macros
+ MUST: The package must contain code, or permissible content
0 MUST: Large documentation files must go in a -doc subpackage
0 MUST: If a package includes something as %doc, it must not affect the runtime
of the application
+ MUST: Header files must be in a -devel package
0 MUST: Static libraries must be in a -static package
0 MUST: Packages containing pkgconfig(.pc) files must 'Requires: pkgconfig'
+ MUST: If a package contains library files with a suffix (e.g. libfoo.so.1.1),
then library files that end in .so (without suffix) must go in a -devel package
+ MUST: devel packages must require the base package using a fully versioned
dependency: Requires: %{name} = %{version}-%{release}
+ MUST: Packages must NOT contain any .la libtool archives, these must be
removed in the spec if they are built
0 MUST: Packages containing GUI applications must include a %{name}.desktop
file, and that file must be properly installed with desktop-file-install in the
%install section
+ MUST: Packages must not own files or directories already owned by other
packages
+ MUST: All filenames in rpm packages must be valid UTF-8

Please fix FTBFS, otherwise this looks good.

Comment 2 Jan Kaluža 2012-08-10 05:21:33 UTC

There was different error in koji than the one you have pasted here. I've fixed this error, but I'm not sure how to fix the one you've pasted in your Comment 1.

Those two files are built only when Qt > 4.7. Is it somehow possible to check that in spec file? I haven't found solution yet.

Comment 3 Dan Mashal 2012-08-10 05:41:51 UTC

I am not able to compile this from source. Even after installing the following:

mingw64-qt-qmake
qconf
qt-devel
libicu-devel
libircclient-qt

AND editing the configure script to use qmake-qt4:

# ./configure 
which: no qmake in (/usr/local/bin:/usr/bin:/bin:/usr/local/sbin:/usr/sbin:/home/dan/.local/bin:/home/dan/bin)
Running qmake:  

===

[root@Fedora174 communi-communi-68ddd57]# ./configure
Running qmake: qmake-qt4 
Project MESSAGE: Building Communi 1.1.0
[root@Fedora174 communi-communi-68ddd57]# make
cd src/ && /usr/bin/qmake-qt4 /home/dan/846913-communi/srpm-unpacked/communi-communi-68ddd57/src/src.pro -o Makefile
cd src/ && make -f Makefile 
make[1]: Entering directory `/home/dan/846913-communi/srpm-unpacked/communi-communi-68ddd57/src'
compiling 3rdparty/uchardet-0.0.1/src/CharDistribution.cpp
/bin/sh: g++: command not found
make[1]: *** [release/CharDistribution.o] Error 127
make[1]: Leaving directory `/home/dan/846913-communi/srpm-unpacked/communi-communi-68ddd57/src'
make: *** [sub-src-make_default-ordered] Error 2
[root@Fedora174 communi-communi-68ddd57]# qmake-qt4
Project MESSAGE: Building Communi 1.1.0


I am still not able to compile.

I also have an issue with the naming of libircclient-qt.

The reason why I took this review request is because I package the BitchX IRC client, so I'm wondering if this would benefit BitchX in any way shape or form and/or would it mislead people as to what that package actually does, but that is NOT a blocker for THIS particular package review.

Thanks,
Dan

Comment 4 Jan Kaluža 2012-08-10 05:47:12 UTC

Note that it currently compiles for epel6 and fedora in koji for me.

Comment 5 Dan Mashal 2012-08-10 05:48:15 UTC

OK so this is not meant for Fedora 17 or 16 at all?

Comment 6 Jan Kaluža 2012-08-10 05:53:20 UTC

You don't have g++ installed.

yum install @development-tools

Comment 7 Dan Mashal 2012-08-10 05:53:47 UTC

After further looking into the INSTALL file the following should PROBABLY be added to the BuildRequires field:

gcc-c++
mingw64-qt-qmake
qconf
qt-devel
libicu-devel

This should probably be added to the requires field:

libircclient-qt

You should also work with upstream (if you are not upstream) to make the configure script find qmake-qt4 instead of just looking for "qmake" and erroring out.

Comment 8 Dan Mashal 2012-08-10 05:57:21 UTC

After compiling, installing and running the client I am in #communi on freenode now. 

I am very interested in this client, and working with you if you are upstream on this and/or other projects.

Most notable merging communi and BitchX. Please find me on irc, nick dan408 and let's chat.

I can see that this irc client is a work in progress, but it is a good work in progress. :)

Comment 9 Matěj Cepl 2012-08-10 07:58:43 UTC

(In reply to comment #2)
> There was different error in koji than the one you have pasted here. I've
> fixed this error, but I'm not sure how to fix the one you've pasted in your
> Comment 1.
> 
> Those two files are built only when Qt > 4.7. Is it somehow possible to
> check that in spec file? I haven't found solution yet.

I am not sure what's the problem, but it seems to me that

%if 0%{?rhel}
BuildRequires:  qt4-devel
%else
BuildRequires:  qt-devel
%endif

is a bit too simple. Usually people want to check for particular versions of the distro ... my EL-7 has probably something else installed than EL-6 in koji. I ended up in spectrum.spec with constructs like

%if 0%{?rhel} > 0 && 0%{?rhel} <= 5

or perhaps (that's from bitlbee)

%if 0%{?fedora} >= 15 || 0%{?rhel} >= 7

or something of that sort. You have to know best what exact packages you need.

Comment 10 Dan Mashal 2012-08-10 22:43:03 UTC

Should not matter, but I will check against EPEL, what you should be more worried about is the configure script itself. 

(If building from source at a terminal like the old days):

Someone should just be able to run ./configure and then the configure script should tell them to run "make" or "gmake" or "qmake" in this case. 

And qmake should compile it and then qmake install or make install would work.

Maybe we need to open a bug on mingwXX-qt-make so that an alias for "qmake" is added, or the configure script should just find it.

Comment 11 J-P Nurmi 2012-08-11 12:21:48 UTC

I have pushed some build system fixes and improvements to communi/1.1:

Fix shadow build
https://github.com/communi/communi/commit/64330271f7bc81e886b1aaa1d77fe856eb152453

configure: pick qmake-qt4 if qmake doesn't exist
https://github.com/communi/communi/commit/b731e0a5e93756d5f1a8661e8f646f7295368810

configure: run qmake recursively
To ensure fresh makefiles across the whole project tree
https://github.com/communi/communi/commit/772fa7178d615e01ec085594ae5f87d520781e5c

Let me know if I can help with remaining packaging issues.

Comment 12 Dan Mashal 2012-08-11 23:26:33 UTC

Thanks J-P.

configure has a different issue now:

[dan@Fedora17 communi]$ ./configure
Running qmake:  -r ./communi.pro
./configure: line 111: -r: command not found

Comment 13 J-P Nurmi 2012-08-12 00:02:24 UTC

Dan, either 'qmake' or 'qmake-qt4' needs to be available in PATH, or the location of qmake needs to be specified. I've added an error message to make it a bit more clear:

ERROR: Unable to find qmake. Try ./configure -qmake /path/to/qmake.

Comment 14 Dan Mashal 2012-08-12 00:20:17 UTC

My bad. I was doing it on the wrong VM. 

It works great now. Will update the review shortly here in a second.

Comment 15 Dan Mashal 2012-08-12 00:26:53 UTC

$ licensecheck -r .
./tests/auto/ircdecoder/tst_ircdecoder.cpp: UNKNOWN
./tests/auto/ircmessage/tst_ircmessage.cpp: UNKNOWN
./tests/auto/ircsession/tst_ircsession.cpp: UNKNOWN
./tests/auto/ircparser/tst_ircparser.cpp: UNKNOWN
./tests/auto/irccommand/tst_irccommand.cpp: UNKNOWN
./tests/auto/ircsender/tst_ircsender.cpp: UNKNOWN
./src/ircutil.cpp: LGPL (v2 or later) 
./src/ircdecoder.cpp: LGPL (v2 or later) 
./src/ircparser.cpp: LGPL (v2 or later) 
./src/plugins/declarative/plugin.cpp: LGPL (v2 or later) 
./src/plugins/uchardet/plugin.cpp: LGPL (v2 or later) 
./src/plugins/icu/plugin.cpp: LGPL (v2 or later) 
./src/ircsender.cpp: LGPL (v2 or later) 
./src/3rdparty/uchardet-0.0.1/script/win32.sh: *No copyright* UNKNOWN
./src/3rdparty/uchardet-0.0.1/script/debug.sh: *No copyright* UNKNOWN
./src/3rdparty/uchardet-0.0.1/script/release.sh: *No copyright* UNKNOWN
./src/3rdparty/uchardet-0.0.1/src/nsSBCharSetProber.cpp: MPL (v1.1) GPL (unversioned/unknown version) 
./src/3rdparty/uchardet-0.0.1/src/LangGreekModel.cpp: MPL (v1.1) GPL (unversioned/unknown version) 
./src/3rdparty/uchardet-0.0.1/src/CharDistribution.h: MPL (v1.1) GPL (unversioned/unknown version) 
./src/3rdparty/uchardet-0.0.1/src/nsGB2312Prober.cpp: MPL (v1.1) GPL (unversioned/unknown version) 
./src/3rdparty/uchardet-0.0.1/src/LangHebrewModel.cpp: MPL (v1.1) GPL (unversioned/unknown version) 
./src/3rdparty/uchardet-0.0.1/src/nsCharSetProber.h: MPL (v1.1) GPL (unversioned/unknown version) 
./src/3rdparty/uchardet-0.0.1/src/nsCharSetProber.cpp: MPL (v1.1) GPL (unversioned/unknown version) 
./src/3rdparty/uchardet-0.0.1/src/uchardet.cpp: MPL (v1.1) GPL (unversioned/unknown version) 
./src/3rdparty/uchardet-0.0.1/src/nsEUCTWProber.h: MPL (v1.1) GPL (unversioned/unknown version) 
./src/3rdparty/uchardet-0.0.1/src/nsSBCharSetProber.h: MPL (v1.1) GPL (unversioned/unknown version) 
./src/3rdparty/uchardet-0.0.1/src/LangHungarianModel.cpp: MPL (v1.1) GPL (unversioned/unknown version) 
./src/3rdparty/uchardet-0.0.1/src/nsEscSM.cpp: MPL (v1.1) GPL (unversioned/unknown version) 
./src/3rdparty/uchardet-0.0.1/src/uchardet.h: MPL (v1.1) GPL (unversioned/unknown version) 
./src/3rdparty/uchardet-0.0.1/src/nsMBCSSM.cpp: MPL (v1.1) GPL (unversioned/unknown version) 
./src/3rdparty/uchardet-0.0.1/src/nsEUCJPProber.h: MPL (v1.1) GPL (unversioned/unknown version) 
./src/3rdparty/uchardet-0.0.1/src/nsLatin1Prober.h: MPL (v1.1) GPL (unversioned/unknown version) 
./src/3rdparty/uchardet-0.0.1/src/nsBig5Prober.h: MPL (v1.1) GPL (unversioned/unknown version) 
./src/3rdparty/uchardet-0.0.1/src/nsSBCSGroupProber.h: MPL (v1.1) GPL (unversioned/unknown version) 
./src/3rdparty/uchardet-0.0.1/src/nsEUCKRProber.cpp: MPL (v1.1) GPL (unversioned/unknown version) 
./src/3rdparty/uchardet-0.0.1/src/LangCyrillicModel.cpp: MPL (v1.1) GPL (unversioned/unknown version) 
./src/3rdparty/uchardet-0.0.1/src/nsCodingStateMachine.h: MPL (v1.1) GPL (unversioned/unknown version) 
./src/3rdparty/uchardet-0.0.1/src/nsEUCKRProber.h: MPL (v1.1) GPL (unversioned/unknown version) 
./src/3rdparty/uchardet-0.0.1/src/nsUniversalDetector.cpp: MPL (v1.1) GPL (unversioned/unknown version) 
./src/3rdparty/uchardet-0.0.1/src/nsEscCharsetProber.cpp: MPL (v1.1) GPL (unversioned/unknown version) 
./src/3rdparty/uchardet-0.0.1/src/nsGB2312Prober.h: MPL (v1.1) GPL (unversioned/unknown version) 
./src/3rdparty/uchardet-0.0.1/src/nsUniversalDetector.h: MPL (v1.1) GPL (unversioned/unknown version) 
./src/3rdparty/uchardet-0.0.1/src/LangThaiModel.cpp: MPL (v1.1) GPL (unversioned/unknown version) 
./src/3rdparty/uchardet-0.0.1/src/JpCntx.cpp: MPL (v1.1) GPL (unversioned/unknown version) 
./src/3rdparty/uchardet-0.0.1/src/nsMBCSGroupProber.cpp: MPL (v1.1) GPL (unversioned/unknown version) 
./src/3rdparty/uchardet-0.0.1/src/nsHebrewProber.h: MPL (v1.1) GPL (unversioned/unknown version) 
./src/3rdparty/uchardet-0.0.1/src/nsEUCTWProber.cpp: MPL (v1.1) GPL (unversioned/unknown version) 
./src/3rdparty/uchardet-0.0.1/src/nsSBCSGroupProber.cpp: MPL (v1.1) GPL (unversioned/unknown version) 
./src/3rdparty/uchardet-0.0.1/src/CharDistribution.cpp: MPL (v1.1) GPL (unversioned/unknown version) 
./src/3rdparty/uchardet-0.0.1/src/nsSJISProber.h: MPL (v1.1) GPL (unversioned/unknown version) 
./src/3rdparty/uchardet-0.0.1/src/nsEUCJPProber.cpp: MPL (v1.1) GPL (unversioned/unknown version) 
./src/3rdparty/uchardet-0.0.1/src/nscore.h: MPL (v1.1) GPL (unversioned/unknown version) 
./src/3rdparty/uchardet-0.0.1/src/nsLatin1Prober.cpp: MPL (v1.1) GPL (unversioned/unknown version) 
./src/3rdparty/uchardet-0.0.1/src/nsHebrewProber.cpp: MPL (v1.1) GPL (unversioned/unknown version) 
./src/3rdparty/uchardet-0.0.1/src/nsEscCharsetProber.h: MPL (v1.1) GPL (unversioned/unknown version) 
./src/3rdparty/uchardet-0.0.1/src/nsUTF8Prober.cpp: MPL (v1.1) GPL (unversioned/unknown version) 
./src/3rdparty/uchardet-0.0.1/src/nsBig5Prober.cpp: MPL (v1.1) GPL (unversioned/unknown version) 
./src/3rdparty/uchardet-0.0.1/src/nsPkgInt.h: MPL (v1.1) GPL (unversioned/unknown version) 
./src/3rdparty/uchardet-0.0.1/src/nsUTF8Prober.h: MPL (v1.1) GPL (unversioned/unknown version) 
./src/3rdparty/uchardet-0.0.1/src/nsSJISProber.cpp: MPL (v1.1) GPL (unversioned/unknown version) 
./src/3rdparty/uchardet-0.0.1/src/nsMBCSGroupProber.h: MPL (v1.1) GPL (unversioned/unknown version) 
./src/3rdparty/uchardet-0.0.1/src/tools/uchardet.cpp: MPL (v1.1) GPL (unversioned/unknown version) 
./src/3rdparty/uchardet-0.0.1/src/LangBulgarianModel.cpp: MPL (v1.1) GPL (unversioned/unknown version) 
./src/3rdparty/uchardet-0.0.1/src/JpCntx.h: MPL (v1.1) GPL (unversioned/unknown version) 
./src/3rdparty/uchardet-0.0.1/src/prmem.h: MPL (v1.1) GPL (unversioned/unknown version) 
./src/ircmessage.cpp: LGPL (v2 or later) 
./src/irc.cpp: LGPL (v2 or later) 
./src/irccodecplugin.cpp: LGPL (v2 or later) 
./src/ircsession.cpp: LGPL (v2 or later) 
./src/irccommand.cpp: LGPL (v2 or later) 
./include/ircglobal.h: LGPL (v2 or later) 
./include/ircmessage.h: LGPL (v2 or later) 
./include/ircdecoder_p.h: LGPL (v2 or later) 
./include/ircsender.h: LGPL (v2 or later) 
./include/ircsession_p.h: LGPL (v2 or later) 
./include/ircsession.h: LGPL (v2 or later) 
./include/irccodecplugin.h: LGPL (v2 or later) 
./include/irccommand.h: LGPL (v2 or later) 
./include/ircparser_p.h: LGPL (v2 or later) 
./include/ircutil.h: LGPL (v2 or later) 
./include/irc.h: LGPL (v2 or later) 
./examples/shared/commandparser.h: GPL (v2 or later) 
./examples/shared/messageformatter.h: GPL (v2 or later) 
./examples/shared/messagereceiver.h: GPL (v2 or later) 
./examples/shared/session.h: GPL (v2 or later) 
./examples/shared/streamer.h: GPL (v2 or later) 
./examples/shared/messageformatter.cpp: GPL (v2 or later) 
./examples/shared/usermodel.h: GPL (v2 or later) 
./examples/shared/session.cpp: GPL (v2 or later) 
./examples/shared/commandparser.cpp: GPL (v2 or later) 
./examples/shared/messagehandler.h: GPL (v2 or later) 
./examples/shared/channelinfo.h: GPL (v2 or later) 
./examples/shared/usermodel.cpp: GPL (v2 or later) 
./examples/shared/messagehandler.cpp: GPL (v2 or later) 
./examples/shared/connectioninfo.h: GPL (v2 or later) 
./examples/bot/ircbot.h: GPL (v2 or later) 
./examples/bot/main.cpp: GPL (v2 or later) 
./examples/bot/ircbot.cpp: GPL (v2 or later) 
./examples/desktop/src/wizard/generalwizardpage.cpp: GPL (v2 or later) 
./examples/desktop/src/wizard/serverwizardpage.cpp: GPL (v2 or later) 
./examples/desktop/src/wizard/userwizardpage.cpp: GPL (v2 or later) 
./examples/desktop/src/wizard/shortcutswizardpage.cpp: GPL (v2 or later) 
./examples/desktop/src/wizard/colorswizardpage.h: GPL (v2 or later) 
./examples/desktop/src/wizard/settingswizard.h: GPL (v2 or later) 
./examples/desktop/src/wizard/treewidget.h: GPL (v2 or later) 
./examples/desktop/src/wizard/settingswizard.cpp: GPL (v2 or later) 
./examples/desktop/src/wizard/serverwizardpage.h: GPL (v2 or later) 
./examples/desktop/src/wizard/generalwizardpage.h: GPL (v2 or later) 
./examples/desktop/src/wizard/connectionwizard.h: GPL (v2 or later) 
./examples/desktop/src/wizard/messageswizardpage.cpp: GPL (v2 or later) 
./examples/desktop/src/wizard/connectionwizardpage.cpp: GPL (v2 or later) 
./examples/desktop/src/wizard/treewidget.cpp: GPL (v2 or later) 
./examples/desktop/src/wizard/userwizardpage.h: GPL (v2 or later) 
./examples/desktop/src/wizard/connectionwizardpage.h: GPL (v2 or later) 
./examples/desktop/src/wizard/messageswizardpage.h: GPL (v2 or later) 
./examples/desktop/src/wizard/colorswizardpage.cpp: GPL (v2 or later) 
./examples/desktop/src/wizard/connectionwizard.cpp: GPL (v2 or later) 
./examples/desktop/src/wizard/shortcutswizardpage.h: GPL (v2 or later) 
./examples/desktop/src/application.h: GPL (v2 or later) 
./examples/desktop/src/mainwindow.cpp: GPL (v2 or later) 
./examples/desktop/src/homepage.cpp: GPL (v2 or later) 
./examples/desktop/src/main.cpp: GPL (v2 or later) 
./examples/desktop/src/homepage.h: GPL (v2 or later) 
./examples/desktop/src/3rdparty/qtwin/qtwin.h: UNKNOWN
./examples/desktop/src/3rdparty/qtwin/qtwin.cpp: UNKNOWN
./examples/desktop/src/3rdparty/qtdocktile/qtdocktile_unity.cpp: LGPL (v3 or later) 
./examples/desktop/src/3rdparty/qtdocktile/qtdocktile.h: LGPL (v3 or later) 
./examples/desktop/src/3rdparty/qtdocktile/qtdocktile_win.cpp: LGPL (v3 or later) 
./examples/desktop/src/3rdparty/qtdocktile/winutils.h: LGPL (v2.1 or later) 
./examples/desktop/src/3rdparty/qtdocktile/qtdocktile_p.h: LGPL (v3 or later) 
./examples/desktop/src/3rdparty/qtdocktile/qtdocktile.cpp: LGPL (v3 or later) 
./examples/desktop/src/gui/util/textbrowser.h: GPL (v2 or later) 
./examples/desktop/src/gui/util/historylineedit.h: GPL (v2 or later) 
./examples/desktop/src/gui/util/textbrowser.cpp: GPL (v2 or later) 
./examples/desktop/src/gui/util/sharedtimer.h: GPL (v2 or later) 
./examples/desktop/src/gui/util/completer.cpp: GPL (v2 or later) 
./examples/desktop/src/gui/util/completer.h: GPL (v2 or later) 
./examples/desktop/src/gui/util/historylineedit.cpp: GPL (v2 or later) 
./examples/desktop/src/gui/util/sharedtimer.cpp: GPL (v2 or later) 
./examples/desktop/src/gui/util/tabwidget.h: GPL (v2 or later) 
./examples/desktop/src/gui/util/tabwidget.cpp: GPL (v2 or later) 
./examples/desktop/src/gui/util/tabwidget_p.h: GPL (v2 or later) 
./examples/desktop/src/gui/sessiontabwidget.cpp: GPL (v2 or later) 
./examples/desktop/src/gui/searcheditor.cpp: GPL (v2 or later) 
./examples/desktop/src/gui/searcheditor.h: GPL (v2 or later) 
./examples/desktop/src/gui/multisessiontabwidget.cpp: GPL (v2 or later) 
./examples/desktop/src/gui/sortedusermodel.h: GPL (v2 or later) 
./examples/desktop/src/gui/settings.h: GPL (v2 or later) 
./examples/desktop/src/gui/sortedusermodel.cpp: GPL (v2 or later) 
./examples/desktop/src/gui/sessiontabwidget.h: GPL (v2 or later) 
./examples/desktop/src/gui/3rdparty/fancylineedit.cpp: UNKNOWN
./examples/desktop/src/gui/3rdparty/fancylineedit.h: UNKNOWN
./examples/desktop/src/gui/messageview.cpp: GPL (v2 or later) 
./examples/desktop/src/gui/lineeditor.h: GPL (v2 or later) 
./examples/desktop/src/gui/settings.cpp: GPL (v2 or later) 
./examples/desktop/src/gui/multisessiontabwidget.h: GPL (v2 or later) 
./examples/desktop/src/gui/messageview.h: GPL (v2 or later) 
./examples/desktop/src/gui/lineeditor.cpp: GPL (v2 or later) 
./examples/desktop/src/mainwindow.h: GPL (v2 or later) 
./examples/desktop/src/trayicon.h: GPL (v2 or later) 
./examples/desktop/src/trayicon.cpp: GPL (v2 or later) 
./examples/desktop/src/application.cpp: GPL (v2 or later) 
./examples/mobile/src/sessionitem.h: GPL (v2 or later) 
./examples/mobile/src/abstractsessionitem.cpp: GPL (v2 or later) 
./examples/mobile/src/sessionitem.cpp: GPL (v2 or later) 
./examples/mobile/src/main.cpp: GPL (v2 or later) 
./examples/mobile/src/settings.h: GPL (v2 or later) 
./examples/mobile/src/completer.cpp: GPL (v2 or later) 
./examples/mobile/src/completer.h: GPL (v2 or later) 
./examples/mobile/src/sessionchilditem.cpp: GPL (v2 or later) 
./examples/mobile/src/settings.cpp: GPL (v2 or later) 
./examples/mobile/src/abstractsessionitem.h: GPL (v2 or later) 
./examples/mobile/src/sessionmanager.h: GPL (v2 or later) 
./examples/mobile/src/sessionchilditem.h: GPL (v2 or later) 
./examples/mobile/src/sessionmanager.cpp: GPL (v2 or later) 
./examples/mobile/qmlapplicationviewer/qmlapplicationviewer.h: *No copyright* GENERATED FILE
./examples/mobile/qmlapplicationviewer/qmlapplicationviewer.cpp: *No copyright* GENERATED FILE

Comment 16 Dan Mashal 2012-08-12 00:40:05 UTC

J-P:

There are some other issues that need to be fixed upstream.


$tar -zxvf communi-1.1.0.tar.gz 
communi-communi-68ddd57/


This should NOT extract to this dir. This should extract to communi-1.1.0/

I will try and work around this and post a better spec and tarball.

Comment 17 J-P Nurmi 2012-08-12 00:52:40 UTC

I believe that tarball is actually made by GitHub.

Comment 18 Dan Mashal 2012-08-12 00:53:52 UTC

Yes. It is. I've created my own tarball and spec file based off the original one. I'll post some notes shortly.

Comment 19 Dan Mashal 2012-08-12 01:00:35 UTC

J-P:

Does the make file respect DESTDIR? That's a major blocker.


+ make install DESTDIR=/home/dan/rpmbuild/BUILDROOT/communi-1.1.0-1.fc17.x86_64
cd src/ && make -f Makefile install
make[1]: Entering directory `/home/dan/rpmbuild/BUILD/communi-1.1.0/src'
install -m 755 -p "../lib/libCommuni.so.1.2.0" "/usr/lib64/libCommuni.so.1.2.0"
install: cannot remove `/usr/lib64/libCommuni.so.1.2.0': Permission denied
make[1]: [install_target] Error 1 (ignored)
ln -f -s "libCommuni.so.1.2.0" "/usr/lib64/libCommuni.so"
ln: cannot remove `/usr/lib64/libCommuni.so': Permission denied
make[1]: [install_target] Error 1 (ignored)
ln -f -s "libCommuni.so.1.2.0" "/usr/lib64/libCommuni.so.1"
ln: cannot remove `/usr/lib64/libCommuni.so.1': Permission denied
make[1]: [install_target] Error 1 (ignored)
ln -f -s "libCommuni.so.1.2.0" "/usr/lib64/libCommuni.so.1.2"
ln: cannot remove `/usr/lib64/libCommuni.so.1.2': Permission denied
make[1]: [install_target] Error 1 (ignored)
install -m 644 -p /home/dan/rpmbuild/BUILD/communi-1.1.0/include/irc.h /usr/include/Communi/
install: cannot remove `/usr/include/Communi/irc.h': Permission denied
make[1]: [install_headers] Error 1 (ignored)
install -m 644 -p /home/dan/rpmbuild/BUILD/communi-1.1.0/include/irccommand.h /usr/include/Communi/
install: cannot remove `/usr/include/Communi/irccommand.h': Permission denied
make[1]: [install_headers] Error 1 (ignored)
install -m 644 -p /home/dan/rpmbuild/BUILD/communi-1.1.0/include/irccodecplugin.h /usr/include/Communi/
install: cannot remove `/usr/include/Communi/irccodecplugin.h': Permission denied
make[1]: [install_headers] Error 1 (ignored)
install -m 644 -p /home/dan/rpmbuild/BUILD/communi-1.1.0/include/ircglobal.h /usr/include/Communi/
install: cannot remove `/usr/include/Communi/ircglobal.h': Permission denied
make[1]: [install_headers] Error 1 (ignored)
install -m 644 -p /home/dan/rpmbuild/BUILD/communi-1.1.0/include/ircmessage.h /usr/include/Communi/
install: cannot remove `/usr/include/Communi/ircmessage.h': Permission denied
make[1]: [install_headers] Error 1 (ignored)
install -m 644 -p /home/dan/rpmbuild/BUILD/communi-1.1.0/include/ircsender.h /usr/include/Communi/
install: cannot remove `/usr/include/Communi/ircsender.h': Permission denied
make[1]: [install_headers] Error 1 (ignored)
install -m 644 -p /home/dan/rpmbuild/BUILD/communi-1.1.0/include/ircsession.h /usr/include/Communi/
install: cannot remove `/usr/include/Communi/ircsession.h': Permission denied
make[1]: [install_headers] Error 1 (ignored)
install -m 644 -p /home/dan/rpmbuild/BUILD/communi-1.1.0/include/ircutil.h /usr/include/Communi/
install: cannot remove `/usr/include/Communi/ircutil.h': Permission denied
make[1]: [install_headers] Error 1 (ignored)
install -m 644 -p /home/dan/rpmbuild/BUILD/communi-1.1.0/include/Irc /usr/include/Communi/
install: cannot remove `/usr/include/Communi/Irc': Permission denied
make[1]: [install_headers] Error 1 (ignored)
install -m 644 -p /home/dan/rpmbuild/BUILD/communi-1.1.0/include/IrcCommand /usr/include/Communi/
install: cannot remove `/usr/include/Communi/IrcCommand': Permission denied
make[1]: [install_headers] Error 1 (ignored)
install -m 644 -p /home/dan/rpmbuild/BUILD/communi-1.1.0/include/IrcCodecPlugin /usr/include/Communi/
install: cannot remove `/usr/include/Communi/IrcCodecPlugin': Permission denied
make[1]: [install_headers] Error 1 (ignored)
install -m 644 -p /home/dan/rpmbuild/BUILD/communi-1.1.0/include/IrcGlobal /usr/include/Communi/
install: cannot remove `/usr/include/Communi/IrcGlobal': Permission denied
make[1]: [install_headers] Error 1 (ignored)
install -m 644 -p /home/dan/rpmbuild/BUILD/communi-1.1.0/include/IrcMessage /usr/include/Communi/
install: cannot remove `/usr/include/Communi/IrcMessage': Permission denied
make[1]: [install_headers] Error 1 (ignored)
install -m 644 -p /home/dan/rpmbuild/BUILD/communi-1.1.0/include/IrcSender /usr/include/Communi/
install: cannot remove `/usr/include/Communi/IrcSender': Permission denied
make[1]: [install_headers] Error 1 (ignored)
install -m 644 -p /home/dan/rpmbuild/BUILD/communi-1.1.0/include/IrcSession /usr/include/Communi/
install: cannot remove `/usr/include/Communi/IrcSession': Permission denied
make[1]: [install_headers] Error 1 (ignored)
install -m 644 -p /home/dan/rpmbuild/BUILD/communi-1.1.0/include/IrcUtil /usr/include/Communi/
install: cannot remove `/usr/include/Communi/IrcUtil': Permission denied
make[1]: [install_headers] Error 1 (ignored)
make[1]: Leaving directory `/home/dan/rpmbuild/BUILD/communi-1.1.0/src'
cd src/plugins/ && make -f Makefile install
make[1]: Entering directory `/home/dan/rpmbuild/BUILD/communi-1.1.0/src/plugins'
cd uchardet/ && make -f Makefile install
make[2]: Entering directory `/home/dan/rpmbuild/BUILD/communi-1.1.0/src/plugins/uchardet'
install -m 755 -p "../../../plugins/communi/libuchardetplugin.so" "/usr/lib64/qt4/plugins/communi/libuchardetplugin.so"
install: cannot remove `/usr/lib64/qt4/plugins/communi/libuchardetplugin.so': Permission denied
make[2]: [install_target] Error 1 (ignored)
make[2]: Leaving directory `/home/dan/rpmbuild/BUILD/communi-1.1.0/src/plugins/uchardet'
cd declarative/ && make -f Makefile install
make[2]: Entering directory `/home/dan/rpmbuild/BUILD/communi-1.1.0/src/plugins/declarative'
install -m 755 -p "../../../imports/Communi/libcommuniplugin.so" "/usr/lib64/qt4/imports/Communi/libcommuniplugin.so"
install: cannot remove `/usr/lib64/qt4/imports/Communi/libcommuniplugin.so': Permission denied
make[2]: [install_target] Error 1 (ignored)
install -m 644 -p /home/dan/rpmbuild/BUILD/communi-1.1.0/src/plugins/declarative/qmldir /usr/lib64/qt4/imports/Communi/
install: cannot remove `/usr/lib64/qt4/imports/Communi/qmldir': Permission denied
make[2]: [install_other_files] Error 1 (ignored)
make[2]: Leaving directory `/home/dan/rpmbuild/BUILD/communi-1.1.0/src/plugins/declarative'
make[1]: Leaving directory `/home/dan/rpmbuild/BUILD/communi-1.1.0/src/plugins'
cd tests/ && make -f Makefile install
make[1]: Entering directory `/home/dan/rpmbuild/BUILD/communi-1.1.0/tests'
cd auto/ && make -f Makefile install
make[2]: Entering directory `/home/dan/rpmbuild/BUILD/communi-1.1.0/tests/auto'
cd irccommand/ && make -f Makefile install
make[3]: Entering directory `/home/dan/rpmbuild/BUILD/communi-1.1.0/tests/auto/irccommand'
make[3]: Nothing to be done for `install'.
make[3]: Leaving directory `/home/dan/rpmbuild/BUILD/communi-1.1.0/tests/auto/irccommand'
cd ircdecoder/ && make -f Makefile install
make[3]: Entering directory `/home/dan/rpmbuild/BUILD/communi-1.1.0/tests/auto/ircdecoder'
make[3]: Nothing to be done for `install'.
make[3]: Leaving directory `/home/dan/rpmbuild/BUILD/communi-1.1.0/tests/auto/ircdecoder'
cd ircmessage/ && make -f Makefile install
make[3]: Entering directory `/home/dan/rpmbuild/BUILD/communi-1.1.0/tests/auto/ircmessage'
make[3]: Nothing to be done for `install'.
make[3]: Leaving directory `/home/dan/rpmbuild/BUILD/communi-1.1.0/tests/auto/ircmessage'
cd ircparser/ && make -f Makefile install
make[3]: Entering directory `/home/dan/rpmbuild/BUILD/communi-1.1.0/tests/auto/ircparser'
make[3]: Nothing to be done for `install'.
make[3]: Leaving directory `/home/dan/rpmbuild/BUILD/communi-1.1.0/tests/auto/ircparser'
cd ircsender/ && make -f Makefile install
make[3]: Entering directory `/home/dan/rpmbuild/BUILD/communi-1.1.0/tests/auto/ircsender'
make[3]: Nothing to be done for `install'.
make[3]: Leaving directory `/home/dan/rpmbuild/BUILD/communi-1.1.0/tests/auto/ircsender'
cd ircsession/ && make -f Makefile install
make[3]: Entering directory `/home/dan/rpmbuild/BUILD/communi-1.1.0/tests/auto/ircsession'
make[3]: Nothing to be done for `install'.
make[3]: Leaving directory `/home/dan/rpmbuild/BUILD/communi-1.1.0/tests/auto/ircsession'
make[2]: Leaving directory `/home/dan/rpmbuild/BUILD/communi-1.1.0/tests/auto'
make[1]: Leaving directory `/home/dan/rpmbuild/BUILD/communi-1.1.0/tests'
cd examples/ && make -f Makefile install
make[1]: Entering directory `/home/dan/rpmbuild/BUILD/communi-1.1.0/examples'
cd bot/ && make -f Makefile install
make[2]: Entering directory `/home/dan/rpmbuild/BUILD/communi-1.1.0/examples/bot'
make[2]: Nothing to be done for `install'.
make[2]: Leaving directory `/home/dan/rpmbuild/BUILD/communi-1.1.0/examples/bot'
cd desktop/ && make -f Makefile.communi install
make[2]: Entering directory `/home/dan/rpmbuild/BUILD/communi-1.1.0/examples/desktop'
install -m 755 -p "bin/communi" "/usr/bin/communi"
install: cannot remove `/usr/bin/communi': Permission denied
make[2]: [install_target] Error 1 (ignored)
install -m 644 -p /home/dan/rpmbuild/BUILD/communi-1.1.0/examples/desktop/resources/icons/128x128/communi.png /usr/share/pixmaps/
install: cannot remove `/usr/share/pixmaps/communi.png': Permission denied
make[2]: [install_icon] Error 1 (ignored)
install -m 644 -p /home/dan/rpmbuild/BUILD/communi-1.1.0/examples/desktop/resources/communi.desktop /usr/share/applications/
install: cannot remove `/usr/share/applications/communi.desktop': Permission denied
make[2]: [install_desktop] Error 1 (ignored)
make[2]: Leaving directory `/home/dan/rpmbuild/BUILD/communi-1.1.0/examples/desktop'
make[1]: Leaving directory `/home/dan/rpmbuild/BUILD/communi-1.1.0/examples'
install -m 644 -p /home/dan/rpmbuild/BUILD/communi-1.1.0/features/communi.prf /usr/lib64/qt4/mkspecs/features/
install: cannot remove `/usr/lib64/qt4/mkspecs/features/communi.prf': Permission denied
make: [install_mkspecs] Error 1 (ignored)
install -m 644 -p /home/dan/rpmbuild/BUILD/communi-1.1.0/communi-config.prf /usr/lib64/qt4/mkspecs/features/
install: cannot remove `/usr/lib64/qt4/mkspecs/features/communi-config.prf': Permission denied

Comment 20 J-P Nurmi 2012-08-12 07:51:45 UTC

make install INSTALL_ROOT=/home/dan/rpmbuild/BUILDROOT/communi-1.1.0-1.fc17.x86_64

Comment 21 J-P Nurmi 2012-08-12 08:00:55 UTC

Hey Dan, I just noticed from the output above that it seems to be the unstable development version (1.2) from the master branch. Please use the 1.1 branch instead.

Comment 22 Jan Kaluža 2012-08-13 07:43:56 UTC

Ehm, why not to check my spec file which works? You would see I'm using qmake-qt4 and INSTALL_ROOT there...

Comment 23 Jan Kaluža 2012-09-05 13:05:25 UTC

Ok, finally had some time to update this review. After discussion with upstream, we've decided to rename the package to "libcommuni" and added "communi" subpackage which contains the Communi client. This subpackage is not built for EPEL-6, because the Qt is too old there.

Spec URL: http://jkaluza.fedorapeople.org/libcommuni.spec
SRPM URL: http://jkaluza.fedorapeople.org/libcommuni-1.1.2-1.fc17.src.rpm

Comment 24 Dan Mashal 2012-09-05 13:06:01 UTC

sounds good, will check it now.

Comment 25 Dan Mashal 2012-09-05 13:24:24 UTC

Please look at the must items and rpmlint items below. This is very close to approval.


Package Review
==============

Key:
- = N/A
x = Pass
! = Fail
? = Not evaluated



==== C/C++ ====
[x]: MUST Header files in -devel subpackage, if present.
[x]: MUST ldconfig called in %post and %postun if required.
[x]: MUST Package does not contain any libtool archives (.la)
[ ]: MUST Package does not contain kernel modules.
[ ]: MUST Package contains no static executables.
[x]: MUST Rpath absent or only used for internal libs.
[x]: MUST Development (unversioned) .so files in -devel subpackage, if
     present.


==== Generic ====
[x]: EXTRA Rpmlint is run on all installed packages.
     Note: There are rpmlint messages (see attachment).
[x]: EXTRA Spec file according to URL is the same as in SRPM.
[ ]: MUST Package is licensed with an open-source compatible license and meets
     other legal requirements as defined in the legal section of Packaging
     Guidelines.
[x]: MUST Package successfully compiles and builds into binary rpms on at
     least one supported primary architecture.
[ ]: MUST %build honors applicable compiler flags or justifies otherwise.
[x]: MUST All build dependencies are listed in BuildRequires, except for any
     that are listed in the exceptions section of Packaging Guidelines.
[ ]: MUST Package contains no bundled libraries.
[ ]: MUST Changelog in prescribed format.
[ ]: MUST Sources contain only permissible code or content.
[!]: MUST Each %files section contains %defattr if rpm < 4.4
     Note: defattr(....) present in %files devel section. This is OK if
     packaging for EPEL5. Otherwise not needed
[ ]: MUST Macros in Summary, %description expandable at SRPM build time.
[x]: MUST Package contains desktop file if it is a GUI application.
[!]: MUST Package installs a %{name}.desktop using desktop-file-install if
     there is such a file.
[ ]: MUST Development files must be in a -devel package
[ ]: MUST Package requires other packages for directories it uses.
[ ]: MUST Package uses nothing in %doc for runtime.
[ ]: MUST Package is not known to require ExcludeArch.
[x]: MUST Permissions on files are set properly.
[x]: MUST Package does not contain duplicates in %files.
[x]: MUST Fully versioned dependency in subpackages, if present.
[ ]: MUST Package complies to the Packaging Guidelines
[x]: MUST Spec file lacks Packager, Vendor, PreReq tags.
[x]: MUST Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the
     beginning of %install.
     Note: rm -rf would be needed if support for EPEL5 is required
[ ]: MUST Large documentation files are in a -doc subpackage, if required.
[x]: MUST If (and only if) the source package includes the text of the
     license(s) in its own file, then that file, containing the text of the
     license(s) for the package is included in %doc.
[ ]: MUST License field in the package spec file matches the actual license.
     Note: Checking patched sources after %prep for licenses. Licenses found:
     "LGPL (v2 or later)", "GPL (v2 or later)", "LGPL (v3 or later)", "MPL
     (v1.1) GPL (unversioned/unknown version)", "LGPL (v2.1 or later)" For
     detailed output of licensecheck see file:
     /home/dan/846913-libcommuni/licensecheck.txt
[ ]: MUST License file installed when any subpackage combination is installed.
[ ]: MUST Package consistently uses macro is (instead of hard-coded directory
     names).
[x]: MUST Package is named using only allowed ascii characters.
[ ]: MUST Package is named according to the Package Naming Guidelines.
[ ]: MUST Package does not generate any conflict.
     Note: Package contains no Conflicts: tag(s)
[ ]: MUST Package obeys FHS, except libexecdir and /usr/target.
[ ]: MUST If the package is a rename of another package, proper Obsoletes and
     Provides are present.
[ ]: MUST Package must own all directories that it creates.
[ ]: MUST Package does not own files or directories owned by other packages.
[x]: MUST Package installs properly.
[ ]: MUST Package is not relocatable.
[ ]: MUST Requires correct, justified where necessary.
[x]: MUST Rpmlint is run on all rpms the build produces.
     Note: There are rpmlint messages (see attachment).
[x]: MUST Sources used to build the package match the upstream source, as
     provided in the spec URL.
[ ]: MUST Spec file is legible and written in American English.
[x]: MUST Spec file name must match the spec package %{name}, in the format
     %{name}.spec.
[ ]: MUST Package contains systemd file(s) if in need.
[x]: MUST File names are valid UTF-8.
[ ]: MUST Useful -debuginfo package or justification otherwise.
[x]: SHOULD Reviewer should test that the package builds in mock.
[x]: SHOULD Buildroot is not present
     Note: Unless packager wants to package for EPEL5 this is fine
[x]: SHOULD Package has no %clean section with rm -rf %{buildroot} (or
     $RPM_BUILD_ROOT)
     Note: Clean would be needed if support for EPEL5 is required
[ ]: SHOULD If the source package does not include license text(s) as a
     separate file from upstream, the packager SHOULD query upstream to
     include it.
[x]: SHOULD Dist tag is present.
[x]: SHOULD No file requires outside of /etc, /bin, /sbin, /usr/bin,
     /usr/sbin.
[ ]: SHOULD Final provides and requires are sane (rpm -q --provides and rpm -q
     --requires).
[ ]: SHOULD Package functions as described.
[ ]: SHOULD Latest version is packaged.
[ ]: SHOULD Package does not include license text files separate from
     upstream.
[ ]: SHOULD Patches link to upstream bugs/comments/lists or are otherwise
     justified.
[ ]: SHOULD Scriptlets must be sane, if used.
[x]: SHOULD SourceX tarball generation or download is documented.
[!]: SHOULD SourceX / PatchY prefixed with %{name}.
     Note: Patch0 (desktop.patch) Source0 (communi-1.1.2.tar.gz)
[x]: SHOULD SourceX is a working URL.
[ ]: SHOULD Description and summary sections in the package spec file contains
     translations for supported Non-English languages, if available.
[ ]: SHOULD Package should compile and build into binary rpms on all supported
     architectures.
[ ]: SHOULD %check is present and all tests pass.
[ ]: SHOULD Packages should try to preserve timestamps of original installed
     files.
[!]: SHOULD Spec use %global instead of %define.
     Note: %define tardirname communi-communi-939dc37

Issues:
[!]: MUST Each %files section contains %defattr if rpm < 4.4
     Note: defattr(....) present in %files devel section. This is OK if
     packaging for EPEL5. Otherwise not needed
See: http://fedoraproject.org/wiki/Packaging/Guidelines#FilePermissions
[!]: MUST Package installs a %{name}.desktop using desktop-file-install if
     there is such a file.
See: http://fedoraproject.org/wiki/Packaging/Guidelines#desktop

Rpmlint
-------
Checking: communi-1.1.2-1.fc17.x86_64.rpm
          libcommuni-1.1.2-1.fc17.src.rpm
          libcommuni-debuginfo-1.1.2-1.fc17.x86_64.rpm
          libcommuni-1.1.2-1.fc17.x86_64.rpm
          libcommuni-devel-1.1.2-1.fc17.x86_64.rpm
communi.x86_64: W: obsolete-not-provided libircclient-qt-devel
communi.x86_64: W: no-manual-page-for-binary communi
libcommuni.src: W: spelling-error %description -l en_US Communi -> Commune, Comm uni, Comm-uni
libcommuni.x86_64: W: spelling-error %description -l en_US Communi -> Commune, Comm uni, Comm-uni
libcommuni.x86_64: W: obsolete-not-provided libircclient-qt
libcommuni-devel.x86_64: W: obsolete-not-provided libircclient-qt-devel
5 packages and 0 specfiles checked; 0 errors, 6 warnings.


Rpmlint (installed packages)
----------------------------
# rpmlint libcommuni-devel libcommuni-debuginfo
libcommuni-devel.x86_64: W: obsolete-not-provided libircclient-qt-devel
2 packages and 0 specfiles checked; 0 errors, 1 warnings.
# echo 'rpmlint-done:'

Requires
--------
communi-1.1.2-1.fc17.x86_64.rpm (rpmlib, GLIBC filtered):
    
    libCommuni.so.1()(64bit)  
    libQtCore.so.4()(64bit)  
    libQtDBus.so.4()(64bit)  
    libQtGui.so.4()(64bit)  
    libQtNetwork.so.4()(64bit)  
    libQtXml.so.4()(64bit)  
    libc.so.6()(64bit)  
    libcommuni(x86-64) = 1.1.2-1.fc17
    libgcc_s.so.1()(64bit)  
    libgcc_s.so.1(GCC_3.0)(64bit)  
    libm.so.6()(64bit)  
    libpthread.so.0()(64bit)  
    libstdc++.so.6()(64bit)  
    libstdc++.so.6(CXXABI_1.3)(64bit)  
    rtld(GNU_HASH)  

libcommuni-debuginfo-1.1.2-1.fc17.x86_64.rpm (rpmlib, GLIBC filtered):
    

libcommuni-1.1.2-1.fc17.x86_64.rpm (rpmlib, GLIBC filtered):
    
    /sbin/ldconfig  
    libQtCore.so.4()(64bit)  
    libQtNetwork.so.4()(64bit)  
    libc.so.6()(64bit)  
    libgcc_s.so.1()(64bit)  
    libgcc_s.so.1(GCC_3.0)(64bit)  
    libm.so.6()(64bit)  
    libpthread.so.0()(64bit)  
    libstdc++.so.6()(64bit)  
    libstdc++.so.6(CXXABI_1.3)(64bit)  
    rtld(GNU_HASH)  

libcommuni-devel-1.1.2-1.fc17.x86_64.rpm (rpmlib, GLIBC filtered):
    
    libCommuni.so.1()(64bit)  
    libQtCore.so.4()(64bit)  
    libQtDeclarative.so.4()(64bit)  
    libQtGui.so.4()(64bit)  
    libQtNetwork.so.4()(64bit)  
    libQtScript.so.4()(64bit)  
    libQtSql.so.4()(64bit)  
    libQtSvg.so.4()(64bit)  
    libQtXmlPatterns.so.4()(64bit)  
    libc.so.6()(64bit)  
    libcommuni(x86-64) = 1.1.2-1.fc17
    libgcc_s.so.1()(64bit)  
    libgcc_s.so.1(GCC_3.0)(64bit)  
    libm.so.6()(64bit)  
    libpthread.so.0()(64bit)  
    libstdc++.so.6()(64bit)  
    libstdc++.so.6(CXXABI_1.3)(64bit)  
    rtld(GNU_HASH)  

Provides
--------
communi-1.1.2-1.fc17.x86_64.rpm:
    
    communi = 1.1.2-1.fc17
    communi(x86-64) = 1.1.2-1.fc17

libcommuni-debuginfo-1.1.2-1.fc17.x86_64.rpm:
    
    libcommuni-debuginfo = 1.1.2-1.fc17
    libcommuni-debuginfo(x86-64) = 1.1.2-1.fc17

libcommuni-1.1.2-1.fc17.x86_64.rpm:
    
    libCommuni.so.1()(64bit)  
    libcommuni = 1.1.2-1.fc17
    libcommuni(x86-64) = 1.1.2-1.fc17

libcommuni-devel-1.1.2-1.fc17.x86_64.rpm:
    
    libcommuni-devel = 1.1.2-1.fc17
    libcommuni-devel(x86-64) = 1.1.2-1.fc17
    libcommuniplugin.so()(64bit)  

MD5-sum check
-------------
http://github.com/communi/communi/tarball/v1.1.2/communi-1.1.2.tar.gz :
  CHECKSUM(SHA256) this package     : 7f319f79e5505e3409ae481ac21637e30ad9cfb909b93629bd4feb63def4482f
  CHECKSUM(SHA256) upstream package : 7f319f79e5505e3409ae481ac21637e30ad9cfb909b93629bd4feb63def4482f


Generated by fedora-review 0.2.2 (9f8c0e5) last change: 2012-08-09
Command line :/usr/bin/fedora-review -b 846913
External plugins:

Comment 26 Dan Mashal 2012-09-05 13:31:58 UTC

Regarding the rename:

package should be named communi and have a communi-libs subpackage.

i.e. 

%package libs

%package devel
shared libraries, headers, .pc files go hereles go here

%files libs

%files devel

See:
http://fedoraproject.org/wiki/How_to_create_an_RPM_package#Subpackages
http://fedoraproject.org/wiki/Packaging:Guidelines#Devel_Packages

Comment 27 Dan Mashal 2012-09-05 13:34:39 UTC

Sorry, didn't mean to cancel review flag, getting that set back to ? shortly.

Comment 28 J-P Nurmi 2012-09-05 15:10:28 UTC

Just keep in mind that the framework is the main product. The desktop client is just optional extra. Having said that, applications should be able to use libcommuni(-devel) without installing the example client. Embedding libcommuni to other applications as suggested on #communi is IMHO not an acceptable solution.

Comment 29 Dan Mashal 2012-09-06 02:29:19 UTC

thanks JP

Comment 30 Jan Kaluža 2012-09-06 06:03:14 UTC

Thanks for the review, I think I've fixed the bugs you've found.

Spec URL: http://jkaluza.fedorapeople.org/libcommuni.spec
SRPM URL: http://jkaluza.fedorapeople.org/libcommuni-1.1.2-1.fc17.src.rpm

(In reply to comment #25)
> [!]: MUST Package installs a %{name}.desktop using desktop-file-install if
>      there is such a file.

Fixed.

> [!]: SHOULD SourceX / PatchY prefixed with %{name}.
>      Note: Patch0 (desktop.patch) Source0 (communi-1.1.2.tar.gz)

Fixed, Patch0 is no longer there.

> [!]: SHOULD Spec use %global instead of %define.
>      Note: %define tardirname communi-communi-939dc37

Fixed.

> Issues:
> [!]: MUST Each %files section contains %defattr if rpm < 4.4
>      Note: defattr(....) present in %files devel section. This is OK if
>      packaging for EPEL5. Otherwise not needed

I plan to add this package only to EPEL6 and Fedora, so no need for %defattr.
> Rpmlint
> -------
> communi.x86_64: W: obsolete-not-provided libircclient-qt-devel

"communi" package should not obsolete libircclient-qt-devel, fixed.

> communi.x86_64: W: no-manual-page-for-binary communi

This is not review blocker.

> libcommuni.x86_64: W: obsolete-not-provided libircclient-qt
> libcommuni-devel.x86_64: W: obsolete-not-provided libircclient-qt-devel

That's according to guidelines I think. See the Comment 1.

Comment 31 Jan Kaluža 2012-09-06 06:03:54 UTC

Sorry, I meant Comment 0 :).

Comment 32 Matěj Cepl 2012-10-03 19:24:15 UTC

All issues indicated in comment 1 and comment 25 have been resolved.

Builds in koji (http://koji.fedoraproject.org/koji/taskinfo?taskID=4557061)

APPROVED!

Comment 33 Dan Mashal 2012-10-03 20:57:23 UTC

cool thanks

Comment 34 Jan Kaluža 2012-10-04 05:22:20 UTC

New Package SCM Request
=======================
Package Name: libcommuni
Short Description: Communi is a cross-platform IRC client library written with Qt 4
Owners: jkaluza
Branches: f17 f18 el6

Comment 35 Gwyn Ciesla 2012-10-04 11:18:55 UTC

Git done (by process-git-requests).

Comment 36 Fedora Update System 2012-10-09 08:43:13 UTC

libcommuni-1.2.0-1.fc18 has been submitted as an update for Fedora 18.
https://admin.fedoraproject.org/updates/libcommuni-1.2.0-1.fc18

Comment 37 Fedora Update System 2012-10-09 08:57:36 UTC

libcommuni-1.2.0-1.fc17 has been submitted as an update for Fedora 17.
https://admin.fedoraproject.org/updates/libcommuni-1.2.0-1.fc17

Comment 38 Fedora Update System 2012-10-09 17:19:39 UTC

libcommuni-1.2.0-1.fc18 has been pushed to the Fedora 18 testing repository.

Comment 39 Fedora Update System 2012-10-10 06:33:18 UTC

libcommuni-1.2.0-1.el6 has been submitted as an update for Fedora EPEL 6.
https://admin.fedoraproject.org/updates/libcommuni-1.2.0-1.el6

Comment 40 Jan Kaluža 2012-10-29 13:43:26 UTC

Closing this one, package is in Fedora/EPEL now. Thanks you all for a review.