Bug 226200 - Merge Review: nkf
Merge Review: nkf
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Parag AN(पराग)
Fedora Package Reviews List
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2007-01-31 15:16 EST by Nobody's working on this, feel free to take it
Modified: 2014-09-02 08:19 EDT (History)
3 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2007-09-25 05:43:13 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
panemade: fedora‑review+
limburgher: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Nobody's working on this, feel free to take it 2007-01-31 15:16:50 EST
Fedora Merge Review: nkf

http://cvs.fedora.redhat.com/viewcvs/devel/nkf/
Initial Owner: tagoh@redhat.com
Comment 1 Parag AN(पराग) 2007-09-20 04:50:06 EDT
preliminary review ->
rpmlint complained
nkf.src:29: W: rpm-buildroot-usage %prep %{__rm} -rf    $RPM_BUILD_ROOT
$RPM_BUILD_ROOT should not be touched during %build or %prep stage, as it
will break short circuiting.

nkf.src:30: W: rpm-buildroot-usage %prep %{__mkdir_p}   $RPM_BUILD_ROOT/%{_bindir}
$RPM_BUILD_ROOT should not be touched during %build or %prep stage, as it
will break short circuiting.

nkf.src:31: W: rpm-buildroot-usage %prep %{__mkdir_p}  
$RPM_BUILD_ROOT%{_mandir}/{man1,ja/man1}
$RPM_BUILD_ROOT should not be touched during %build or %prep stage, as it
will break short circuiting.

nkf.src:39: W: rpm-buildroot-usage %build CFLAGS="$RPM_OPT_FLAGS" perl
Makefile.PL PREFIX=$RPM_BUILD_ROOT%{_prefix} INSTALLDIRS=vendor
$RPM_BUILD_ROOT should not be touched during %build or %prep stage, as it
will break short circuiting.

===> Remove %prep section lines add them to %build

nkf.src: W: summary-ended-with-dot A Kanji code conversion filter.
Summary ends with a dot.

perl-NKF.i386: W: hidden-file-or-dir
/usr/lib/perl5/vendor_perl/5.8.8/i386-linux-thread-multi/auto/NKF/.packlist
The file or directory is hidden. You should see if this is normal,
and delete it from the package if not.

perl-NKF.i386: W: perl-temp-file
/usr/lib/perl5/vendor_perl/5.8.8/i386-linux-thread-multi/auto/NKF/.packlist
You have a perl temporary file in your package. Usually, this
file is beginning with a dot (.) and contain "perl" in its name.

perl-NKF.i386: E: non-standard-executable-perm
/usr/lib/perl5/vendor_perl/5.8.8/i386-linux-thread-multi/auto/NKF/NKF.so 0555
A standard executable should have permission set to 0755. If you get this
message, it means that you have a wrong executable permissions in some files
included in your package.

nkf.src: W: no-url-tag
The URL tag is missing.


SHOULD:
  Add missing BR perl-ExtUtils-MakeMaker
  Preserve timestamps
  clean buildroot in install stage
Comment 2 Akira TAGOH 2007-09-20 23:54:58 EDT
Thanks for reviewing. the below should works:

Spec file: http://tagoh.fedorapeople.org/nkf/nkf.spec
SRPM file: http://tagoh.fedorapeople.org/nkf/nkf-2.0.8b-1.fc8.src.rpm
Comment 3 Parag AN(पराग) 2007-09-21 01:07:58 EDT
thanks.
source URL though not worked with wget but worked in browser. I guess it should
be problem.
Comment 4 Mamoru TASAKA 2007-09-21 01:20:27 EDT
In reply to comment #3)
> thanks.
> source URL though not worked with wget but worked in browser. I guess it should
> be problem.
> 
$ wget -N http://downloads.sourceforge.jp/nkf/26243/nkf-2.0.8b.tar.gz

