Bug 461393

Since the included icon files have separate licensing and that licensing is intact in the final built RPMs, you need to indicate the various licenses in the package.  You should have at least "License: GPLv3+ and CC-BY-SA" with a comment about which png file carries the separate license, but you should also investigate the license of those icon-*.png files and see which GPL versions they are under.  (The LICENSE.txt files doesn't specify.)  If they're not GPLv3+ as well, then you'll need to indicate them in License, too.

A desktop file is required so that this package properly appears in the menus.

* source files match upstream:
   c43b3884e3d91e9a866be39bc6d3cc1452503cf5f89b5cac10065dd25b30525f  
   congruity-9.tar.bz2
* package meets naming and versioning guidelines.
* specfile is properly named, is cleanly written and uses macros consistently.
* summary is OK.
* description is OK.
* dist tag is present.
* build root is OK.
X license field matches the actual license.
* license is open source-compatible.
* license text included in package.
* latest version is being packaged.
* BuildRequires are proper (none).
* %clean is present.
* package builds in mock (rawhide, x86_64).
* package installs properly.
* rpmlint is silent.
* final provides and requires are sane:
   congruity = 9-1.fc10
  =
   /usr/bin/python
   libconcord-python >= 0.20
   wxPython

* %check is not present; no test suite.  I installed and ran the program, but I 
   have neither the necessary remote nor a programming file for it.
* owns the directories it creates.
* doesn't own any directories it shouldn't.
* no duplicates in %files.
* file permissions are appropriate.
* no scriptlets present.
* code, not content.
* documentation is small, so no -doc subpackage is necessary.
* %docs are not necessary for the proper functioning of the package.
X GUI program, but no desktop file is installed.

Thanks for the review.

1) I have tracked down all the icons, found which RPM they're in, and determined the RPM's license, list below. I assume that since the RPMs each list just a single license, I can take that as the correct license for each of the files in those RPMs, and not do further research to determine the image's original source?

2) Whilst congruity is a GUI application, the end-user is not intended to run it directly (doing so would simply cause congruity to display an error), so I'm not sure that a desktop file is appropriate. Instead, the user is intended to configure Firefox/... to automatically open certain downloaded files using congruity. These files are obtained by using the Logitech Harmony website, where certain actions (e.g. request firmware update, or request that remote be updated with the created configuration)  will trigger a download that congruity should handle.

-----

Note: I'm upstream for congruity, and will add this information to LICENSE.txt

icon-failed.png
  /usr/share/icons/gnome/32x32/emblems/emblem-important.png
  gnome-icon-theme-2.22.0-6.fc9.noarch
  GPL+

icon-complete.png
  /usr/share/icons/gnome/48x48/emblems/emblem-certified.png
  Resized and recolored
  gnome-icon-theme-2.22.0-6.fc9.noarch
  GPL+

icon-in-progress.png
  /usr/share/f-spot/icons/hicolor/32x32/emblems/emblem-event.png
  f-spot-0.4.3.1-1.fc9.i386
  GPL

icon-unstarted.png
  Part of congruity
  GPLv3+

-----

Let me know your answers to these points and I'll rev the spec. Thanks.

Sorry to bug you, but ping? Thanks!

I live in Houston.  I cannot speculate on when I will have the resources (like power or reasonable Internet access) to continue this review.

Sorry to hear that; hope everything's OK for you.

I'm unassigning the bug for now then, just in case somebody else wants to pick it up in the meantime.

it'd be nice for this review to get resurrected - I used congruity this weekend but had to download it manually.

it'd be good for congruity to ship a configuration file to make browsers use it for *.EZ* files automatically, but I'm not sure if that's possible, because they use generic MIME types:

[root@adam tmp]# file --mime-type *.EZ*
Connectivity-1.EZHex:       application/xml
Connectivity-2.EZHex:       application/xml
Connectivity-3.EZHex:       application/xml
Connectivity.EZHex:         application/xml
ConnectivitySimple-1.EZHex: application/xml
ConnectivitySimple-2.EZHex: application/xml
ConnectivitySimple-3.EZHex: application/xml
ConnectivitySimple.EZHex:   application/xml
LatestFirmware.EZUp:        text/html
Update-1.EZHex:             application/xml
Update-2.EZHex:             application/xml
Update.EZHex:               application/xml

and I'm not sure if you can package a file to tell browsers to open a certain file type with a certain program _by extension_ rather than MIME type (which is obviously what happens in Windows). I've asked caillon if he can help answer that question...

-- 
Fedora Bugzappers volunteer triage team
https://fedoraproject.org/wiki/BugZappers

Whee, I just learned how to do MIME types. :)

I'll submit a patch for libconcord which defines appropriate files to be of type application/x-libconcord , and an updated congruity package with a .desktop file that specifies it can handle this MIME type. I believe that would be correct.

-- 
Fedora Bugzappers volunteer triage team
https://fedoraproject.org/wiki/BugZappers

I've submitted the patch to libconcord:

https://bugzilla.redhat.com/show_bug.cgi?id=506536

