Bug 213600

Not an official review since I am just a rookie.

- rpmlint gives one warning on the src: tinyca2 mixed-use-of-spaces-and-tabs. A
quick glance makes me think that the indentation used in the %description and
for the sed lines (in %setup) might be the culprit
- the buildroot line does not respect the preferred value for FE
(%{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)). Since this is
just a PREFERRED not a MUST, should not be a blocker
- MUST: package is named according to the guidelines
- MUST: spec file name matches the base package name
- MUST: license field matches actual license
- MUST: the program is licensed under GPL but upstream did not include the
actual text of the license is the source, just the reference to it. I guess you
should ping upstream to add the license to the provided tar files.
- MUST: spec file is in American English
- MUST: spec file is legible 
- MUST: source matches upstream, md5sum being a7f63806dbdc38a34ed58e42e79f4822
for both
- MUST: builds fine in mock/i386. Since the content is actually just a perl
script + some message (.po) files which are formatted during the build phase, I
assume it would succesfully build on any platform; created rpm is noarch
- MUST: %find_lang macro is correctly used to pick locales
- MUST: no libraries are installed, so there is no need for calling ldconfig in
%post/%postun
- MUST: package is not relocatable
- MUST: owns all directories (and files) that it creates
- MUST: no duplicate files in the %files listing
- MUST: %clean is correct
- MUST: makes consistent use of macros
- MUST: no forbidden code/content included
- MUST: large documentation does not exist, so no need for a separate -doc
- MUST: the content of %doc is a small CHANGES file, so runtime functionality is
not affected
- MUST: no header or static files, no pkgconfig(.pc), no library files with a
suffix, no ibtool archives, so no need for -devel
- MUST: IS a GUI application; correctly includes %{name}.desktop (provided by
upstream) and properly installs it with desktop-file-install; someone more
experienced please comment if the "--add-category=X-Fedora" is still required
(according to yesterday's FESCO:
  === Packaging Committee Report ===
 * Voting to stop using the X-Fedora category in the desktop file is
currently underway via email.)
- MUST: does not take ownership of foreign files/directories
- SHOULD: includes available translations
- SHOULD: as specified above, builds fine in mock
- SHOULD: on a RHEL4 system the rpm installed fine but the program did not run,
failing with:
error: Failed dependencies:
        perl(Gtk2) is needed by tinyca2-0.7.5-2.noarch
        perl(Gtk2::SimpleMenu) is needed by tinyca2-0.7.5-2.noarch
        perl(Locale::gettext) is needed by tinyca2-0.7.5-2.noarch
On FC6 it detected the missing Requires, but failed to get installed even after
installing perl-Gtk2 and gettext:
error: Failed dependencies:
        perl(Locale::gettext) is needed by tinyca2-0.7.5-2.noarch
It seems that the correct Requires should be perl-gettext rather then gettext.
The program runs successfully after installing perl-Gtk2 and perl-gettext.
- SHOULD: no scriplets at all, so neither unsane scriptlets


Bottom line
- cosmetic fixes: make rpmlint happy by replacing multiple spaces with tab (non
blocker)
- make reviewers happy by using the recommended build root line (non blocker)
- use a correct Requires line (perl-gettext instead of gettext) (BLOCKER)

Sorry, I have mixed BuildRequires with Requires in the last part of the review.
I intented to say that you must add perl-gettext to the Requires line, which
should become
Requires: openssl tar zip perl-gettext


... unless I have made an evaluation error here and yum automatically picks and
solves the dependencies for the needed perl modules. I for one I had to use yum
search in order to find out which package provides perl-Locale-gettext.

mmh... I don't understand

> - SHOULD: on a RHEL4 system the rpm installed fine but the program
>   did not run, failing with:
> error: Failed dependencies:
>         perl(Gtk2) is needed by tinyca2-0.7.5-2.noarch

This seems to be an rpm message but not an error given out by the
program.


> On FC6 it detected the missing Requires, but failed to get installed
> even after installing perl-Gtk2 and gettext:

Who is "it"?


> error: Failed dependencies:
>         perl(Locale::gettext) is needed by tinyca2-0.7.5-2.noarch

mmh... this seems to be a bug in FC6 yum then. 'perl(Locale::gettext)'
is provided by the perl-gettext packages both from FC5 and FC6.

Within FC5, package and the perl-gettext dependency installed fine:

| # http_proxy= yum localinstall tinyca2-0.7.5-2.noarch.rpm
| Installing:
|  tinyca2                 noarch     0.7.5-2          tinyca2-0.7.5-2.noarch.rpm  546 k
| Installing for dependencies:
|  perl-gettext            i386       1.05-8.fc5       extras             20 k
| ...
| Installed: tinyca2.noarch 0:0.7.5-2
| Dependency Installed: perl-gettext.i386 0:1.05-8.fc5
| Complete!
| #

OK,. let me rephrase:
Everything is fine, my test failed because I was using only rpm rather then yum.:)

The only problems are the two cosmetic ones (mixed spaces with tabs and the
build root).

sorry; I won't change these two issues.

rpmlint is wrong about the whitespace-tab mixes

and the buildroot with %(id...) is just stupid and not required

Use :set list in vi and you will notice that rpmlint is not wrong about the mixes :)

The buildroot is just a recommended value, so should not be a blocker. But it's
up to a sponsor to decide. I am just a newcomer.

I do not see mixed tab-spaces; TABs are used consistently. But I do not replace
every '  +' with a TAB just because it ends at a tab-position.

I do not need a sponsor; just a reviewer.

I will review this later.

NOTE:
* would you consider using braces {} for using macro? I think
  using braces should make spec file easier to read......

Well,
1. From http://fedoraproject.org/wiki/Packaging/Guidelines :
* License
  - (NOT A BLOCKER) original tarball does not include the copy of
  GPL license. You should ask upstream to include GPL copy.

* Desktop files
  - For desktop-file-install, the line
    "--add-category X-Fedora" should be removed as
    * this is of no use any longer.
    * original desktop-file-utils 0.11 refuses this (although Fedora
      patched against this to only warn against this).
    See:
    http://fedoraproject.org/wiki/PackagingDrafts/DesktopFiles

2. From http://fedoraproject.org/wiki/Packaging/ReviewGuidelines
   = Nothing

3. Other things I have noticed :
* desktop file
  - (NOT A BROCKER) I strongly recommend that you install a 
    icon which can be used for desktop entry and write to 
    the spec file where (or how) we can get the icon. However,
    for a quick look I cannot find a proper icon from
    http://tinyca.sm-zone.net/ ....

    Would you ask upstream to provide a icon?

Consider the comments above. However I can approve this
package now.

----------------------------------------------------------
  This package (tinyca2) is APPROVED by me.

Don't forget to close this bug when rebuilds are done successfully.

Well, what is the current state of this package?

sorry; password manager does not work for wiki and I forget everytime the
password  and was not able to file a branch request

(Well, for libextractor, I will check it later, althogh it may be
 tomorrow. By the way, it seems that tinyca2 has been already imported
 into FE-devel/6/5, so please close this bug when it is possible)

ok; closed...