Spec URL: http://people.redhat.com/jsafrane/libcpuset/libcpuset.spec SRPM URL: http://people.redhat.com/jsafrane/libcpuset/libcpuset-1.0-1.fc11.src.rpm Description: Cpusets provide system-wide control of the CPUs on which tasks may execute, and the memory nodes on which they allocate memory. libcpuset provides a C API to leverage cpusets. It uses libbitmask, which goes through review as bug #516779
1/ please change BuildRoot: to %(mktemp -ud %{_tmppath}/%{name}-%{version}-%{release}-XXXXXX) (recommended one in https://fedoraproject.org/wiki/PackagingGuidelines) 2/ remove empty row: #Requires:....... 3/ AUTHORS -file could be add to documentation that`s all
1, 2, 3: done In addition, I moved /usr/share/man/man4/cpuset.4.gz from -devel subpackage to libcpuset.rpm I uploaded new .spec and .srpm to aforementioned location.
Looks ok now.
Not OK: %build autoreconf -i %configure --disable-static --disable-dependency-tracking CFLAGS="$RPM_OPT_FLAGS" make %{?_smp_mflags} 1. running the autotools during builds is a silly idea. 2. %configure already contain CFLAGS. The explicit CFLAGS= is superflous.
(In reply to comment #4) > Not OK: > 1. running the autotools during builds is a silly idea. Surprisingly, upstream provides tarball without ./configure and autoreconf is really needed to create one. > 2. %configure already contain CFLAGS. The explicit CFLAGS= is superflous. Here you are right. I uploaded new .spec and .srpm
(In reply to comment #5) > (In reply to comment #4) > > Not OK: > > 1. running the autotools during builds is a silly idea. > > Surprisingly, upstream provides tarball without ./configure and autoreconf is > really needed to create one. Yes, upstream doesn't package their package properly but ships a crippled package, which doesn't comply to the autotools' working principles. If you want deterministic builts, you're better off running the autotools in advance to building, generate patches and apply them during builds.
Hello, (In reply to comment #4) > %configure --disable-static --disable-dependency-tracking > CFLAGS="$RPM_OPT_FLAGS" > make %{?_smp_mflags} [...] > 2. %configure already contain CFLAGS. The explicit CFLAGS= is superflous. ... and misplaced; CFLAGS should be set *before* configure is called. (In reply to comment #6) > Yes, upstream doesn't package their package properly but ships a [..] > package, which doesn't comply to the autotools' working principles. Correct. Jan, please report that upstream. > If you want deterministic builts, you're better off running the autotools in > advance to building, generate patches and apply them during builds. ... and make sure that the timestamps of all the autotools generated files (no matter whether they chnaged or not) are in the correct order. Otherwise, the autotools rebuild rules are triggered. They do nothing in autotools-free buildroot, but do the rebuild otherwise; that can be confusing. But getting the timestamps right is often quite tricky. That's why I recommend calling autoreconf if the changes to autoconfigury are bigger. I would recommend it in this case as well. Moreover, the rule "do not run autotools" evolved in times when autotools were much less stable than these days, so the risk that things will break with an update was higher.
(In reply to comment #7) > Hello, > > (In reply to comment #4) > > %configure --disable-static --disable-dependency-tracking > > CFLAGS="$RPM_OPT_FLAGS" > > make %{?_smp_mflags} > [...] > > 2. %configure already contain CFLAGS. The explicit CFLAGS= is superflous. > > ... and misplaced; CFLAGS should be set *before* configure is called. Wrong. With modern autoconf (ca. 2.50) CFLAGS should be set after configure.
CFLAGS are not in the .spec anymore, stay on topic please :). Regarding autoreconf... I think it's pretty stable and probability of broken builds is pretty low now, so I am going to stick to it as it is now. Of course, I'll report it upstream.
(In reply to comment #9) > CFLAGS are not in the .spec anymore, stay on topic please :). > > Regarding autoreconf... I think it's pretty stable and probability of broken > builds is pretty low now, It's mere random luck. I guess, you also give a loaded gun to your children to play with? > so I am going to stick to it as it is now. Sufficent reason for me, to recommend other reviewers to not approve this package.
Are you folks interested in continuing? This would be closed if there will be no response soon.
Created attachment 439112 [details] proposed .spec file I am not interested in libcpuset in Fedora anymore, feel free to close the review. I'm attaching current .spec file in case someone wants to reopen the review (and maintain the package afterwards).