Bug 203249 - Review Request: kpolynome
Review Request: kpolynome
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Mamoru TASAKA
Fedora Package Reviews List
:
Depends On:
Blocks: FE-ACCEPT
  Show dependency treegraph
 
Reported: 2006-08-19 15:32 EDT by Chitlesh GOORAH
Modified: 2007-11-30 17:11 EST (History)
1 user (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2006-08-26 13:33:38 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:


Attachments (Terms of Use)

  None (edit)
Description Chitlesh GOORAH 2006-08-19 15:32:09 EDT
Spec URL: http://chitlesh.funpic.de/rpm/kpolynome.spec
SRPM URL: http://chitlesh.funpic.de/rpm/kpolynome-0.1.2-1.src.rpm
Description: KPolynome is a program to calculate mathematical polynomes based on given data coordinates.
Comment 1 Michał Bentkowski 2006-08-19 15:58:36 EDT
Quick, not-official review:

 * mock build fails. You have to add autoconf and automake BRs.
 * rpmlint gives a lot of output:
E: kpolynome script-without-shellbang /usr/share/doc/kpolynome-0.1.2/AUTHORS
W: kpolynome dangling-symlink /usr/share/doc/HTML/en/kpolynome/common /usr/
share/doc/HTML/en/common
W: kpolynome symlink-should-be-relative /usr/share/doc/HTML/en/kpolynome/common 
/usr/share/doc/HTML/en/common
E: kpolynome zero-length /usr/share/doc/kpolynome-0.1.2/README
E: kpolynome script-without-shellbang /usr/share/doc/kpolynome-0.1.2/NEWS
W: kpolynome dangling-symlink /usr/share/doc/HTML/hu/kpolynome/common /usr/
share/doc/HTML/hu/common
W: kpolynome symlink-should-be-relative /usr/share/doc/HTML/hu/kpolynome/common 
/usr/share/doc/HTML/hu/common
E: kpolynome script-without-shellbang /usr/share/doc/kpolynome-0.1.2/COPYING
E: kpolynome script-without-shellbang /usr/share/doc/kpolynome-0.1.2/ChangeLog

You should change doc files permissions in %%prep section and probably remove
README file.
Comment 2 Michał Bentkowski 2006-08-19 16:13:31 EDT
I forgot to look into %%files section in your spec file. There is a few things
you have to correct. Your package doesn't own directories that it creates.
For example, RPM creates %{_datadir}/apps/%{name} directory, but during
uninstall, it won't be deleted. Why? Because it is refering to file 
(%{name}ui.rc), not directory. Idem with HTML-doc files. You have to refer
to whole directory, not to particular files. So, your %%files section should
look like:
%{_datadir}/apps/%{name}
%{_docdir}/HTML/en/%{name}
etc.
Comment 3 Chitlesh GOORAH 2006-08-23 13:22:49 EDT
Updated:
Spec URL: http://chitlesh.funpic.de/rpm/kpolynome.spec
SRPM URL: http://chitlesh.funpic.de/rpm/kpolynome-0.1.2-2.src.rpm
Comment 4 Mamoru TASAKA 2006-08-23 14:37:57 EDT
Well, only for packaging issue.
mock build is okay.

* %post and %postun calls %{_bindir}/gtk-update-icon-cache.
  Require it as Requires(post) and Requires(postun).

* %patch0 -p0 -b .
  Very cosmetic, however, adding some suffix is perhaps
  preferable.

* chmod 644 %{_builddir}/%{name}-0.1/AUTHORS
  Why the explicit directory %{_builddir}/%{name}-0.1 necessary?
  Usually, in %build or %install stage, the working directory is
  what is written in %setup stage.

* Any reasons that document HTML files should be in 
  %{_docdir}/HTML/??/%{name} ? On my system, the only rpm which
  uses %{_docdir}/HTML/ is fedora-release-notes.

  Documents in other rpms should be in %{_docdir}/%{name}-%{version}.
  I suggest moving HTML document files.

* rpmlint is not silent.

E: kpolynome-debuginfo script-without-shellbang
/usr/src/debug/kpolynome-0.1/src/drawwidget.h
E: kpolynome-debuginfo script-without-shellbang
/usr/src/debug/kpolynome-0.1/src/maindialog.h
E: kpolynome-debuginfo script-without-shellbang
/usr/src/debug/kpolynome-0.1/src/drawwidget.cpp
E: kpolynome-debuginfo script-without-shellbang
/usr/src/debug/kpolynome-0.1/src/maindlg.ui.h
E: kpolynome-debuginfo script-without-shellbang
/usr/src/debug/kpolynome-0.1/src/matdata.cpp
E: kpolynome-debuginfo script-without-shellbang
/usr/src/debug/kpolynome-0.1/src/curvedialog.h
E: kpolynome-debuginfo script-without-shellbang
/usr/src/debug/kpolynome-0.1/src/main.cpp
E: kpolynome-debuginfo script-without-shellbang
/usr/src/debug/kpolynome-0.1/src/curvedialog.cpp
E: kpolynome-debuginfo script-without-shellbang
/usr/src/debug/kpolynome-0.1/src/matdata.h
E: kpolynome-debuginfo script-without-shellbang
/usr/src/debug/kpolynome-0.1/src/maindialog.cpp
E: kpolynome-debuginfo script-without-shellbang
/usr/src/debug/kpolynome-0.1/src/curvedlg.ui.h

  --- permission issue. Fix this by changing the permissions of these files to 644.
Comment 5 Chitlesh GOORAH 2006-08-25 06:24:26 EDT
Updated:
Spec URL: http://chitlesh.funpic.de/rpm/kpolynome.spec
SRPM URL: http://chitlesh.funpic.de/rpm/kpolynome-0.1.2-3.src.rpm
Comment 6 Mamoru TASAKA 2006-08-25 08:39:15 EDT
rpmlint became clean. I think that this package is almost
okay.

Assigning to me.

Well:
* HTML documents
[tasaka1@localhost SOURCES]$ rpm -ql kpolynome
/usr/bin/kpolynome
/usr/share/applications/fedora-kpolynome.desktop
/usr/share/apps/kpolynome
/usr/share/apps/kpolynome/kpolynomeui.rc
/usr/share/doc/kpolynome-0.1.2
/usr/share/doc/kpolynome-0.1.2/AUTHORS
/usr/share/doc/kpolynome-0.1.2/COPYING
/usr/share/doc/kpolynome-0.1.2/ChangeLog
/usr/share/doc/kpolynome-0.1.2/NEWS
/usr/share/doc/kpolynome-0.1.2/TODO
/usr/share/icons/hicolor/16x16/apps/kpolynome.png
/usr/share/icons/hicolor/32x32/apps/kpolynome.png

...... All HTML docs disappeared. I think you forgot to
add some entry to %doc.

* Changelog
%changelog
* Fri Aug 25 2006 Chitlesh Goorah 
- Fixed xcircuit.desktop

I think this is only a typo.

* chmod -x src/{drawwidget.*,maindi .....
Umm, well, it is okay, however,

$ find . -name \*.cpp -o -name \*.h | xargs chmod 0644
(pwd is $builddir/kpolynome-0.1.2)

in %prep stage is perhaps simpler.

* Requires:  kdelibs
is necessary? This package requires libkdecore.so.4 and this
correctly requires kdelibs.
Comment 7 Chitlesh GOORAH 2006-08-25 16:47:08 EDT
Updated:
Spec URL: http://chitlesh.funpic.de/rpm/kpolynome.spec
SRPM URL: http://chitlesh.funpic.de/rpm/kpolynome-0.1.2-4.src.rpm
Comment 8 Mamoru TASAKA 2006-08-25 22:37:26 EDT
Moving to final check.
Comment 9 Mamoru TASAKA 2006-08-26 03:10:25 EDT
A question:

Would you check the files in /usr/share/doc/kpolynome-0.1.2/{en,hu}/ ,
especially Makefile* in that directory necessary?
Comment 10 Chitlesh GOORAH 2006-08-26 03:52:12 EDT
Updated:
Spec URL: http://chitlesh.funpic.de/rpm/kpolynome.spec
SRPM URL: http://chitlesh.funpic.de/rpm/kpolynome-0.1.2-5.src.rpm
Comment 11 Mamoru TASAKA 2006-08-26 10:03:52 EDT
Fully checked. The remained things are:

From http://fedoraproject.org/wiki/Packaging/Guidelines :

* Requires

  Requires:         kdelibs

  Again, why is this exclicit requirement needed?
  /usr/bin/kpolynome requires libkdecore.so.4 and this
  is included in kdelibs. So this package automatically
  requires kdelibs even without the explicit statement
  above is not written.

  I suspect that the sentence can be removed.

* Using %{buildroot} and %{optflags} vs 
  $RPM_BUILD_ROOT and $RPM_OPT_FLAGS
  You use both %{_builddir} and $RPM_BUILD_ROOT . Choose one
  (although this is very cosmetic).

Other thing are all okay.
Comment 12 Chitlesh GOORAH 2006-08-26 10:26:17 EDT
Updated:
Spec URL: http://chitlesh.funpic.de/rpm/kpolynome.spec
SRPM URL: http://chitlesh.funpic.de/rpm/kpolynome-0.1.2-6.src.rpm

%Changelog - 0.1.2-6
- Replaced $$RPM_BUILD_ROOT by %%{buildroot}
- Removed requires: kdelibs
Comment 13 Mamoru TASAKA 2006-08-26 11:39:58 EDT
Okay.

Now I am pleased to say that this package (kpolynome) is
APPROVED.

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