Bug 210189 - (granule) Review Request: granule - Gtk+-based flashcards program
Review Request: granule - Gtk+-based flashcards program
Status: CLOSED RAWHIDE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Michael Schwendt
Fedora Package Reviews List
:
Depends On: libassa
Blocks: FE-ACCEPT
  Show dependency treegraph
 
Reported: 2006-10-10 13:54 EDT by Vladislav Grinchenko
Modified: 2007-11-30 17:11 EST (History)
0 users

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2006-11-19 06:41:25 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:


Attachments (Terms of Use)
build failure log (7.94 KB, application/x-gzip)
2006-10-16 19:48 EDT, Michael Schwendt
no flags Details
working granule.spec (2.24 KB, text/plain)
2006-10-18 09:41 EDT, Michael Schwendt
no flags Details

  None (edit)
Comment 1 Vladislav Grinchenko 2006-10-10 13:57:23 EDT
This is my first package. The program's home page is
http://granule.sourceforge.net/.

I need a sponsor to include this package into Fedora Core extras.
Comment 2 Gianluca Sforna 2006-10-10 17:55:47 EDT
I had a quick look at the spec and I regret to say there are a lot of issues
with it.

In order to speed up this thing, I'll give you some suggestions:

start building a spec from the template you get from the command
"rpmdev-newspec" (in package rpmdevtools): this will save you from a lot of problems

always run rpmlint on the resulting src.rpm: this gives you more hints on things
to fix before submitting again.

Always prefer standard macros instead of redefining them:
http://fedoraproject.org/wiki/Extras/RPMMacros

Now a quick list of thing to check:
* Do not use tags like Packager and Prefix
* Buildroot is not the recommended one
* Do not use both spaces and tabs for indentation
* Summary is a little too concise...
* a GUI app with no BuildRequire tags look suspicious :)
* the commands you use in %build probably calls for more BRs (hint: mock)
* no need to test for SMP in the build phase: just use "make %{?_smp_mflags}"
* %find_lang do not seems to be used in the proper way
* always $RPM_BUILD_ROOT in %install
* usual defattr on files is %defattr(-,root,root,-)
* you do not seems to own the directory %{datadir}/granule
* the .destkop fil is not installed in the proper way

All of this is covered the the packaging guidelines:
http://fedoraproject.org/wiki/Packaging/Guidelines



Comment 3 Vladislav Grinchenko 2006-10-13 22:40:28 EDT
I have made all necessary changes, verified the build, and uploaded new versions
of both .spec and .src.rpm files (as of Fri Oct 13 22:26 EDT 2006).

This should be sufficient to get the acceptance process going.

Can someone sponsor this project?

thanks,
-Vlad
Comment 4 Michael Schwendt 2006-10-16 19:45:51 EDT
$ rpmlint granule-1.2.3-1.src.rpm 
W: granule summary-ended-with-dot A flashcards program based on Leitner methodology.
W: granule non-standard-group Applications
W: granule setup-not-quiet
E: granule no-cleaning-of-buildroot %install
E: granule no-cleaning-of-buildroot %clean


* If you use %find_lang (which is a MUST), you need to include its
results in the %files section like this:

%files -f %{name}.lang

The manually added %{_datadir}/locale/*/*/* entry must be removed.


* The package does not include several directories:

%{_datadir}/granule/
%{_datadir}/granule/xml/

Instead of writing

  %{_datadir}/granule/xml/*

which does not include the "xml" directory itself,
you could simply write

  %{_datadir}/granule/xml/

which includes the "xml" directory and its contents _recursively_.


* Don't use the %makeinstall macro when "make DESTDIR=%{buildroot} install"
works fine (as is the case with most GNU autotools based tarballs).


* At the beginning of the %install section, "rm -rf %{buildroot}" is
missing.


* The %clean section is missing.


* These two

> glib-gettextize --copy --force
> intltoolize --automake --copy --force

ought to be applied in the tarball already.


* In %setup, using "%setup -q" gives more quiet output and doesn't
list the tarball files when extracting it. Makes build logs much more
readable and short and to the point.


* Where you run "install ...", prefer "install -p ..." to preserve
time-stamps of copied files. Same applies to "cp ..." => "cp -p ...".
End-users appreciate this.


* Build fails due to missing build requirements:

checking for XMLCPP... configure: error: Package requirements (libxml-2.0 >=
2.6.11) were not met:

No package 'libxml-2.0' found

(!) It's missing "BuildRequires: libxml2-devel"!


checking for ASSA... configure: error: Package requirements (assa-3.4 >= 3.4.2)
were not met:

No package 'assa-3.4' found

(!) It's missing "BuildRequires: libassa-devel"!


* Build fails here. Attaching log.
Comment 5 Michael Schwendt 2006-10-16 19:48:11 EDT
Created attachment 138641 [details]
build failure log
Comment 6 Michael Schwendt 2006-10-16 19:56:49 EDT
* Reason is it's missing "BuildRequires: gettext".

* Next build failure (can be worked around with %makeinstall, but not
recommended) and ought to be fixed in the tarball:

Making install in pixmaps
make[1]: Entering directory `/home/qa/tmp/rpm/BUILD/granule-1.2.3/pixmaps'
make[2]: Entering directory `/home/qa/tmp/rpm/BUILD/granule-1.2.3/pixmaps'
make[2]: Nothing to be done for `install-exec-am'.
test -z "/usr/share/granule//pixmaps" || mkdir -p -- "/usr/share/granule//pixmaps"
mkdir: cannot create directory `/usr/share/granule': Permission denied
make[2]: *** [install-pixmapDATA] Error 1
Comment 7 Michael Schwendt 2006-10-17 08:44:56 EDT
Bottom part of previous comment is wrong. It breaks because currently
the spec uses %makeinstall instead of "make DESTDIR=%{buildroot} install".
The %makeinstall macro breaks due to the way $(pkg_datadir) and
$(pkg_xmldir) are defined -- they are not derived from $(datadir)

When not using %makeinstall, the software installs, and the package buids.
I will continue there.
Comment 8 Michael Schwendt 2006-10-18 09:41:28 EDT
Created attachment 138781 [details]
working granule.spec
Comment 9 Vladislav Grinchenko 2006-10-18 17:42:31 EDT
Michael,

thanks for all the heavy lifting. I updated the spec file and SRPM package and
checked SRPM with `rpmlint'. The changes to the spec have been committed to
granule's CVS.

thanks again,
-Vlad
Comment 10 Michael Schwendt 2006-10-18 18:51:44 EDT
Okay, please proceed to the account creation, step 10 here,
http://fedoraproject.org/wiki/Extras/Contributors
and I'll sponsor your Fedora Extras membership.

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