Note: This is a beta release of Red Hat Bugzilla 5.0. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Also email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback here.
Bug 226200 - Merge Review: nkf
Summary: Merge Review: nkf
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Parag AN(पराग)
QA Contact: Fedora Package Reviews List
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2007-01-31 20:16 UTC by Nobody's working on this, feel free to take it
Modified: 2014-09-02 12:19 UTC (History)
3 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2007-09-25 09:43:13 UTC
panemade: fedora-review+
gwync: fedora-cvs+


Attachments (Terms of Use)

Description Nobody's working on this, feel free to take it 2007-01-31 20:16:50 UTC
Fedora Merge Review: nkf

http://cvs.fedora.redhat.com/viewcvs/devel/nkf/
Initial Owner: tagoh@redhat.com

Comment 1 Parag AN(पराग) 2007-09-20 08:50:06 UTC
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-21 03:54:58 UTC
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 05:07:58 UTC
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 05:20:27 UTC
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 05:36:58 UTC
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 05:56:07 UTC
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 06:31:00 UTC
(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 07:11:05 UTC
Thanks for catching them up. should be ok now with the same URL above.

Comment 9 Mamoru TASAKA 2007-09-21 07:48:49 UTC
(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 07:52:32 UTC
* And please change the SourceURL.

Comment 11 Parag AN(पराग) 2007-09-21 08:30:00 UTC
(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 08:55:59 UTC
(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 10:09:31 UTC
(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 09:43:13 UTC
Ok, the updated nkf has been built on koji.

Comment 15 Akira TAGOH 2014-09-01 04:04:20 UTC
Package Change Request
======================
Package Name: nkf
New Branches: epel7
Owners: tagoh
InitialCC: i18n-team

Comment 16 Gwyn Ciesla 2014-09-02 12:19:37 UTC
Git done (by process-git-requests).


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