Please review this package. Bugzilla for old package review of vixie-cron #226529.
Just judging by the name - ew. fcron?
I forgot. Package could be found here: http://mmaslano.fedorapeople.org/vixie-cron-ng/ [1] My fork hasn't anything common with fcron.
The point is - why bother forking the ancient cruft of vixie-cron when there are other upstream projects with more features, etc.?
there still seems to be some problematic issues to me. The release is broken since the autotools generated files don't seem to be there. Also in the release, a proper changelog should be there. If the changelog is in git commit messages, you should use the proper tool to generate a changelog file from the VCS. The upstream tarball cannot be found.
to [3] because I can't say selinux and pam in fcron is ok. I didn't test many other features and the main reason is the vixie-cron-ng will support inotify, which is needed for battery saving. I don't see anything ancient on using inotify ;-)
to [4] I'm sorry that takes so long. I was thought about auto tools and I've finally attached ./configure into source code. Also CHANGES were generated from git-log.
(In reply to comment #5) > to [3] because I can't say selinux and pam in fcron is ok. I didn't test many > other features and the main reason is the vixie-cron-ng will support inotify, > which is needed for battery saving. I don't see anything ancient on using > inotify ;-) Sure, but I think it might be better to work with an existing upstream project on these features if possible (akin to using/extending rsyslog rather than forking ourselves.) Do we have other distros interested in picking up our fork?
[7] I hope that they will. I was looking in other distribution, which cron they are using and they have some fork of vixie-cron. I think on the servers everyone is using some vixie-cron with patches.
CHECK: - pkg name possibly in conflict with 3rd clause of BSD license (no promotion or endorsement with contributor names) -- search for alternative name is in progress - rpmlint checks return: vixie-cron-ng.src:17: W: unversioned-explicit-provides vixie-cron --> Possibly change that to "Provides: vixie-cron = 4:4.3" (or whatever else vixie-cron version this package replaces, just be sure to list the epoch as well) vixie-cron-ng.src: W: mixed-use-of-spaces-and-tabs (spaces: line 80, tab: line 92) --> use either spaces or tabs consistently for indentation, not both - license text not in %doc, possibly add AUTHORS, CHANGES, README as well - source matches upstream, but the URL is incorrect. Because the tarball is attached to a Trac wiki page, "?format=raw" is needed to actually download the tarball. Perhaps add a comment with points to the real URL (as changing the "Source: ..." line won't work with koji, it can't handle CGI parameters) - possibly unnecessary "BR: automake", you don't use it anywhere in the spec file - macro use inconsistent: use '%{_initrddir}' instead of '%{_sysconfdir}/rc.d/init.d' in the spec file - what purpose does "make ... RPM_OPT_FLAGS="$RPM_OPT_FLAGS -DLINT -Dlint" have? - %postun mustn't fail (append " || :" like in %preun) - "make install" does much too few which you workaround in the spec file by creating directories, copying/moving stuff to the directories, I can help with automake/autofoo if you wish so "make install" just works (possibly followup issues of the last one: - no need to use %attr() for all the files if "make install" just does "the right thing" - don't move files out of the source/build directory as it breaks "rpm -bi --short-circuit", just copy them, or better let them be installed with "make install" - instead of renaming %{_sysconfdir}/pam.d/crond.pam to .../crond, just let "make install" do the right thing (i.e. install the file as "crond", not "crond.pam") ) GOOD: - package meets Fedora naming guidelines - license (BSD and MIT) OK, matches source - spec file legible, in am. english - package compiles on devel (x86_64) - no missing BR - no locales - not relocatable - owns all directories that it creates - no duplicate files - permissions ok - %clean ok - code, not content - no need for -docs - nothing in %doc affects runtime - no need for .desktop file
I've fixed some of mentioned problems. I'm working on better Makefile. The package was renamed on cronie (successor of cron) and I didn't find any problems with this name. git and page on fedorahosted will be renamed, I've already create a ticket.
And the new source is here: http://mmaslano.fedorapeople.org/cronie/
I like the name. I have some comments on upstream. * I think it would be better to isolate crond.pam, crond.sysconfig vixie-cron.init (renamed cronie.init) in something like a contrib directory and not install them, they are platform specific. * you should not use AC_TRY_RUN, but AC_TRY_LINK (or similar) otherwise cross compilation is impossible. * you should use sysconfdir instead of etcdir * There are some strange unconditional AC_DEFINE. * The following shouldn't be useful: crond_LDFLAGS = \ $(LDFLAGS) * cp in man/Makefile.am are wrong. automake should be able to figure out what to do with these files, and if it cannot there are better way to handle them. * The fact that vixie-cron.init and crond.sysconfig appear in the distribution is complete magic to me * You should copy the automake/autoconf files, not link them. Like automake -c or simply autoreconf * there are many things in src/pathnames.h that should be set from configure values. * there is an url missing in the README I can make patches for the things for which patches make sense. Is there a version control somewhere for cronie?
CHECK: - Summary and description contain bad English, summary is too long, I'd use: Summary: Cron daemon for executing programs at set times %description Cronie contains the standard UNIX daemon crond that runs specified programs at scheduled times and related tools. It is a fork of the original vixie-cron and has security and configuration enhancements like the ability to use pam and SELinux. - URL in the spec file doesn't match new name (needs action from Fedora infrastructure) - Add the epoch to "Obsoletes: vixie-cron <= 4.3" otherwise upgrading won't work (a missing epoch has the same effect as a zero epoch) - Unnecessary build requirement on automake (you don't use it in the spec file, if the Makefiles try to rebuild themselves, there's something wrong) - %preun mustn't fail, add ' || :' to the chkconfig command as well (it's always the last command that defines the exit code of a %pre/%post/un scriptlet) - Get rid of the %triggerpostun, the vixie-cron version where it would trigger is older than RHEL2.1 Likely not harmful, but odd: - I don't know why you have this in %build: CFLAGS="$RPM_OPT_FLAGS"; export CFLAGS [...] %configure \ [...] If you use %configure, it already evaluates $RPM_OPT_FLAGS. - what purpose does "make ... RPM_OPT_FLAGS="$RPM_OPT_FLAGS -DLINT -Dlint" have? Does the Makefile make use of $RPM_OPT_FLAGS (which wouldn't be good IMHO)? I take it your working on the following issues in the course of you Makefile fixes: - "make install" does much too few which you workaround in the spec file by creating directories, copying/moving stuff to the directories, I can help with automake/autofoo if you wish so "make install" just works (possibly followup issues of the last one: - no need to use %attr() for all the files if "make install" just does "the right thing" - don't move files out of the source/build directory as it breaks "rpm -bi --short-circuit", just copy them, or better let them be installed with "make install" - instead of renaming %{_sysconfdir}/pam.d/crond.pam to .../crond, just let "make install" do the right thing (i.e. install the file as "crond", not "crond.pam") ) GOOD: - pkg name issue fixed - rpmlint checks return nothing - license text and other documentation in %doc - consistent use of macros
(In reply to comment #12) > I have some comments on upstream. > * I think it would be better to isolate crond.pam, crond.sysconfig > vixie-cron.init (renamed cronie.init) in something like a contrib > directory and not install them, they are platform specific. IMO contrib/ directories are usually for stuff contributors submit but upstream doesn't plan to support (which clearly is not the case here). I'd add e.g. --with-pam and --with-sysvinit to configure.ac and install the files if the user wants it. Marcela, shout if you want help with that. > * The fact that vixie-cron.init and crond.sysconfig appear in the > distribution is complete magic to me What do you mean?
[12] Thanks, the git archive could be found on https://hosted.fedoraproject.org/vixie-cron [9] source could be also from my page http://mmaslano.fedorapeople.org/cronie/
(In reply to comment #14) > (In reply to comment #12) > > I have some comments on upstream. > > * I think it would be better to isolate crond.pam, crond.sysconfig > > vixie-cron.init (renamed cronie.init) in something like a contrib > > directory and not install them, they are platform specific. > > IMO contrib/ directories are usually for stuff contributors submit but upstream > doesn't plan to support (which clearly is not the case here). I'd add e.g. > --with-pam and --with-sysvinit to configure.ac and install the files if the user > wants it. Marcela, shout if you want help with that. Yes, that would be another possibility. And indeed contrib is a bad name for a directory holding those files. Maybe data or utils? In any case they shouldn't be installed in the default case since they are too platform dependent. > > * The fact that vixie-cron.init and crond.sysconfig appear in the > > distribution is complete magic to me > > What do you mean? I may have missed something but they are not in any variable in Makefile.am nor in EXTRA_DIST...
[14] with-pam is defined, I can add with-sysvinit. [15] I can move these files into some /data. I think you mean installing crond.pam etc. with Makefile.am.
Is it the plan to replace vixie cron with this by default in Fedora 9? If so, following the feature process would be appropriate http://fedoraproject.org/wiki/Features/Policy
(In reply to comment #18) > Is it the plan to replace vixie cron with this by default in Fedora 9? If so, > following the feature process would be appropriate > > http://fedoraproject.org/wiki/Features/Policy As we already had this forked version (albeit under the old name vixie-cron) in F-8, this is merely a re-packaged version of that with the most user-visible change being the change of the package name. I don't think that counts as a new feature (but I may be wrong).
[18] Fork is already in rawhide, but now is renaming of package needed. I have to also in this case go through Features/Policy?
I think the upstream notes should move on upstream page https://fedorahosted.org/cronie/ and here should be discussed only packaging, rpmlint output and so on.
I try to fix the makefile and configure according to comments #12 an #13, but I didn't find how to install files like cronie.init into system like /etc/sysconfig/crond (in Makefile.ac). I'll be really happy for a patch. The files crond.sysconfig and crond.pam could be in directory f.e. /etc, if it's really needed. Did anyone know if the announcement in Features is needed?
Forgot, updated source are here: http://mmaslano.fedorapeople.org/cronie and git is here: http://git.fedorahosted.org/git/cronie.git
Ping? The autotools are really fixed. Now is nothing wrong with packaging or spec file. I think there are now other serious problems, which could slower the review of package.
http://mmaslano.fedorapeople.org/cronie/
CHECK: - Add the epoch to "Obsoletes: vixie-cron <= 4.3" otherwise upgrading won't work (a missing epoch has the same effect as a zero epoch) - Unnecessary build requirement on automake (you don't use it in the spec file, if the Makefiles try to rebuild themselves, there's something wrong) - %preun mustn't fail, add ' || :' to the chkconfig command as well (it's always the last command that defines the exit code of a %pre/%post/un scriptlet). Alternatively, just append "exit 0" to all the scriptlets. See the Fedora Wiki for details ("Syntax" section): http://fedoraproject.org/wiki/Packaging/ScriptletSnippets (I've mentioned the issues above in comment #13 already) - You unconditionally remove $RPM_BUILD_ROOT%{_sysconfdir}/pam.d/crond.pam in the %install section (the %if conditional is commented out). You probably should remove the comments so that the file only gets removed if %{with_pam} isn't defined, and change the command to: rm -f $RPM_BUILD_ROOT%{_sysconfdir}/pam.d/crond I.e. the file name is "crond", not "crond.pam" and recursive removal isn't needed as it's not a directory. GOOD: - Summary and description fixed - URL fixed (will this be the final location?) - Obsolete %triggerpostun removed - Unnecessary use of $RPM_OPT_FLAGS removed - Files aren't moved out of the source/build directory Have you checked whether you can leave out the individual %attr flags in the %files section, i.e. if "make install" installs the files with the proper %permissions?
(In reply to comment #26) > CHECK: > - Add the epoch to "Obsoletes: vixie-cron <= 4.3" otherwise upgrading won't > work (a missing epoch has the same effect as a zero epoch) Not really, this is true only the other way around - if it is not specified in comparsions for obsoletes/requires etc. it is not compared. So it doesn't matter what epoch the vixie-cron package is. So this is OK.
> - Add the epoch to "Obsoletes: vixie-cron <= 4.3" otherwise upgrading won't > work (a missing epoch has the same effect as a zero epoch) It's ok (comment #27) > Unnecessary build requirement on automake (you don't use it in the spec file, > if the Makefiles try to rebuild themselves, there's something wrong) Removed. > - %preun mustn't fail, add ' || :' to the chkconfig command as well (it's > always the last command that defines the exit code of a %pre/%post/un > scriptlet). Alternatively, just append "exit 0" to all the scriptlets. > See the Fedora Wiki for details ("Syntax" section): > http://fedoraproject.org/wiki/Packaging/ScriptletSnippets Added || : > - You unconditionally remove $RPM_BUILD_ROOT%{_sysconfdir}/pam.d/crond.pam in > the %install section (the %if conditional is commented out). You probably > should remove the comments so that the file only gets removed if %{with_pam} The autotools can install crond in the correct place. If someone change it in spec without pam, then the installed file will be removed. I renamed false cron.pam on crond. > Have you checked whether you can leave out the individual %attr flags in the > %files section, i.e. if "make install" installs the files with the proper > %permissions? I removed as many permissions as I dare. I uploaded correct spec file and I'd also built the package in mock.
(In reply to comment #27) > (In reply to comment #26) > > CHECK: > > - Add the epoch to "Obsoletes: vixie-cron <= 4.3" otherwise upgrading won't > > work (a missing epoch has the same effect as a zero epoch) > Not really, this is true only the other way around - if it is not specified in > comparsions for obsoletes/requires etc. it is not compared. So it doesn't matter > what epoch the vixie-cron package is. So this is OK. > I just tried it, updating to my "test-new" package wouldn't get the epoched "test-old" package removed: nils@gibraltar:~> rpm -qa test\* test-old-4.2-1 test-new-1-1 nils@gibraltar:~> rpm -q --qf '%{epoch}:%{v}-%{r}\n' test-old 4:4.2-1 nils@gibraltar:~> rpm -q --obsoletes test-new test-old <= 4.3
(In reply to comment #28) > > - Add the epoch to "Obsoletes: vixie-cron <= 4.3" otherwise upgrading won't > > work (a missing epoch has the same effect as a zero epoch) > It's ok (comment #27) Unforunately not (comment #29 ;-). Otherwise the package is okay now.
Now I'm convinced. I had test it myself. So I updated again. And the URL, I have to use url from my web page, because the fedorahosted isn't functional yet.
(In reply to comment #31) > And the URL, I have to use url from my web page, because the fedorahosted isn't > functional yet. I thought so. APPROVED.
I'm sorry for the mistake about the epoch. It works the way I wrote for release but apparently not for epoch.
I may have missed something but the source url doesn't work for me, http://mmaslano.fedorapeople.org/cronie/cronie-1.0.tar.gz cannot be found, maybe because fedorahosted is down, but the source from the home page doesn't look quite like the source in the srpm. Anyway it looks like the most importants and blocking comments I had about upstream are fixed in the source found in the srpm.
Thanks all for comment. Everything fixed.
New Package CVS Request ======================= Package Name: cronie Short Description: time scheduler Owners: mmaslano Cvsextras Commits: yes
Please use the template completely including braches and InitialCC I cant process this because i dont know what braches you want the package for
I didn't write branch, because it's only for rawhide, but ok here it is. New Package CVS Request ======================= Package Name: cronie Short Description: task scheduler Owners: mmaslano Branches: rawhide Cvsextras Commits: yes
Please use fedora account name in Owners field. cvs done.
Ok, owners should be only: mmaslano Thank you for quick cvs branching.
The obsoletes is still wrong. You're providing /and/ obsoleting the same vixie-cron version: Provides: vixie-cron = 4:4.3 Obsoletes: vixie-cron <= 4:4.3 You just need to take the '=' out of the Obsoletes and you'll likely be fine. Although, the latest vixie-cron build is 4:4.3-2. This means the existing vixie-cron won't actually be obsoleted: $ rpmdev-vercmp 4:4.3 4:4.3-2.fc9 4:4.3-2.fc9 is newer Please fix your obsoletes/provides.
(In reply to comment #41) > The obsoletes is still wrong. You're providing /and/ obsoleting the same > vixie-cron version: > > Provides: vixie-cron = 4:4.3 > Obsoletes: vixie-cron <= 4:4.3 > > You just need to take the '=' out of the Obsoletes and you'll likely be fine. > > Although, the latest vixie-cron build is 4:4.3-2. This means the existing > vixie-cron won't actually be obsoleted: > > $ rpmdev-vercmp 4:4.3 4:4.3-2.fc9 > 4:4.3-2.fc9 is newer This is OK - see http://marc.info/?l=fedora-devel-list&m=119679575625910&w=2 But the provides should provide something newer - So use Provides: vixie-cron = 4:4.3.1 or something like that.
*** Bug 226529 has been marked as a duplicate of this bug. ***