Fedora Account System
Red Hat Associate
Red Hat Customer
Spec URL: http://getpopfile.org/browser/trunk/linux/fedora/popfile.spec?format=raw SRPM URL: http://getpopfile.org/downloads/popfile-1.1.1-1.1.src.rpm Description: POPFile is an automatic mail classification tool. Once properly set up and trained, it will scan all email as it arrives and classify it based on your training. You can give it a simple job, like separating out junk e-mail, or a complicated one-like filing mail into a dozen folders. Think of it as a personal assistant for your inbox.
Hi, I've forgotten to mention that this is my first package and I need a sponsor.
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.
Thanks for comments. > * 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. OK. I've removed the license statements. > * 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 OK. I've update the release number to "2%{?dist}". > * Vendor > - Vendor tag must be removed on Fedora. This tag is automatically imported > on Fedora build server. OK. I've removed the Vendor tag. > * Exclusiveos > - I don't think you have to write this explicitly. OK. I've removed the Exclusiveos tag. > * Perl related packaging treatment > https://fedoraproject.org/wiki/Packaging/Perl > - "Requires: perl >= 5.8.1" does almost nothing because Fedora's perl > rpm has epoch: > <snip> > - And anyway I don't think this is needed because even Fedora 10 > (the oldest branch currently supported on Fedora) uses perl 5.10. OK. I've removed the Requires tag for perl. > - For perl module dependency, please write virtual Provides names > instead of rpm names directly: > https://fedoraproject.org/wiki/Packaging/Perl#Perl_Requires_and_Provides OK. I've updated the perl module dependencies to use virtual Provides names. > * 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. OK. I've applied appropriate permissions. > * %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) > ------------------------------------------------------ > <snip> Thanks. I've updated the %install section. > 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. I want to pass "-a" option to unzip for converting line endings of the text files. So that I splitted the %setup and %{__unzip}. > - 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 Thanks for the information. I've added "-p" option. > - If you want to close macros with {}, please do so consistently > (i.e. please use %{name}) OK. I've done. > * Initscripts treatment > <snip> > Some notes: > - Some Requires(post) or so are missing I've added Requres(post), Requires(preun) and Requires(postun). > - Why your spec file stopps daemon every time this package is > upgraded (on %pre)? > https://fedoraproject.org/wiki/Packaging/ScriptletSnippets#Syntax I wanted to stop the daemon before upgrading and restart it after upgrading. Now I've commented out '/sbin/service popfile stop'. > ! Also please check %postun scriptlet example (and the usage of > condrestart) I've written %postun script. > - Please use either "/sbin/service popfile" style or "%{_initrddir}/popfile" > style consistently I've updated the scripts to use "/sbin/service" style. > - Currently using %{_initddir} macro instead of %{_initrddir} is preferred: > https://fedoraproject.org/wiki/Packaging/SysVInitScript#Initscripts_on_the_filesystem I've updated the spec file to use %%{_initddir} macro instead of %%{_initrddir}. > - rpmlint warns: > ----------------------------------------------------------------------------- > popfile.noarch: W: service-default-enabled /etc/rc.d/init.d/popfile > ----------------------------------------------------------------------------- > <snip> OK. I've updated chkconfig line and deleted Default-Start and Default-Stop lines. > * %files > - Now %defattr(-,root,root,-) is preferred on Fedora. OK. I've updated %defattr. > - By the way > ----------------------------------------------------------------------------- > <snip> > ----------------------------------------------------------------------------- > can be unified as > ----------------------------------------------------------------------------- > %files > %{_datadir}/%{name}/Classifier/ > ----------------------------------------------------------------------------- > <snip> Thanks for the information. I've simplified the %files section. I've uploaded the new SPEC and SRPM files: Spec URL: http://getpopfile.org/browser/trunk/linux/fedora/popfile.spec?format=raw SRPM URL: http://getpopfile.org/downloads/popfile-1.1.1-2.fc11.src.rpm Naoki
Almost okay. * Documents directory - Usually documents (like "license" text) should be installed under %{_defaultdocdir}/%{name}-%{version}. If this package really wants document files to be unstalled under %{_datadir}/%{name}, it can be accepted. Otherwise please consider to move them to the directory used on Fedora by default. ! Note ------------------------------------------------------------- %files %doc license ------------------------------------------------------------- will do this automatically. * Empty scriptlets - Please remove %pre stage completely. Currently %pre stage essentially does nothing, however leaving this calls unneeded shell process (however Fedora suggests to leave %build stage even if this is empty) * logrotate file - rpmlint says: ------------------------------------------------------------- popfile.noarch: W: log-files-without-logrotate /var/log/popfile ------------------------------------------------------------- Please consider to create logrotate file (not a blocker).
Now: ------------------------------------------------------------- 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 package collection review requests which are waiting for someone to review can be checked on my wiki page: http://fedoraproject.org/wiki/User:Mtasaka#B._Review_request_tickets (Check "No one is reviewing") Review guidelines are described mainly on: http://fedoraproject.org/wiki/Packaging/ReviewGuidelines http://fedoraproject.org/wiki/Packaging/Guidelines http://fedoraproject.org/wiki/Packaging/ScriptletSnippets ------------------------------------------------------------
Thanks for another comments. > Almost okay. Thank you for reviewing. > * Documents directory > - Usually documents (like "license" text) should be installed > under %{_defaultdocdir}/%{name}-%{version}. OK. I've moved three document files (license and two changelogs) to the appropriate directory. > ! Note > ------------------------------------------------------------- > %files > %doc license > ------------------------------------------------------------- > will do this automatically. Thanks for the information. > * Empty scriptlets > - Please remove %pre stage completely. Currently %pre stage > essentially does nothing, however leaving this calls > unneeded shell process (however Fedora suggests to leave > %build stage even if this is empty) OK. Done. > * logrotate file > - rpmlint says: > ------------------------------------------------------------- > popfile.noarch: W: log-files-without-logrotate /var/log/popfile > ------------------------------------------------------------- > Please consider to create logrotate file (not a blocker). POPFile has its own log rotation feature. It checks log directory per hour and removes log file which is older than three days before. The new SPEC and SRPM files: SPEC URL: http://getpopfile.org/browser/trunk/linux/fedora/popfile.spec?format=raw SRPM URL: http://getpopfile.org/downloads/popfile-1.1.1-3.fc11.src.rpm And here's the full changes between release 2 and 3: http://getpopfile.org/changeset/3620/trunk/linux/fedora/popfile.spec Naoki
I appreciate your thoughtful message. > 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) OK. I'll try to pre-review someone's review request. > 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. I see. I'll do so. Naoki
I've done my first pre-review: https://bugzilla.redhat.com/show_bug.cgi?id=522920 And I've built SRPM on koji: http://koji.fedoraproject.org/koji/taskinfo?taskID=1716333 BTW, > http://fedoraproject.org/wiki/PackageMaintainers/HowToGetSponsored#Sponsorship_model > If you are an upstream author of the package you are submitting, or if you > are active in the community that surrounds it, please say so. I am a member of upstream's maintenance team (called POPFile Core Team): http://getpopfile.org/docs/Glossary:POPFileCoreTeam And as for my recent activities in the upstream project, please see: (check-ins to the repository (naoki is me)) http://getpopfile.org/log/branches/b0_22_2/engine (assigned or closed tickets owned by me) http://getpopfile.org/query?status=assigned&status=closed&owner=amatubu&order=priority Naoki
Okay. -------------------------------------------------------- This package (popfile) is APPROVED by mtasaka -------------------------------------------------------- Please follow the procedure written on: http://fedoraproject.org/wiki/PackageMaintainers/Join from "Install the Client Tools (Koji)". Now I am sponsoring you. If you want to import this package into Fedora 10/11/12, you also have to look at http://fedoraproject.org/wiki/Infrastructure/UpdatesSystem/Bodhi-info-DRAFT (after once you rebuilt this package on koji Fedora rebuilding system). If you have questions, please ask me. Removing NEEDSPONSOR.
(In reply to comment #9) > -------------------------------------------------------- > This package (popfile) is APPROVED by mtasaka > -------------------------------------------------------- Thanks. > Please follow the procedure written on: > http://fedoraproject.org/wiki/PackageMaintainers/Join > from "Install the Client Tools (Koji)". OK. I've built my SRPM using koji: Fedora 10 http://koji.fedoraproject.org/koji/taskinfo?taskID=1717407 Fedora 11 http://koji.fedoraproject.org/koji/taskinfo?taskID=1717412 Fedora 12(same as above) http://koji.fedoraproject.org/koji/taskinfo?taskID=1716333 Next, I'm going to do CVS admin request. > Now I am sponsoring you. Thank you. I'm glad to join the package maintainer! > If you want to import this package into Fedora 10/11/12, you also have > to look at > http://fedoraproject.org/wiki/Infrastructure/UpdatesSystem/Bodhi-info-DRAFT > (after once you rebuilt this package on koji Fedora rebuilding system). OK. I'll read the document. > If you have questions, please ask me. I appreciate you. Naoki
New Package CVS Request ======================= Package Name: popfile Short Description: Automatic Email Classification Owners: amatubu Branches: F-10 F-11 F-12 InitialCC:
cvs done.
popfile-1.1.1-3.fc10 has been submitted as an update for Fedora 10. http://admin.fedoraproject.org/updates/popfile-1.1.1-3.fc10
popfile-1.1.1-3.fc11 has been submitted as an update for Fedora 11. http://admin.fedoraproject.org/updates/popfile-1.1.1-3.fc11
Closing. Note that currently bodhi has not been set up to accept F-12 updates request. When bodhi begins to do so (a notification will be sent to fedora-devel-list ), please submit updates request for F-12.
Now bodhi began to accept F-12 updates request, so please submit it.
popfile-1.1.1-3.fc12 has been submitted as an update for Fedora 12. http://admin.fedoraproject.org/updates/popfile-1.1.1-3.fc12
(In reply to comment #16) > Now bodhi began to accept F-12 updates request, so please submit > it. Thanks. I've submitted update request. Naoki
popfile-1.1.1-3.fc10 has been pushed to the Fedora 10 stable repository. If problems still persist, please make note of it in this bug report.
popfile-1.1.1-3.fc11 has been pushed to the Fedora 11 stable repository. If problems still persist, please make note of it in this bug report.