Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=525786 --- Comment #2 from Mamoru Tasaka <mtasaka@xxxxxxxxxxxxxxxxxxx> 2009-09-27 13:15:19 EDT --- Some random commends: * License statement of the spec file itself - It is not forbidden, however ususally spec files in Fedora packages don't contain license statements on the spec files themselves. Moreover: -------------------------------------------------------- # Copyright (c) 2001-2009 John Graham-Cumming # This file is part of POPFile -------------------------------------------------------- - Why is the copyright holder of your spec file not you? - Is this spec file really "a part of POPFile"? * Naming/EVR (Epoch-Version-Release) - The release number of the spec file should have integer value except for the cases written in https://fedoraproject.org/wiki/Packaging/NamingGuidelines#Package_Release - And usually adding %{?dist} tag is preferred. https://fedoraproject.org/wiki/Packaging/DistTag * Vendor - Vendor tag must be removed on Fedora. This tag is automatically imported on Fedora build server. * Exclusiveos - I don't think you have to write this explicitly. * Perl related packaging treatment https://fedoraproject.org/wiki/Packaging/Perl Some notes: - "Requires: perl >= 5.8.1" does almost nothing because Fedora's perl rpm has epoch: ------------------------------------------------------ $ rpm -q --qf '%{epoch}:%{name}-%{version}-%{release}\n' perl 4:perl-5.10.0-82.fc12 ------------------------------------------------------ - And anyway I don't think this is needed because even Fedora 10 (the oldest branch currently supported on Fedora) uses perl 5.10. - For perl module dependency, please write virtual Provides names instead of rpm names directly: https://fedoraproject.org/wiki/Packaging/Perl#Perl_Requires_and_Provides * rpmlint issue ------------------------------------------------------ popfile.src: W: strange-permission popfile 0775 popfile.src: W: strange-permission start_popfile.sh 0775 ------------------------------------------------------ - All files in srpm should have 0644 permission. * %setup/%build/%install - Umm... I think your current spec file is unneededly verbose and redundant. While listing %files section verbosely can be accepted, is there any reason you don't prefer to write like below? (all commentes removed for now) ------------------------------------------------------ %prep %setup -q -c find . -type f | xargs chmod 0644 %{__cp} -p %{SOURCE1} . %{__cp} -p %{SOURCE2} . %build %install %{__rm} -rf $RPM_BUILD_ROOT %{__mkdir_p} $RPM_BUILD_ROOT%{_datadir}/%{name} %{__cp} -a * $RPM_BUILD_ROOT%{_datadir}/%{name}/ %{__rm} -f $RPM_BUILD_ROOT%{_datadir}/%{name}/popfile %{__mkdir_p} $RPM_BUILD_ROOT%{_localstatedir}/lib/%{name} %{__install} -p -m 644 stopwords $RPM_BUILD_ROOT%{_localstatedir}/lib/%{name} %{__mkdir_p} $RPM_BUILD_ROOT%{_localstatedir}/log/%{name} %{__mkdir_p} $RPM_BUILD_ROOT%{_initrddir} %{__install} -p -m 755 popfile $RPM_BUILD_ROOT%{_initrddir}/popfile %{__install} -p -m 755 start_popfile.sh $RPM_BUILD_ROOT%{_datadir}/%{name}/ ------------------------------------------------------ Some notes: - %setup recognizes zip file. "-c" option on setup creates the diretory beforehand when unpacking source. Also "-q" option is preferred to suppress messages when unpacking source. - When using "install" or "cp" commands, add "-p" option (or "-a" for "cp") to keep timestamps on installed files: https://fedoraproject.org/wiki/Packaging/Guidelines#Timestamps - If you want to close macros with {}, please do so consistently (i.e. please use %{name}) * Initscripts treatment Please refer to: https://fedoraproject.org/wiki/Packaging/SysVInitScript#Initscripts_in_spec_file_scriptlets Some notes: - Some Requires(post) or so are missing - Why your spec file stopps daemon every time this package is upgraded (on %pre)? https://fedoraproject.org/wiki/Packaging/ScriptletSnippets#Syntax ! Also please check %postun scriptlet example (and the usage of condrestart) - Please use either "/sbin/service popfile" style or "%{_initrddir}/popfile" style consistently - Currently using %{_initddir} macro instead of %{_initrddir} is preferred: https://fedoraproject.org/wiki/Packaging/SysVInitScript#Initscripts_on_the_filesystem - rpmlint warns: ----------------------------------------------------------------------------- popfile.noarch: W: service-default-enabled /etc/rc.d/init.d/popfile ----------------------------------------------------------------------------- Usually service should be enabled by admin manually afterwards and should not be enabled by default. So popfile initscript should have ----------------------------------------------------------------------------- # chkconfig: - 80 20 ----------------------------------------------------------------------------- And "Default-Start", "Default-Stop" lines should be removed https://fedoraproject.org/wiki/Packaging/SysVInitScript#.23_chkconfig:_line https://fedoraproject.org/wiki/Packaging/SysVInitScript#.23_Default-Start:_line * %files - Now %defattr(-,root,root,-) is preferred on Fedora. - By the way ----------------------------------------------------------------------------- %files %dir %{_datadir}/%{name}/Classifier %{_datadir}/%{name}/Classifier/* ----------------------------------------------------------------------------- can be unified as ----------------------------------------------------------------------------- %files %{_datadir}/%{name}/Classifier/ ----------------------------------------------------------------------------- The latter %files entry contains both the directory %{_datadir}/%{name}/Classifier itself and all files/directories/etc under the directory. -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are on the CC list for the bug. _______________________________________________ Fedora-package-review mailing list Fedora-package-review@xxxxxxxxxx http://www.redhat.com/mailman/listinfo/fedora-package-review