once that gets applied (the maintainer's very quick to respond), I'll put a new congruity package in this bug, with the MIME type added.

-- 
Fedora Bugzappers volunteer triage team
https://fedoraproject.org/wiki/BugZappers

I sent a patch upstream to congruity to add a .desktop file to associate with the MIME type from the libconcord patch:

https://sourceforge.net/tracker/?func=detail&aid=2808133&group_id=231128&atid=1082218

here's an updated congruity build which bumps to 10, adds this patch, and fixes a couple other minor things.

http://adamwill.fedorapeople.org/congruity/congruity.spec
http://adamwill.fedorapeople.org/congruity/congruity-10-1.aw_fc12.src.rpm

could someone please complete this review?

-- 
Fedora Bugzappers volunteer triage team
https://fedoraproject.org/wiki/BugZappers

congruity-11 is released with the .desktop patch included.

Is there a mechanism to create/admin Fedora packages from Ubuntu? I converted all my client machines...

not reliably, the conventions and formats are too different. I'm happy to maintain congruity for Fedora if that would be OK with you, though, Stephen...

-- 
Fedora Bugzappers volunteer triage team
https://fedoraproject.org/wiki/BugZappers

You might want to grab your brown paper bag - you left the .desktop file out of the 11 release tarball :)

-- 
Fedora Bugzappers volunteer triage team
https://fedoraproject.org/wiki/BugZappers

Crap, I forgot my release script had an explicit file list in it. I'll fix that up tomorrow I hope.

Yes, I'm fine with you maintaining this in Fedora if you want; having it packaged would be great.

11 package build is up:

http://adamwill.fedorapeople.org/congruity/congruity.spec
http://adamwill.fedorapeople.org/congruity/congruity-11-1.aw_fc12.src.rpm

-- 
Fedora Bugzappers volunteer triage team
https://fedoraproject.org/wiki/BugZappers

You need to validate the desktop file.

https://fedoraproject.org/wiki/Packaging/Guidelines#Desktop_files

add this to the end of the %install  section and add BuildRequires:  desktop-file-utils

desktop-file-validate %{buildroot}/%{_datadir}/applications/%{name}.desktop

You need to validate the desktop file.

https://fedoraproject.org/wiki/Packaging/Guidelines#Desktop_files

add this to the end of the %install  section and add BuildRequires:  desktop-file-utils

desktop-file-validate %{buildroot}/%{_datadir}/applications/%{name}.desktop

oh, yeah, sorry, forgot about that. coming...

-- 
Fedora Bugzappers volunteer triage team
https://fedoraproject.org/wiki/BugZappers

here:

http://adamwill.fedorapeople.org/congruity/congruity.spec
http://adamwill.fedorapeople.org/congruity/congruity-11-2.aw_fc12.src.rpm


-- 
Fedora Bugzappers volunteer triage team
https://fedoraproject.org/wiki/BugZappers

* source files match upstream: OK.
b41601c3a13d889422b707f854b7a23b652a48b8
congruity-11.tar.bz2
* package meets naming and versioning guidelines. OK.
* specfile is properly named, is cleanly written and uses macros consistently. OK.
* summary is OK.
* description is OK.
* dist tag is present.
* build root is OK.
* license field matches the actual license. OK
* license is open source-compatible. OK.
* license text included in package. OK.
* latest version is being packaged. OK.
* BuildRequires are proper . OK.
* %clean is present. OK.
* Desktop file is properly installed with desktop-file-install. OK.
* package builds in Koji dist-f11 OK.
http://koji.fedoraproject.org/koji/taskinfo?taskID=1423291
* package installs and uninstalls properly. OK.
* rpmlint is silent. OK.
rpmlint -vi congruity-11-2.fc11.src.rpm
congruity.src: I: checking
1 packages and 0 specfiles checked; 0 errors, 0 warnings

rpmlint -vi congruity-11-2.fc11.noarch.rpm
congruity.noarch: I: checking
1 packages and 0 specfiles checked; 0 errors, 0 warnings.
* final provides and requires are sane:
congruity = 11-1.fc11

/bin/sh  
/bin/sh  
/usr/bin/env  
libconcord-python >= 0.20
rpmlib(CompressedFileNames) <= 3.0.4-1
rpmlib(FileDigests) <= 4.6.0-1
rpmlib(PayloadFilesHavePrefix) <= 4.0-1
wxPython >= 2.8

* owns the directories it creates. OK.
* doesn't own any directories it shouldn't. OK.
* no duplicates in %files. OK.
* %check is not present; no test suite.  I installed and ran the program, but I 
   have neither the necessary remote nor a programming file for it.
* code, not content.
* documentation is small, so no -doc subpackage is necessary.
* %docs are not necessary for the proper functioning of the package.
* file permissions are appropriate. OK 
* scriptlets present and correct. OK



Note:

The License includes this, I'm not sure if you need to add it the License tag

Licensing for remote.png:
CC-BY-SA-2.5
Downloaded from the URL below, resized, and made transparent:
http://commons.wikimedia.org/wiki/Image:Harmony670.jpg