should work.
Comment 5 Parag AN(पराग) 2007-09-21 01:36:58 EDT
yes source URL working fine. looks I got some problem. its working fine.
Review:
+ package builds in mock (development i386).
+ rpmlint is silent for SRPM and RPM.
+ source files match upstream.
1851260a2719629294740783c14ca3d5  nkf-2.0.8b.tar.gz
+ package meets naming and packaging guidelines.
+ specfile is properly named, is cleanly written
+ Spec file is written in American English.
+ Spec file is legible.
+ dist tag is present.
+ build root is correct.
+ license is open source-compatible.
+ License text is included in package.
+ %doc files present.
+ BuildRequires are proper.
+ %clean is present.
+ package installed properly.
+ Macro use appears rather consistent.
+ Package contains code.
+ no static libraries.
+ no .pc file present.
+ no -devel subpackage exists.
+ no .la files.
+ no translations are available.
+ Does owns the directories it creates.
+ no duplicates in %files.
+ file permissions are appropriate.
+ no scriptlets are used.
+ Pacakge nkf-2.0.8b-1.fc8 ->
  Requires: libc.so.6 libc.so.6(GLIBC_2.0) libc.so.6(GLIBC_2.1) rtld(GNU_HASH)
  Package perl-NKF-2.0.8b-1.fc8 ->
  Provides: NKF.so perl(NKF) = 2.08
  Requires: libc.so.6 libc.so.6(GLIBC_2.0) libc.so.6(GLIBC_2.1.3)
libc.so.6(GLIBC_2.3.4) libc.so.6(GLIBC_2.4) perl(DynaLoader) perl(Exporter)
perl(strict) perl(vars) rtld(GNU_HASH)
+ Not a GUI app.
APPROVED.
Comment 6 Mamoru TASAKA 2007-09-21 01:56:07 EDT
Sorry, but please wait.

* Compilation does not honor %optflags
  http://koji.fedoraproject.org/koji/getfile?taskID=168881&name=build.log
------------------------------------------------------
+ make nkf
cc -O -c utf8tbl.c
cc -O -o nkf nkf.c utf8tbl.o
------------------------------------------------------

* perl macro
  - Please use %perl_vendorarch or %perl_vendorlib

* Timestamp
  - For SOURCE3, please use "cp -p" to keep timestamp.

* defattr
  - Now we recommend %defattr(-,root,root,-)

* Using direct command vs using macro command
  - Please choose if you use write commands directly, or
    use macros as much as possible.

    For example, if you want to use %__install_p, you also
    want to use %__rm, %__cp, %__make, %__chmod.
