Bug 229154

(In reply to comment #0)
> Spec URL: http://users.jyu.fi/~divanov/devel/
> SRPM URL: http://users.jyu.fi/~divanov/devel/konwert-1.8-7.src.rpm
> Description: Konwert is a charset converter similar to iconv. Available features
> include one-to-many conversions, context-dependent conversions,
> approximations of some unavailable characters. 'filterm' applies
> filter conversion to a terminal's I/O, to get on-the-fly charset
> conversion, and customized input methods.
> 
> rpmlint -i -v konwert-1.8-7.src.rpm 
> I: konwert checking
> 
> This is my first package, and I am seeking a sponsor.

Sorry,
Spec URL: http://users.jyu.fi/~divanov/devel/konwert.spec

i'll make a full review within a day in waitting you find out a sponsor.

Xavier, please review this request if you like (and
mark this as FE-REVIEW)

As this request is sponsor needed issue and you are not
a sponsor, a sponsor must review this finally. If the time
comes, I will do a final check and do some needed procedure
for sponsorship issue as one of the sponsors.

Hi mamoru,

i'll within an hours (mock's working for)

Hi mamoru,

i'll within an hours (mock's working for)

Created attachment 148349 [details]
mock build log of konwert-1.8-7.fc7

Xavier, if you want to check mock build log of me,
please check the attached (on FC-devel i386)

Well, this package seems to have some big and minor
issues, however I leave this to you for now.

Indeed,

There's some things which should be fix.
-------------------------------
spec file:
-------------------------------
 ** BuildRequires

 - BR perl is useless as it requires by default installed package (such as 
xorg-x11-server-Xorg)

 ** %package Devel

 - Requires: konwert SHOULD be : requires:  %{name} = %{version}-%{release}
 - Group should be Development/Tools

 ** %description devel

 -  SHOULD only describe contains about -devel package, not   
  main package.

 ** %build

 - OPTFLAGS="$RPM_OPT_FLAGS %{!?debug:-fomit-frame-pointer}" can be remove.
   Build and work well without this.
 - in this case, use OPTFLAGS="$RPM_OPT_FLAGS -fno-rtti -fno-exceptions \   
   -fno-implicit-templates" instead of
 - use make instead %{_make}.

 ** %install 

 - use make install instead of %{_make} install.
 - The use of : prefix=$RPM_BUILD_ROOT%{_prefix} \
	        mandir=$RPM_BUILD_ROOT%{_mandir} \
	        mydocdir=$RPM_BUILD_ROOT%{_docdir}/konwert-%{version} \
	        perl=%{_bindir}/perl \
	        libdir=$RPM_BUILD_ROOT%{_libdir} \
   sound good and work except mydocdir= (see below for more explaination).
   after have a look on build.log (and Makefile), i can see the use of "sed"
   to change default location (which match with fedora install location) 
   such as /usr/share/ by /usr/local/share).
   Can be fix by patching Makefile but require more working time.
 - the use of "dontfixmanconfig=1", check if it's necessary and comment why.
 - keep timestamps on make install by addind "INSTALL=install -p"

** %post

 - Doesn't look good.

** %files and %files devel

 - doesn't sound good at all.
 - The use of "%{_docdir}/konwert-%{version}/en/*" is deprecated and must be drop.
   All doc MUST be set in %doc just after %defattr macros.
 - the use of %attr(755,root,root):
   Personnaly, I prefere use chmod command in %install section and comment the