Recommend:

You have set the file permissions, this isn't needed.

%defattr(0644,root,root,0755)
%doc Changelog COPYING LICENSE.txt README.txt
%attr(0755, root, root) %{_bindir}/*

should be

%defattr(-,root,root,-)
%doc Changelog COPYING LICENSE.txt README.txt
%{_bindir}/*

I have tested this

rpm -qav --list  congruity 
-rwxr-xr-x    1 root    root                    61266 Jun 18 20:33 /usr/bin/congruity
-rw-r--r--    1 root    root                      117 Jun 18 20:33 /usr/share/applications/congruity.desktop
drwxr-xr-x    2 root    root                        0 Jun 18 20:33 /usr/share/congruity
-rw-r--r--    1 root    root                     2192 Jun 18 20:33 /usr/share/congruity/icon-complete.png
-rw-r--r--    1 root    root                     1960 Jun 18 20:33 /usr/share/congruity/icon-failed.png
-rw-r--r--    1 root    root                     1956 Jun 18 20:33 /usr/share/congruity/icon-in-progress.png
-rw-r--r--    1 root    root                     1400 Jun 18 20:33 /usr/share/congruity/icon-unstarted.png
-rw-r--r--    1 root    root                   136801 Jun 18 20:33 /usr/share/congruity/remote.png
drwxr-xr-x    2 root    root                        0 Jun 18 20:33 /usr/share/doc/congruity-11
-rw-r--r--    1 root    root                    35147 Jun 18 04:54 /usr/share/doc/congruity-11/COPYING
-rw-r--r--    1 root    root                     5840 Jun 18 04:54 /usr/share/doc/congruity-11/Changelog
-rw-r--r--    1 root    root                     1281 Jun 18 04:54 /usr/share/doc/congruity-11/LICENSE.txt
-rw-r--r--    1 root    root                     5840 Jun 18 04:54 /usr/share/doc/congruity-11/README.txt
-rw-r--r--    1 root    root                      623 Jun 18 20:33 /usr/share/man/man1/congruity.1.gz




***Package Approved***

I have just noticed this in the build log.


+ unset DISPLAY
+ make all
Nothing to build, run 'make install' as root
+ exit 0


make all should be removed from the %build section




I believe this can be ignored

+ exit 0
/usr/lib/rpm/pythondeps.sh: line 8: python: command not found
/usr/lib/rpm/pythondeps.sh: line 8: python: command not found
Requires(interp): /bin/sh /bin/sh


as it doesn't seem to apply

http://osdir.com/ml/fedora-devel-list/2009-02/msg01589.html

yeah, it shouldn't be needed for this package, I don't think. I'll take out the make all.

-- 
Fedora Bugzappers volunteer triage team
https://fedoraproject.org/wiki/BugZappers

spec and srpm updated:

http://adamwill.fedorapeople.org/congruity/congruity.spec
http://adamwill.fedorapeople.org/congruity/congruity-11-4.aw_fc12.src.rpm

to address all above points.

-- 
Fedora Bugzappers volunteer triage team
https://fedoraproject.org/wiki/BugZappers

New Package CVS Request
=======================
Package Name: congruity
Short Description: Application to program Logitech Harmony universal remote controls 
Owners: adamwill
Branches: F-10 F-11
InitialCC: s-t-rhbugzilla


-- 
Fedora Bugzappers volunteer triage team
https://fedoraproject.org/wiki/BugZappers

congruity-12 released; should fix missing .desktop file.

spec and .src.rpm updated in the same directory, for anyone interested.

-- 
Fedora Bugzappers volunteer triage team
https://fedoraproject.org/wiki/BugZappers

"InitialCC" must be an existing Fedora account, not an email address, so I've left it blank.  If you have an account already, you can go to the pkgdb page for this package and add yourself.

Otherwise, CVS done.

congruity-12-1.fc11 has been submitted as an update for Fedora 11.
http://admin.fedoraproject.org/updates/congruity-12-1.fc11

congruity-12-1.fc11 has been pushed to the Fedora 11 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update congruity'.  You can provide feedback for this update here: http://admin.fedoraproject.org/updates/F11/FEDORA-2009-7877

congruity-12-1.fc10 has been pushed to the Fedora 10 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update congruity'.  You can provide feedback for this update here: http://admin.fedoraproject.org/updates/F10/FEDORA-2009-7918

congruity-12-1.fc10 has been pushed to the Fedora 10 stable repository.  If problems still persist, please make note of it in this bug report.

congruity-12-1.fc11 has been pushed to the Fedora 11 stable repository.  If problems still persist, please make note of it in this bug report.

Stephen, I just filed an upstream issue to change the .desktop file, since we changed the MIME types in libconcord to match upstream (Logitech):

https://sourceforge.net/tracker/?func=detail&aid=2832287&group_id=231128&atid=1082218

-- 
Fedora Bugzappers volunteer triage team
https://fedoraproject.org/wiki/BugZappers