Bug 248857
Summary: | Review Request: schedtool - A tool to query or alter process scheduling policy | ||
---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Adel Gadllah <adel.gadllah> |
Component: | Package Review | Assignee: | Thorsten Leemhuis <fedora> |
Status: | CLOSED NEXTRELEASE | QA Contact: | Fedora Extras Quality Assurance <extras-qa> |
Severity: | medium | Docs Contact: | |
Priority: | low | ||
Version: | rawhide | CC: | fedora-package-review, manuel.wolfshant, notting |
Target Milestone: | --- | Flags: | fedora:
fedora-review+
wtogami: fedora-cvs+ |
Target Release: | --- | ||
Hardware: | All | ||
OS: | Linux | ||
Whiteboard: | |||
Fixed In Version: | Doc Type: | Bug Fix | |
Doc Text: | Story Points: | --- | |
Clone Of: | Environment: | ||
Last Closed: | 2007-08-12 08:25:57 UTC | Type: | --- |
Regression: | --- | Mount Type: | --- |
Documentation: | --- | CRM: | |
Verified Versions: | Category: | --- | |
oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |
Cloudforms Team: | --- | Target Upstream Version: | |
Embargoed: |
Description
Adel Gadllah
2007-07-19 09:51:55 UTC
There are two problems with your package - please use the full URL (http://freequaos.host.sk/schedtool/%{name}-%{version}.tar.bz2 for instance) in the Source tag - RPM_OPT_FLAGS are not used: make CFLAGS="-Os -fomit-frame-pointer -s -pipe -DHAVE_AFFINITY" schedtool make[1]: Entering directory `/builddir/build/BUILD/schedtool-1.2.9' gcc -Os -fomit-frame-pointer -s -pipe -DHAVE_AFFINITY -c -o schedtool.o schedtool.c schedtool.c: In function 'set_process': schedtool.c:450: warning: pointer/integer type mismatch in conditional expression gcc -Os -fomit-frame-pointer -s -pipe -DHAVE_AFFINITY -c -o error.o error.c gcc schedtool.o error.o -o schedtool make[1]: Leaving directory `/builddir/build/BUILD/schedtool-1.2.9' Fixed both: http://tgmweb.at/gadllah/schedtool.spec http://tgmweb.at/gadllah/schedtool-1.2.9-2.fc7.src.rpm Quick glance at http://tgmweb.at/gadllah/schedtool-1.2.9-3.fc7.src.rpm - the rpm_opt_flags problem is still there: Executing(%build): /bin/sh -e /var/tmp/rpm-tmp.28294 + umask 022 + cd /builddir/build/BUILD + cd schedtool-1.2.9 + LANG=C + export LANG + unset DISPLAY + make make CFLAGS="-Os -fomit-frame-pointer -s -pipe -DHAVE_AFFINITY" schedtool make[1]: Entering directory `/builddir/build/BUILD/schedtool-1.2.9' gcc -Os -fomit-frame-pointer -s -pipe -DHAVE_AFFINITY -c -o schedtool.o schedtool.c schedtool.c: In function 'set_process': schedtool.c:450: warning: pointer/integer type mismatch in conditional expression gcc -Os -fomit-frame-pointer -s -pipe -DHAVE_AFFINITY -c -o error.o error.c gcc schedtool.o error.o -o schedtool make[1]: Leaving directory `/builddir/build/BUILD/schedtool-1.2.9' + exit 0 Executing(%install): /bin/sh -e /var/tmp/rpm-tmp.28294 + umask 022 + cd /builddir/build/BUILD + cd schedtool-1.2.9 + LANG=C + export LANG + unset DISPLAY + rm -rf /var/tmp/schedtool-1.2.9-3.fc7-root-mockbuild + make install 'CFLAGS=-O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector --param=ssp-buffer-size=4 -m64 -mtune=generic' D ESTPREFIX=/usr DESTDIR=/var/tmp/schedtool-1.2.9-3.fc7-root-mockbuild make CFLAGS="-O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector --param=ssp-buffer-size=4 -m64 -mtune=generic -DHAVE_AFFIN ITY" schedtool make[1]: Entering directory `/builddir/build/BUILD/schedtool-1.2.9' make[1]: `schedtool' is up to date. solution: you have to pass the RPM_OPT_FLAGS flag at build time, not at install - the timestamp of the files is not preserved. you'd better revert to %doc and remove the docdir line - schedtool-debuginfo is empty fixed package and spec: http://tgmweb.at/gadllah/schedtool.spec http://tgmweb.at/gadllah/schedtool-1.2.9-5.fc7.src.rpm [Un]official package review (due to needsponsor status) coming soon Key: - = N/A x = Check ! = Problem ? = Not evaluated === REQUIRED ITEMS === [x] Package is named according to the Package Naming Guidelines. [x] Spec file name must match the base package %{name}, in the format %{name}.spec. [x] Package meets the Packaging Guidelines. [x] Package successfully compiles and builds into binary rpms on at least one supported architecture. Tested on:devel/x86_64 [x] Rpmlint output: none [x] Package is not relocatable. [x] Buildroot is correct (%{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)) [x] Package is licensed with an open-source compatible license and meets other legal requirements as defined in the legal section of Packaging G uidelines. [x] License field in the package spec file matches the actual license. License type:GPL v2 [x] 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. [x] Spec file is legible and written in American English. [x] Sources used to build the package matches the upstream source, as provided in the spec URL. SHA1SUM of package: ab7743cba970d16ebe6ea8039e4bb57bd26027c8 /home/wolfy/schedtool-1.2.9.tar.bz2 [x] Package is not known to require ExcludeArch, OR: Arches excluded: Why: [-] All build dependencies are listed in BuildRequires, except for any that are listed in the exceptions section of Packaging Guidelines. [-] The spec file handles locales properly. [-] ldconfig called in %post and %postun if required. [x] Package must own all directories that it creates. [-] Package requires other packages for directories it uses. [x] Package does not contain duplicates in %files. [x] Permissions on files are set properly. [x] Package has a %clean section, which contains rm -rf %{buildroot} (or $RPM_BUILD_ROOT). [x] Package consistently uses macros. [x] Package contains code, or permissable content. [-] Large documentation files are in a -doc subpackage, if required. [x] Package uses nothing in %doc for runtime. [-] Header files in -devel subpackage, if present. [-] Static libraries in -devel subpackage, if present. [-] Package requires pkgconfig, if .pc files are present. [-] Development .so files in -devel subpackage, if present. [-] Fully versioned dependency in subpackages, if present. [x] Package does not contain any libtool archives (.la). [-] Package contains a properly installed %{name}.desktop file if it is a GUI application. [x] Package does not own files or directories owned by other packages. === SUGGESTED ITEMS === [x] Latest version is packaged. [x] Package does not include license text files separate from upstream. [-] Description and summary sections in the package spec file contains translations for supported Non-English languages, if available. [x] Reviewer should test that the package builds in mock. Tested on:devel/x86_64 and F7/x86_64 [x] Package should compile and build into binary rpms on all supported architectures. Tested on:devel/x86_64 and F7/x86_64 [x] Package functions as described. [-] Scriptlets must be sane, if used. [-] The placement of pkgconfig(.pc) files are correct. [-] File based requires are sane. === Issues === 1. None === Final Notes === 1. Everything seems OK I would approve this package if the packager would not be in NEEDSPONSOR status. new package and spec: (new upstream release and dropped patch -> merged upstream) http://tgmweb.at/gadllah/schedtool.spec http://tgmweb.at/gadllah/schedtool-1.2.10-1.fc7.src.rpm Package is still OK, builds fine in mock / F7 / x86_64. Adel, I have a suggestion (it's pretty minor, feel free to ignore it): please consider shipping the file CHANGES. I think it could be useful for some of the users. ok, thx for looking at the package again and for your suggestion. I will ship the CHNAGES file too (nice to have and its not much work to include it ;) ) Will upload a new one tomorrow morning or later today. Ok, here is the new one (ship CHANGES too): http://tgmweb.at/gadllah/schedtool.spec http://tgmweb.at/gadllah/schedtool-1.2.10-2.fc7.src.rpm Blocker: * Please s!%{_prefix}/bin/!%{_bindir}/! in %files section Some other notes; please think about them and fix where you agree with them: * the summary starts with "A " -- the rule of tumb iirc is to go without it (e.g. Summary: Tool to foo) * The description starts in lower case; rule of tumb iirc is to start capitalized * Please tell upstream that > Copyright (C) 19yy <name of author> > Gnomovision version 69, Copyright (C) 19yy name of author in LICENSE looks bogus ;-) * is there a specific reasons why you excluded TODO? I'd say it should be shipped -- it's small and doesn't do any harm for those not intersted in it * that DESTPREFIX stuff looks intersting, but well, it seems to be needed... * might be better to not let the Makefile gzip the man page as rpm does this on its own (in case rpm starts to use bz2 or whatever in the long term) Will approve the package and sponser you if you fix the blocker and comment on the other stuff. (In reply to comment #11) > Blocker: > * Please s!%{_prefix}/bin/!%{_bindir}/! in %files section > ok fixed that one. > Some other notes; please think about them and fix where you agree with them: > > * the summary starts with "A " -- the rule of tumb iirc is to go without it > (e.g. Summary: Tool to foo) > ok, fixed > * The description starts in lower case; rule of tumb iirc is to start capitalized > ok changed. > * Please tell upstream that > > > Copyright (C) 19yy <name of author> > > Gnomovision version 69, Copyright (C) 19yy name of author > > in LICENSE looks bogus ;-) > ok mail sent. > * is there a specific reasons why you excluded TODO? I'd say it should be > shipped -- it's small and doesn't do any harm for those not intersted in it > ok shipped now. > * that DESTPREFIX stuff looks intersting, but well, it seems to be needed... > I already contacted upstream (and sent a patch) and they will fix it in the next version. > * might be better to not let the Makefile gzip the man page as rpm does this on > its own (in case rpm starts to use bz2 or whatever in the long term) > this would require patching the makefile... is this really needed? can add a patch to do it if its prefferd that way. > Will approve the package and sponser you if you fix the blocker and comment on > the other stuff. ok, thx here is the new spec and srpm: http://tgmweb.at/gadllah/schedtool.spec http://tgmweb.at/gadllah/schedtool-1.2.10-2.fc7.src.rpm Correct URL is: http://tgmweb.at/gadllah/schedtool-1.2.10-3.fc7.src.rpm sorry for the typo (In reply to comment #12) > > * might be better to not let the Makefile gzip the man page as rpm does this on > > its own (in case rpm starts to use bz2 or whatever in the long term) > this would require patching the makefile... is this really needed? No, likely not; I just wanted to make sure you know about it and keep it in mind. > http://tgmweb.at/gadllah/schedtool-1.2.10-3.fc7.src.rpm Approved. Apply for cvs_extras in the accounts system and I'll sponsor you.
> Apply for cvs_extras in the accounts system and I'll sponsor you.
done.
New Package CVS Request ======================= Package Name: schedtool Short Description: Tool to query or alter process scheduling policy Owners: adel.gadllah Branches: FC-6 F-7 |