reason of.
 - You package doesn't own the %{_datadir}/%{name}.
 - The use of %lang(p1) is useless
 - some files mentioned twice.
 - %{_mandir}/man1 instead of %{_mandir}/man*/*
 - You don't need to mentione explicitly all files, just the use of main
location is enough.
   such as %{_datadir}/%{name} (and own in the same time)

Spec file doesn't meet the packeging Guidlines, see
http://fedoraproject.org/wiki/Packaging/Guidelines.

--------------------------
 Rpmlint output
--------------------------

 ** From srpm file: clean.

 ** From main rpm package:

   W: konwert file-not-utf8 /usr/share/man/man1/filterm.1.gz
   W: konwert file-not-utf8 /usr/share/man/pl/man1/trs.1.gz
   W: konwert file-not-utf8 /usr/share/man/pl/man1/filterm.1.gz
   W: konwert file-not-utf8 /usr/share/man/pl/man1/konwert.1.gz
   W: konwert file-not-utf8 /usr/share/man/man1/konwert.1.gz

 - Can be fix by the use of iconv command in %prep or %install.

   W: konwert one-line-command-in-%post /usr/share/konwert/aux/fixmanconfig
 
 - As i mentioned above, this is not quite good.


** From -devel rpm package: clean.

** From -debuginfos rpm package:

  E: konwert-debuginfo empty-debuginfo-package

 - this package contains no files.
   rpmbuild not being able to strip the binaries.


If i forgot to mentione something, i will post

FE-REVIEW

Created attachment 148464 [details]
mock build log of konwert-1.8-7.fc7

Hi,

 attempt #2
Spec URL: http://users.jyu.fi/~divanov/devel/konwert.spec
SRPM URL: http://users.jyu.fi/~divanov/devel/konwert-1.8-7.fc7.src.rpm

rpmlint -iv konwert-1.8-7.fc7.x86_64.rpm 
I: konwert checking

rpmlint -i konwert-1.8-7.fc7.src.rpm 
I: konwert checking

rpmlint -i konwert-debuginfo-1.8-7.fc7.x86_64.rpm 
I: konwert-debuginfo checking
E: konwert-debuginfo empty-debuginfo-package

rpmlint -i konwert-devel-1.8-7.fc7.x86_64.rpm 
I: konwert-devel checking

Please increment release number every time you change/fix
your spec/srpm. Changing spec/srpm without changing release
number causes confusion on the people who are watching your
spec/srpm.

Sorry, I increased the relesed number

Spec URL: http://users.jyu.fi/~divanov/devel/konwert.spec
SRPM URL: http://users.jyu.fi/~divanov/devel/konwert-1.8-8.fc7.src.rpm

> %exclude %{_datadir}/konwert/devel
> %exclude %{_libdir}/konwert/devel

Should be put in "%files devel" without "%exclude" macros.

rpmlint output:

form srpm:

W: konwert mixed-use-of-spaces-and-tabs (spaces: line 68, tab: line 1)

easy fix ;-)

Hi,

 attempt #3
 http://users.jyu.fi/~divanov/devel/konwert.spec
 http://users.jyu.fi/~divanov/devel/konwert-1.8-9.fc7.src.rpm

 Xavier, how did you get warning about the spaces/tabs?
 rpmlint didn't give it for me. should I use some policy?

Thanks, Daniil.

you should use only tabs in your spec file.
Move your forward-key just after your "sharedir=$RPM_BUILD_ROOT%{_datadir} \"
you will see.
Vim is your friend... ;-)

Spec file and srpm don't look to be fix for last comment #12.

ping?

Can't reproduce the problem on my machine.

~Daniil.

Xavier, would you review this review again?

Note: I cannot still approve this request.

Ok, i will

OK - Mock Build on FC-6 FC-Devel (i386,x86_64)
OK - Package meets naming and packaging guidelines
OK - Spec file matches base package name.
OK - Spec has consistant macro usage.
OK - Meets Packaging Guidelines.
OK - License is GPL
OK - License field in spec matches
OK - License file included in package
OK - Spec in American English
OK - Spec is legible.
OK - Sources match upstream md5sum for both:
0a1dcb0fa7a1990980aba8ab9a4c3184  konwert-1.8.tar.gz
962c3d339b99656bb37a25c39d7d3dca  konwert-forbids_data_member.patch
OK - Package has correct buildroot.
OK - BuildRequires isn't required.
OK - Subpackage -devel is present.
OK - %prep stage is correct and work.
OK - The use of iconv is proper.
OK - %build and %install stages is correct and work.
OK - No *.la files is present in -devel subpackage.
OK - Package has %defattr and permissions on files is good.
OK - Package has a correct %clean section.
OK - Package is code or permissible content.
OK - Packages %doc files don't affect runtime.
OK - Package has no duplicate files in %files.
OK - Package doesn't own any directories other packages own.
OK - rpmlint output are clean.

OK - Should function as described.
OK - Should package latest version
OK - Should have %{?dist} tag but, should be remove to avoid trouble in CVS
import/Update/Build procedure.

Mamoru: ping?

Can you approve this request now ?

(In reply to comment #20)
> Can you approve this request now ?

Well, are you referring to 1.8-9?
If so, take my comment 17 as "there is a issue to be fixed
on this package before I can approve".

I have not checked this package precisely, however at least
------------------------------------------------
E: konwert-debuginfo empty-debuginfo-package
------------------------------------------------
cannot be ignored. Mock build log shows:
------------------------------------------------
install -p -m755 -s install/bin/trs    
/var/tmp/konwert-1.8-9.fc7-root-mockbuild/usr/bin
install -p -m755    install/bin/konwert
/var/tmp/konwert-1.8-9.fc7-root-mockbuild/usr/bin
install -p -m755 -s install/bin/filterm
/var/tmp/konwert-1.8-9.fc7-root-mockbuild/usr/bin
------------------------------------------------
and this should be fixed (at least).

(In reply to comment #21)
> (In reply to comment #20)
> > Can you approve this request now ?
> 
> Well, are you referring to 1.8-9?
> If so, take my comment 17 as "there is a issue to be fixed
> on this package before I can approve".
> 
> I have not checked this package precisely, however at least
> ------------------------------------------------
> E: konwert-debuginfo empty-debuginfo-package
> ------------------------------------------------

 Can be fix by disabling the debuginfo

> cannot be ignored. Mock build log shows:
> ------------------------------------------------
> install -p -m755 -s install/bin/trs    
> /var/tmp/konwert-1.8-9.fc7-root-mockbuild/usr/bin
> install -p -m755    install/bin/konwert
> /var/tmp/konwert-1.8-9.fc7-root-mockbuild/usr/bin
> install -p -m755 -s install/bin/filterm
> /var/tmp/konwert-1.8-9.fc7-root-mockbuild/usr/bin
> ------------------------------------------------
> and this should be fixed (at least).

You right.
These files should be installed in 0644 mode.

Well, no..

These are executable binaries or scripts, so the permission
should be 0755.
What is wrong here is that 
--------------------------------------------
install -p -m755 "-s"
--------------------------------------------
means that this binary is stripped on this procedure, which
make it impossible to create debuginfo rpm.

Daniil: ping ?

Again, ping, Daniil?

Hi,

something like this?
http://users.jyu.fi/~divanov/devel/konwert-1.8-10.src.rpm
http://users.jyu.fi/~divanov/devel/konwert.spec

Thanks, Daniil.

Hi,

http://users.jyu.fi/~divanov/devel/konwert-1.8-10.fc7.src.rpm
http://users.jyu.fi/~divanov/devel/build.log

Thanks, Daniil.

Created attachment 151926 [details]
-11 spec file (some fixes added)

Well, as this is a NEEDSPONSOR ticket, I do a full review
for this package.

For -10:
* Timestamps
  - Well, actually 'INSTALL="install -p"' in %install stage does nothing, as
    all files installed in "INSTALL" macro are modified beforhand
    during the build stage.

    However, there are many files under %{_datadir}/%{name},
    of which the timestamps should
    be kept as they are not modified during build stage. This should
    be done by modifying Makefile directly.

* Lang file
  - All _documentation_ other than in English should be marked as
    %%lang(...).


The spec file attached should fix all the issues I mentioned above.
Please this the attached. If you agree with my spec file, I can
say okay for this package.

Then:
-------------------------------------------------------------
NOTE: Before being sponsored:

This package will be accepted with another few work. 
But before I accept this package, someone (I am a candidate) 
must sponsor you.

Once you are sponsored, you have the right to review other 
submitters' review requests and approve the packages formally. 
For this reason, the person who want to be sponsored (like you) 
are required to "show that you have an understanding 
of the process and of the packaging guidelines" as is described
on :
http://fedoraproject.org/wiki/PackageMaintainers/HowToGetSponsored

Usually there are two ways to show this.
A. submit other review requests with enough quality.
B. Do a "pre-review" of other person's review request
   (at the time you are not sponsored, you cannot do
   a formal review)

When you have submitted a new review request or have pre-reviewed other 
person's review request, please write the bug number on this bug report 
so that I can check your comments or review request.

Fedora Extras package review requests which are waiting for someone to
review can be checked on:
https://bugzilla.redhat.com/bugzilla/buglist.cgi?cmdtype=runnamed&namedcmd=mtasaka-review-noone

NOTE: FE-NEW blockers are now not complete.

Review guidelines are described mainly on:
http://fedoraproject.org/wiki/Packaging/ReviewGuidelines
http://fedoraproject.org/wiki/Packaging/Guidelines
http://fedoraproject.org/wiki/Packaging/ScriptletSnippets
------------------------------------------------------------

this bug should be assigned and approuved by a sponsor, removed myself from
assignee block.

Daniil, do you have a updated srpm fix from Mamoru review ?

(In reply to comment #28)
I agree with your changes. Also I agree with you that I don't have an
understanding of the process and of the packaging guidelines.

http://users.jyu.fi/~divanov/devel/konwert-1.8-11.fc7.src.rpm
http://users.jyu.fi/~divanov/devel/build.log

Well, spec/srpm is okay (actually it is what I uploaded).
Then:
* would you do a "pre-review" of a review request you choose
  from the following?
https://bugzilla.redhat.com/bugzilla/buglist.cgi?cmdtype=runnamed&namedcmd=mtasaka-review-noone
* Or do you have a plan to submit another review request
  or do you already have another review request?

ping?

Again ping?

I will close this bug as NOTABUG if no response is
received on this bug within ONE WEEK

Again? I will close this bug tomorrow.

I'm really short on time on working days and the next weekend I again must
travel, so I won't have access to pc/net. I have nothing to do with that.

We can wait for the weekend after next weekend.

Again ping?

Again ping?

I will close this bug if no response from the reporter is
received within one week,

CLOSING.

If someone wants to import this into Fedora, please file
a new bug report, thank you!