[Bug 803089] Review Request: whenjobs - Replacement for cron with dependencies
https://bugzilla.redhat.com/show_bug.cgi?id=803089 Fedora Update System upda...@fedoraproject.org changed: What|Removed |Added Status|ON_QA |CLOSED Resolution|--- |ERRATA Last Closed||2012-07-28 20:50:33 --- Comment #39 from Fedora Update System upda...@fedoraproject.org --- whenjobs-0.7.3-1.fc17 has been pushed to the Fedora 17 stable repository. -- You are receiving this mail because: You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 803089] Review Request: whenjobs - Replacement for cron with dependencies
https://bugzilla.redhat.com/show_bug.cgi?id=803089
Michael Scherer m...@zarb.org changed:
What|Removed |Added
Status|NEW |ASSIGNED
--- Comment #31 from Michael Scherer m...@zarb.org ---
Ok, tested it, it start, it install fine, and compile.
Package Review
==
Key:
- = N/A
x = Pass
! = Fail
? = Not evaluated
C/C++
[x]: MUST Package does not contain any libtool archives (.la)
[x]: MUST Package does not contain kernel modules.
[x]: MUST Package contains no static executables.
[x]: MUST Rpath absent or only used for internal libs.
Generic
[x]: EXTRA Rpmlint is run on all installed packages.
Note: There are rpmlint messages (see attachment).
[x]: EXTRA Spec file according to URL is the same as in SRPM.
[x]: MUST Package is licensed with an open-source compatible license and meets
other legal requirements as defined in the legal section of Packaging
Guidelines.
[x]: MUST Package successfully compiles and builds into binary rpms on at
least one supported primary architecture.
[x]: MUST %build honors applicable compiler flags or justifies otherwise.
[x]: MUST All build dependencies are listed in BuildRequires, except for any
that are listed in the exceptions section of Packaging Guidelines.
[x]: MUST Buildroot is not present
Note: Unless packager wants to package for EPEL5 this is fine
[x]: MUST Package contains no bundled libraries.
[x]: MUST Changelog in prescribed format.
[x]: MUST Package has no %clean section with rm -rf %{buildroot} (or
$RPM_BUILD_ROOT)
Note: Clean would be needed if support for EPEL is required
[x]: MUST Sources contain only permissible code or content.
[x]: MUST Each %files section contains %defattr if rpm 4.4
Note: Note: defattr macros not found. They would be needed for EPEL5
[ ]: MUST Macros in Summary, %description expandable at SRPM build time.
[-]: MUST Package contains desktop file if it is a GUI application.
[-]: MUST Development files must be in a -devel package
[x]: MUST Package requires other packages for directories it uses.
[x]: MUST Package uses nothing in %doc for runtime.
[x]: MUST Package is not known to require ExcludeArch.
[x]: MUST Permissions on files are set properly.
[x]: MUST Package does not contain duplicates in %files.
[x]: MUST Package complies to the Packaging Guidelines
[x]: MUST Spec file lacks Packager, Vendor, PreReq tags.
[x]: MUST Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the
beginning of %install.
Note: rm -rf would be needed if support for EPEL5 is required
[-]: MUST Large documentation files are in a -doc subpackage, if required.
[x]: MUST If (and only if) the source package includes the text of the
license(s) in its own file, then that file, containing the text of the
license(s) for the package is included in %doc.
[-]: MUST License field in the package spec file matches the actual license.
Note: Checking patched sources after %prep for licenses. Licenses found:
*No copyright* GENERATED FILE, GPL (v2 or later) For detailed output
of licensecheck see file:
/home/misc/checkout/git/FedoraReview/803089-whenjobs/licensecheck.txt
[x]: MUST Package consistently uses macro is (instead of hard-coded directory
names).
[x]: MUST Package is named using only allowed ascii characters.
[x]: MUST Package is named according to the Package Naming Guidelines.
[x]: MUST Package does not generate any conflict.
Note: Package contains no Conflicts: tag(s)
[x]: MUST Package obeys FHS, except libexecdir and /usr/target.
[x]: MUST Package must own all directories that it creates.
[x]: MUST Package does not own files or directories owned by other packages.
[x]: MUST Package installs properly.
[x]: MUST Package is not relocatable.
[x]: MUST Requires correct, justified where necessary.
[x]: MUST Rpmlint is run on all rpms the build produces.
Note: There are rpmlint messages (see attachment).
[x]: MUST Sources used to build the package match the upstream source, as
provided in the spec URL.
[x]: MUST Spec file is legible and written in American English.
[x]: MUST Spec file name must match the spec package %{name}, in the format
%{name}.spec.
[-]: MUST Package contains systemd file(s) if in need.
[x]: MUST File names are valid UTF-8.
[x]: MUST Useful -debuginfo package or justification otherwise.
[x]: SHOULD Reviewer should test that the package builds in mock.
[x]: SHOULD If the source package does not include license text(s) as a
separate file from upstream, the packager SHOULD query upstream to
include it.
[x]: SHOULD Dist tag is present.
[x]: SHOULD No file requires outside of /etc, /bin, /sbin, /usr/bin,
/usr/sbin.
[x]: SHOULD Final provides and requires are sane (rpm -q --provides and rpm -q
--requires).
[x]: SHOULD Package functions as
[Bug 803089] Review Request: whenjobs - Replacement for cron with dependencies
https://bugzilla.redhat.com/show_bug.cgi?id=803089 Michael Scherer m...@zarb.org changed: What|Removed |Added Flags|fedora-review? |fedora-review+ -- You are receiving this mail because: You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 803089] Review Request: whenjobs - Replacement for cron with dependencies
https://bugzilla.redhat.com/show_bug.cgi?id=803089 Richard W.M. Jones rjo...@redhat.com changed: What|Removed |Added Flags||fedora-cvs? --- Comment #32 from Richard W.M. Jones rjo...@redhat.com --- New Package SCM Request === Package Name: whenjobs Short Description: Replacement for cron with dependencies Owners: rjones Branches: f17 InitialCC: -- You are receiving this mail because: You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 803089] Review Request: whenjobs - Replacement for cron with dependencies
https://bugzilla.redhat.com/show_bug.cgi?id=803089 --- Comment #33 from Jon Ciesla limburg...@gmail.com --- Git done (by process-git-requests). -- You are receiving this mail because: You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 803089] Review Request: whenjobs - Replacement for cron with dependencies
https://bugzilla.redhat.com/show_bug.cgi?id=803089 Richard W.M. Jones rjo...@redhat.com changed: What|Removed |Added Flags|fedora-cvs+ |fedora-cvs? --- Comment #34 from Richard W.M. Jones rjo...@redhat.com --- Thanks. Just building them now. -- You are receiving this mail because: You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 803089] Review Request: whenjobs - Replacement for cron with dependencies
https://bugzilla.redhat.com/show_bug.cgi?id=803089 --- Comment #35 from Jon Ciesla limburg...@gmail.com --- Already exists. -- You are receiving this mail because: You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 803089] Review Request: whenjobs - Replacement for cron with dependencies
https://bugzilla.redhat.com/show_bug.cgi?id=803089 --- Comment #36 from Richard W.M. Jones rjo...@redhat.com --- Sorry not sure what happened then. -- You are receiving this mail because: You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 803089] Review Request: whenjobs - Replacement for cron with dependencies
https://bugzilla.redhat.com/show_bug.cgi?id=803089 Fedora Update System upda...@fedoraproject.org changed: What|Removed |Added Status|ASSIGNED|MODIFIED -- You are receiving this mail because: You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 803089] Review Request: whenjobs - Replacement for cron with dependencies
https://bugzilla.redhat.com/show_bug.cgi?id=803089 --- Comment #37 from Fedora Update System upda...@fedoraproject.org --- whenjobs-0.7.3-1.fc17 has been submitted as an update for Fedora 17. https://admin.fedoraproject.org/updates/whenjobs-0.7.3-1.fc17 -- You are receiving this mail because: You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 803089] Review Request: whenjobs - Replacement for cron with dependencies
https://bugzilla.redhat.com/show_bug.cgi?id=803089 Fedora Update System upda...@fedoraproject.org changed: What|Removed |Added Status|MODIFIED|ON_QA --- Comment #38 from Fedora Update System upda...@fedoraproject.org --- whenjobs-0.7.3-1.fc17 has been pushed to the Fedora 17 testing repository. -- You are receiving this mail because: You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 803089] Review Request: whenjobs - Replacement for cron with dependencies
https://bugzilla.redhat.com/show_bug.cgi?id=803089
--- Comment #29 from Richard W.M. Jones rjo...@redhat.com ---
This doesn't seem to be to do with 32 bit. Xdr.safe_add is
an ocamlnet symbol, and whenproto_aux.ml is a generated file
(from /usr/bin/ocamlrpcgen). We need to add to %build:
rm -f lib/whenproto_aux.{ml,mli}
--
You are receiving this mail because:
You are on the CC list for the bug.
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 803089] Review Request: whenjobs - Replacement for cron with dependencies
https://bugzilla.redhat.com/show_bug.cgi?id=803089 --- Comment #30 from Richard W.M. Jones rjo...@redhat.com --- Updated: Spec URL: http://oirase.annexia.org/reviews/whenjobs/whenjobs.spec SRPM URL: http://oirase.annexia.org/reviews/whenjobs/whenjobs-0.7.3-1.fc17.src.rpm F17 scratch build: http://koji.fedoraproject.org/koji/taskinfo?taskID=4212335 -- You are receiving this mail because: You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 803089] Review Request: whenjobs - Replacement for cron with dependencies
https://bugzilla.redhat.com/show_bug.cgi?id=803089 --- Comment #22 from Richard W.M. Jones rjo...@redhat.com --- No, that's not normal. All I can think is that I must have done a 'make dist' twice and included another copy of the tarball. Here is a corrected SRPM: http://oirase.annexia.org/reviews/whenjobs/whenjobs-0.7.2-1.fc17.src.rpm -- You are receiving this mail because: You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 803089] Review Request: whenjobs - Replacement for cron with dependencies
https://bugzilla.redhat.com/show_bug.cgi?id=803089 --- Comment #23 from Michael Scherer m...@zarb.org --- It doesn't seems to build in mock ( who has been running since 2/3h ). It seems to block at : /bin/bash - ./test_run.sh ./t100_counter.ml Is this expected ( ie, it take a long time, I am running it on a macbook 2.1, core 2 duo cpu, 2.1 ghz ) ? ( to reproduce, fedora-review -b 803089 on fedora 17 should do the trick ). -- You are receiving this mail because: You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 803089] Review Request: whenjobs - Replacement for cron with dependencies
https://bugzilla.redhat.com/show_bug.cgi?id=803089 --- Comment #24 from Richard W.M. Jones rjo...@redhat.com --- Are you using 32 bit? This patch was required for 32 bit ARM: commit d6da1b74e241e79eb0af9c01e390e98ceead3a49 Author: Richard W.M. Jones rjo...@redhat.com Date: Sat Apr 28 20:40:39 2012 +0100 32 bit: Fix for 31 bit int overflow in time_t. so I'm going to do a 0.7.3 release in a moment which includes everything. -- You are receiving this mail because: You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 803089] Review Request: whenjobs - Replacement for cron with dependencies
https://bugzilla.redhat.com/show_bug.cgi?id=803089 --- Comment #25 from Richard W.M. Jones rjo...@redhat.com --- Spec URL: http://oirase.annexia.org/reviews/whenjobs/whenjobs.spec SRPM URL: http://oirase.annexia.org/reviews/whenjobs/whenjobs-0.7.3-1.fc17.src.rpm -- You are receiving this mail because: You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 803089] Review Request: whenjobs - Replacement for cron with dependencies
https://bugzilla.redhat.com/show_bug.cgi?id=803089 --- Comment #26 from Richard W.M. Jones rjo...@redhat.com --- Koji scratch build which I guess proves that it builds on 32 bit ... http://koji.fedoraproject.org/koji/taskinfo?taskID=4209135 -- You are receiving this mail because: You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 803089] Review Request: whenjobs - Replacement for cron with dependencies
https://bugzilla.redhat.com/show_bug.cgi?id=803089 --- Comment #27 from Michael Scherer m...@zarb.org --- Yup, this laptop is 32 bits. Let me start again mock and go on with the review. -- You are receiving this mail because: You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 803089] Review Request: whenjobs - Replacement for cron with dependencies
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=803089 --- Comment #21 from Michael Scherer m...@zarb.org 2012-04-17 16:56:23 EDT --- Sorry for not updating this review. While working on the formal review, I noticed that the md5sum is not the same between the spec on people.redhat.com and the rpm : $ md5sum ~/rpmbuild/SOURCES/whenjobs-0.7.2.tar.gz ./whenjobs-0.7.2.tar.gz 8b89aa3eeb02c53ed688edd32546d9bc /home/misc/rpmbuild/SOURCES/whenjobs-0.7.2.tar.gz c201788e584dd63891d11295cf9b5788 ./whenjobs-0.7.2.tar.gz is this normal ? ( there is basically 2 bytes missing, that's rather strange ) -- 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. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 803089] Review Request: whenjobs - Replacement for cron with dependencies
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=803089 --- Comment #20 from Michael Scherer m...@zarb.org 2012-04-07 06:52:41 EDT --- Indeed, i tought it would be used for the %build, but that's used for another thing, sorry. -- 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. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 803089] Review Request: whenjobs - Replacement for cron with dependencies
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=803089 --- Comment #17 from Michael Scherer m...@zarb.org 2012-04-06 15:04:13 EDT --- The %global opt is still unused in the spec, so if the makefile work without it, maybe it should be removed ? -- 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. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 803089] Review Request: whenjobs - Replacement for cron with dependencies
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=803089 --- Comment #18 from Michael Scherer m...@zarb.org 2012-04-06 15:12:24 EDT --- And yes, a new rpm would help me to make sure I can test the build and other stuff correctly. -- 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. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 803089] Review Request: whenjobs - Replacement for cron with dependencies
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=803089 --- Comment #19 from Richard W.M. Jones rjo...@redhat.com 2012-04-06 15:27:33 EDT --- (In reply to comment #17) The %global opt is still unused in the spec, so if the makefile work without it, maybe it should be removed ? %opt is used in the spec. On x86/x86-64 it would always be set to 1, because ocamlopt is always available. However on some weird secondary architectures (I think s/390x is the only one) where there is no native compiler backend, ocamlopt wouldn't exist so %opt would be 0. (In reply to comment #18) And yes, a new rpm would help me to make sure I can test the build and other stuff correctly. This is the package I've been using successfully for a few weeks: Spec URL: http://oirase.annexia.org/reviews/whenjobs/whenjobs.spec SRPM URL: http://oirase.annexia.org/reviews/whenjobs/whenjobs-0.7.2-1.fc17.src.rpm -- 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. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 803089] Review Request: whenjobs - Replacement for cron with dependencies
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=803089 --- Comment #16 from Richard W.M. Jones rjo...@redhat.com 2012-04-02 15:44:53 EDT --- This is working out well for me, after about a week of testing. Do you want me to post another spec file (it'll be the same, just with an updated version number). http://git.annexia.org/?p=whenjobs.git;a=blob;f=whenjobs.spec.in;hb=HEAD -- 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. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 803089] Review Request: whenjobs - Replacement for cron with dependencies
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=803089 --- Comment #15 from Richard W.M. Jones rjo...@redhat.com 2012-03-23 14:19:44 EDT --- I just posted a patch that fixes the Dynlink issue, but I need to test it out locally for a bit first. http://git.annexia.org/?p=whenjobs.git;a=commitdiff;h=de72662854c3db9365296dd45cade2253910be7f -- 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. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 803089] Review Request: whenjobs - Replacement for cron with dependencies
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=803089
--- Comment #9 from Michael Scherer m...@zarb.org 2012-03-22 02:56:56 EDT ---
So :
- The guideline say that you should use %global instead of %define :
https://fedoraproject.org/wiki/Packaging:Guidelines#.25global_preferred_over_.25define
- there is no need nowadays to clean the buildroot before install
https://fedoraproject.org/wiki/Packaging:Guidelines#BuildRoot_tag
- the line
%define opt %(test -x %{_bindir}/ocamlopt echo 1 || echo 0)
is still not used. If you have added directly support in the makefile, then
this line can be removed, no ?
- why are debug disabled ? While I am sure that you have good reasons, it
should be mentioned in a comment ( and that's maybe the reason why you have to
strip by hand the software, as you mention later in the spec )
--
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.
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 803089] Review Request: whenjobs - Replacement for cron with dependencies
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=803089
--- Comment #10 from Richard W.M. Jones rjo...@redhat.com 2012-03-22 04:42:11
EDT ---
(In reply to comment #9)
So :
- The guideline say that you should use %global instead of %define :
https://fedoraproject.org/wiki/Packaging:Guidelines#.25global_preferred_over_.25define
G!
http://git.annexia.org/?p=whenjobs.git;a=commitdiff;h=ea28f8eebc4d8434e7e66d62b769d747656d27ae
- there is no need nowadays to clean the buildroot before install
https://fedoraproject.org/wiki/Packaging:Guidelines#BuildRoot_tag
http://git.annexia.org/?p=whenjobs.git;a=commitdiff;h=a84570d0991652a864d90e8e3fee685cc05982c7
- the line
%define opt %(test -x %{_bindir}/ocamlopt echo 1 || echo 0)
is still not used. If you have added directly support in the makefile, then
this line can be removed, no ?
http://git.annexia.org/?p=whenjobs.git;a=commitdiff;h=2c27f26c486961d1b7799d4f4274aa31d04df845
- why are debug disabled ? While I am sure that you have good reasons, it
should be mentioned in a comment ( and that's maybe the reason why you have to
strip by hand the software, as you mention later in the spec )
The reason is that the OCaml compiler doesn't add DWARF
information to its output, so you end up with debuginfo
files that work but are content-free. This is a standard
thing that is added to every OCaml spec file, eg picking
one at random:
http://pkgs.fedoraproject.org/gitweb/?p=ocaml-pcre.git;a=blob;f=ocaml-pcre.spec;hb=HEAD
I've no idea why stripping didn't happen. It *does*
happen normally, even with debug disabled, so it could
be a bug in my local RPM, but in any case rpmlint warned
about it, and stripping twice won't be a problem.
--
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.
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 803089] Review Request: whenjobs - Replacement for cron with dependencies
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=803089 --- Comment #11 from Michael Scherer m...@zarb.org 2012-03-22 06:04:56 EDT --- Then I think it would be nice to add this fact to the ocaml packaging page ( not sure if there is a official process for that, that's why I didn't added it ) https://fedoraproject.org/wiki/Packaging:OCaml So people reviewing can just see this is normal, that's the policy. -- 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. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 803089] Review Request: whenjobs - Replacement for cron with dependencies
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=803089 --- Comment #12 from Richard W.M. Jones rjo...@redhat.com 2012-03-22 09:09:21 EDT --- It turns out there's a problem with the way I'm using Dynlink in native code (I noticed that whenjobs --upload doesn't work in the native version): https://sympa-roc.inria.fr/wws/arc/caml-list/2012-03/msg00276.html -- 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. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 803089] Review Request: whenjobs - Replacement for cron with dependencies
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=803089 --- Comment #13 from Michael Scherer m...@zarb.org 2012-03-22 11:47:58 EDT --- Fedora-review complaint about the address of the FSF in the various files, for example in whenjobs-0.7.1/lib/flock.c but the address is however correct in others files. ( see the licensecheck tools, from rpmdev ). And so, for the Dynlink issue, should I wait for a new tarball before trying to setup a vm for testing it ? -- 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. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 803089] Review Request: whenjobs - Replacement for cron with dependencies
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=803089 --- Comment #14 from Richard W.M. Jones rjo...@redhat.com 2012-03-22 12:08:39 EDT --- (In reply to comment #13) Fedora-review complaint about the address of the FSF in the various files, for example in whenjobs-0.7.1/lib/flock.c but the address is however correct in others files. ( see the licensecheck tools, from rpmdev ). http://git.annexia.org/?p=whenjobs.git;a=commitdiff;h=602f482985977184bfa2794cfb91b15337035a0a And so, for the Dynlink issue, should I wait for a new tarball before trying to setup a vm for testing it ? Unfortunately the Dynlink issue breaks reloads, so I really need to think about how to resolve this. In the worst case the daemon might need to restart itself; or I could go back to compiling the daemon as bytecode. -- 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. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 803089] Review Request: whenjobs - Replacement for cron with dependencies
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=803089 --- Comment #7 from Richard W.M. Jones rjo...@redhat.com 2012-03-21 11:59:29 EDT --- Spec URL: http://oirase.annexia.org/reviews/whenjobs/whenjobs.spec SRPM URL: http://oirase.annexia.org/reviews/whenjobs/whenjobs-0.7.1-1.src.rpm This updated package is native-compiled (fixing point 7 above). -- 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. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 803089] Review Request: whenjobs - Replacement for cron with dependencies
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=803089 --- Comment #8 from Richard W.M. Jones rjo...@redhat.com 2012-03-21 12:02:19 EDT --- Spec URL: http://oirase.annexia.org/reviews/whenjobs/whenjobs.spec SRPM URL: http://oirase.annexia.org/reviews/whenjobs/whenjobs-0.7.1-1.fc17.src.rpm (Fixed SRPM URL) This package is ready for review. -- 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. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 803089] Review Request: whenjobs - Replacement for cron with dependencies
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=803089 --- Comment #6 from Richard W.M. Jones rjo...@redhat.com 2012-03-19 05:44:57 EDT --- The jobs file is parsed by the OCaml compiler, so whenjobs cannot be used at all in an environment that doesn't have the compiler. Yes please wait -- I want to fix at least the bytecode/native code issue. -- 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. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 803089] Review Request: whenjobs - Replacement for cron with dependencies
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=803089
--- Comment #4 from Richard W.M. Jones rjo...@redhat.com 2012-03-18 05:55:22
EDT ---
(In reply to comment #1)
Let me start the review, but I am not a ocaml specialist.
1) %{_libdir}/whenjobs/ is unowned, you should add %dir %{_libdir}/whenjobs/
in
the %files section
Fixed.
2) given that this requires f17 to be built, I think you can remove %defattr (
unless the plan is to backport to EL 5, but I doubt )
Fixed.
3) same goes for %clean and the rm -Rf $RPMBUILDROOT at the start of %install,
if I am not wrong, so you can remove them ( no need to keep unused cruft )
Fixed.
4) per https://fedoraproject.org/wiki/Packaging/Guidelines#File_Dependencies
,
could you change the requires on /usr/bin/ocamlc to the package name ?
same goes for the buildRequires on perl-doc.
My reading of that section is that dependencies on /usr/bin/* are
permitted. I'd prefer to keep them because the program does in
fact run those binaries, like /usr/bin/ocamlc, so it seems more logical
to depend on the binaries, not the packages (which might change in future).
5) nitpicking, but
BuildRequires: pcre-devel, ocaml-pcre-devel
is better on 2 lines, as this ease review of a potential changes. But that's
not blocking for the review.
Fixed.
6) shouldn't whenjobsd be start at boot, with a systemd file ? ( given your
blog posts and the use case, this sound logical to me, but maybe I missed
something )
No, the daemon is started by each user that requires it.
7) you detect if the bytecode is created or not at the beggining of the spec,
but do not act on it as explained on :
https://fedoraproject.org/wiki/Packaging:OCaml#Bytecode-only_architectures
So either there is something missing, or something not used, or something
magic
I would bet on the 1st, but maybe that's the 3rd
This is actually an upstream problem. I need to add all the
upstream Makefile rules so that whenjobs can be compiled as
native code. It was just easier to use bytecode compilation,
and since it's not very performance-sensitive I didn't bother
much about it. One benefit of fixing this upstream is that we
wouldn't need the prelink stuff (because prelink doesn't break
native-compiled programs).
(In reply to comment #2)
and one last one, the release do not have %{?dist}.
Fixed!
I have just pushed the changes to the upstream git repo for now:
http://git.annexia.org/?p=whenjobs.git;a=commitdiff;h=7e01f4e655c5ceadbb571f4ded19427b41ae2933
since item #7 needs a big amount of upstream work to fix properly.
Hopefully I can get some time to do that this week.
Thanks for the initial review.
--
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.
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 803089] Review Request: whenjobs - Replacement for cron with dependencies
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=803089 --- Comment #5 from Michael Scherer m...@zarb.org 2012-03-18 18:21:42 EDT --- For the rpmlint warning about prelink file, I filled https://bugzilla.redhat.com/show_bug.cgi?id=804452 , since your concern about this not being a configuration file is valid. And have you considered filling a RFE for rpm to not strip ocaml software compiled with -custom ? Regarding point 4, that's indeed valid to have that and I will not force you, but that's not convenient for users ( at least for those with a more contrained environment than you and me, for example, in a small vm where parsing a 35 mo xml file can be problematic ). As I doubt the ocaml compiler will move a lot in the future ( or this would break a lot more software than just whenjobs ), I think you could use directly the package without trouble. But again, it is up to you. Should I wait for newer version of whejobs for the formal review ? -- 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. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 803089] Review Request: whenjobs - Replacement for cron with dependencies
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=803089
Michael Scherer m...@zarb.org changed:
What|Removed |Added
CC||m...@zarb.org
Flag||fedora-review?
--- Comment #1 from Michael Scherer m...@zarb.org 2012-03-17 18:05:16 EDT ---
Let me start the review, but I am not a ocaml specialist.
1) %{_libdir}/whenjobs/ is unowned, you should add %dir %{_libdir}/whenjobs/ in
the %files section
2) given that this requires f17 to be built, I think you can remove %defattr (
unless the plan is to backport to EL 5, but I doubt )
3) same goes for %clean and the rm -Rf $RPMBUILDROOT at the start of %install,
if I am not wrong, so you can remove them ( no need to keep unused cruft )
4) per https://fedoraproject.org/wiki/Packaging/Guidelines#File_Dependencies ,
could you change the requires on /usr/bin/ocamlc to the package name ?
same goes for the buildRequires on perl-doc.
5) nitpicking, but
BuildRequires: pcre-devel, ocaml-pcre-devel
is better on 2 lines, as this ease review of a potential changes. But that's
not blocking for the review.
6) shouldn't whenjobsd be start at boot, with a systemd file ? ( given your
blog posts and the use case, this sound logical to me, but maybe I missed
something )
7) you detect if the bytecode is created or not at the beggining of the spec,
but do not act on it as explained on :
https://fedoraproject.org/wiki/Packaging:OCaml#Bytecode-only_architectures
So either there is something missing, or something not used, or something magic
I would bet on the 1st, but maybe that's the 3rd
--
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.
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 803089] Review Request: whenjobs - Replacement for cron with dependencies
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=803089 Michael Scherer m...@zarb.org changed: What|Removed |Added AssignedTo|nob...@fedoraproject.org|m...@zarb.org -- 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. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 803089] Review Request: whenjobs - Replacement for cron with dependencies
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=803089
--- Comment #2 from Michael Scherer m...@zarb.org 2012-03-17 18:20:35 EDT ---
and one last one, the release do not have %{?dist}.
IIRC, that's not mandatory, but I think that's useful to see the distribution a
rpm was compiled for.
--
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.
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 803089] Review Request: whenjobs - Replacement for cron with dependencies
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=803089 --- Comment #3 from Michael Scherer m...@zarb.org 2012-03-17 18:29:16 EDT --- and regarding the point 6, I looked at the doc and it seems that's not how you want it to be run, so no problem. ( even if I think this would be a worthy feature to add, as if someone start to rely on it for scheduling tasks, having a standard way of starting it would be better ). But that's also not blocking for the review. -- 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. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