Comment 7 Parag AN(पराग) 2007-09-21 02:31:00 EDT
(In reply to comment #6)
> Sorry, but please wait.
> 
> * Compilation does not honor %optflags
>   http://koji.fedoraproject.org/koji/getfile?taskID=168881&name=build.log
> ------------------------------------------------------
> + make nkf
> cc -O -c utf8tbl.c
> cc -O -o nkf nkf.c utf8tbl.o
> ------------------------------------------------------

  use make CFLAGS="$RPM_OPT_FLAGS"  nkf

> 
> * perl macro
>   - Please use %perl_vendorarch or %perl_vendorlib
  ohh I missed to check perl macors.
> 
> * Timestamp
>   - For SOURCE3, please use "cp -p" to keep timestamp.
   yes. this is still missing in SPEC.
> 
> * defattr
>   - Now we recommend %defattr(-,root,root,-)
   I am still unclear on usage of this as I guess some packagers still prefer
%defattr(-,root,root) bu tI have seen you always asking packagers to use
%defattr(-,root,root,-) but can't find this thing written in packaging guidelines.
> 
> * Using direct command vs using macro command
>   - Please choose if you use write commands directly, or
>     use macros as much as possible.
> 
>     For example, if you want to use %__install_p, you also
>     want to use %__rm, %__cp, %__make, %__chmod.

 I better should look carefully on macros from next time while doing reviews.

Mamoru,
  Thanks for commenting missing things here.
Comment 8 Akira TAGOH 2007-09-21 03:11:05 EDT
Thanks for catching them up. should be ok now with the same URL above.
Comment 9 Mamoru TASAKA 2007-09-21 03:48:49 EDT
(In reply to comment #7)

> > * defattr
> >   - Now we recommend %defattr(-,root,root,-)
>    I am still unclear on usage of this as I guess some packagers still prefer
> %defattr(-,root,root) bu tI have seen you always asking packagers to use
> %defattr(-,root,root,-) but can't find this thing written in 
> packaging guidelines.
  - Actually also I cannot see the explicit mention in wiki,
    however
    * I was told by some other reviewers to do so
    * and all the examples under http://fedoraproject.org/wiki/Packaging/
     (for example, http://fedoraproject.org/wiki/Packaging/Guidelines )
     uses %defattr(-,root,root,-)

(In reply to comment #8)
> Thanks for catching them up. should be ok now with the same URL above.
- Sorry, however more issues:

* Perl module BuildRequires
  - For Perl module BuildRequires, please do not use the name of
    rpms but write the module name of it like
    http://fedoraproject.org/wiki/Packaging/Perl
    i.e. "BuildRequires: perl(ExtUtils::MakeMaker)" is better.

* Directory ownership issue
   - %{perl_vendorarch}/auto itself is owned by perl and
     this package should not own the directory.
Comment 10 Mamoru TASAKA 2007-09-21 03:52:32 EDT
* And please change the SourceURL.
Comment 11 Parag AN(पराग) 2007-09-21 04:30:00 EDT
(In reply to comment #9)
> (In reply to comment #7)
> 
> > > * defattr
> > >   - Now we recommend %defattr(-,root,root,-)
> >    I am still unclear on usage of this as I guess some packagers still prefer
> > %defattr(-,root,root) bu tI have seen you always asking packagers to use
> > %defattr(-,root,root,-) but can't find this thing written in 
> > packaging guidelines.
>   - Actually also I cannot see the explicit mention in wiki,
>     however
>     * I was told by some other reviewers to do so
>     * and all the examples under http://fedoraproject.org/wiki/Packaging/
>      (for example, http://fedoraproject.org/wiki/Packaging/Guidelines )
>      uses %defattr(-,root,root,-)
> 
    Yes. I also checked same when I saw your comments asking to use that
%defattr(-,root,root,-). But I guess its better to have it mentioned in Guidelines.
> (In reply to comment #8)
> > Thanks for catching them up. should be ok now with the same URL above.
> - Sorry, however more issues:
> 
> * Perl module BuildRequires
>   - For Perl module BuildRequires, please do not use the name of
>     rpms but write the module name of it like
>     http://fedoraproject.org/wiki/Packaging/Perl
>     i.e. "BuildRequires: perl(ExtUtils::MakeMaker)" is better.
> 
> * Directory ownership issue
>    - %{perl_vendorarch}/auto itself is owned by perl and
>      this package should not own the directory.

Comment 12 Akira TAGOH 2007-09-21 04:55:59 EDT
(In reply to comment #9)
> * Perl module BuildRequires
>   - For Perl module BuildRequires, please do not use the name of
>     rpms but write the module name of it like
>     http://fedoraproject.org/wiki/Packaging/Perl
>     i.e. "BuildRequires: perl(ExtUtils::MakeMaker)" is better.
> 
> * Directory ownership issue
>    - %{perl_vendorarch}/auto itself is owned by perl and
>      this package should not own the directory.

Doh. thanks again. hopefully this should be ok then.

(In reply to comment #10)
> * And please change the SourceURL.

have to bump a release?
Comment 13 Mamoru TASAKA 2007-09-21 06:09:31 EDT
(In reply to comment #12)
> Doh. thanks again. hopefully this should be ok then.
  Seems okay for me.

> (In reply to comment #10)
> > * And please change the SourceURL.
> 
> have to bump a release?
  This nkf is not built yet so bumping release number is not
  needed.

Comment 14 Akira TAGOH 2007-09-25 05:43:13 EDT
Ok, the updated nkf has been built on koji.
Comment 15 Akira TAGOH 2014-09-01 00:04:20 EDT
Package Change Request
======================
Package Name: nkf
New Branches: epel7
Owners: tagoh
InitialCC: i18n-team
Comment 16 Jon Ciesla 2014-09-02 08:19:37 EDT
Git done (by process-git-requests).

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