Bug 185845 - Review Request: rpld - RPL/RIPL remote boot daemon
Review Request: rpld - RPL/RIPL remote boot daemon
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
All Linux
medium Severity medium
: ---
: ---
Assigned To: Jason Tibbitts
Fedora Package Reviews List
Depends On:
  Show dependency treegraph
Reported: 2006-03-19 12:34 EST by Paul P Komkoff Jr
Modified: 2011-11-28 08:06 EST (History)
2 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Last Closed: 2006-10-24 14:09:58 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
limburgher: fedora‑cvs+

Attachments (Terms of Use)
Replacement patch which doesn't override OPT. (854 bytes, patch)
2006-09-16 11:57 EDT, Jason Tibbitts
no flags Details | Diff

  None (edit)
Description Paul P Komkoff Jr 2006-03-19 12:34:33 EST
Spec Name or Url: http://stingr.net/l/fe/rpld.spec
SRPM Name or Url: http://stingr.net/l/fe/rpld-1.8-1.src.rpm
Daemon to net-boot IBM style RPL boot ROMs (this is not the
same as the Novell IPX-style RPL protocol, despite the
Comment 1 Jason Tibbitts 2006-06-26 21:39:26 EDT
I saw that this had been sitting for a bit and thought I'd take a look, but
there are major problems getting this build.

First off, you need BuildRequires: byacc, flex

That gets things to build to the point where it tries to install rpld as a
different user:

install -c -o root -g kmem -m 555 rpld /var/tmp/rpld-1.8-1-root-mockbuild/usr/sbin

That is never allowable; it should put the file into place and rely on the
%files section to set the ownership.  This should be pretty easy to patch out of
the Makefile.

Finally, the compiler isn't called with the proper flags; you need to pass in
$RPM_OPT_FLAGS somehow.
Comment 2 Paul P Komkoff Jr 2006-08-08 12:52:02 EDT
Sorry for delay, work :(

and http://stingr.net/l/fe/rpld.spec
have been updated accorting to your corrections. Please take a look.
Comment 3 Jason Tibbitts 2006-09-16 11:57:04 EDT
Sorry for taking so long to get back to this.

The compiler is still not being called properly; I'm just seeing:

gcc -O     -c -o protocol.o protocol.c

I was able to get this to work by changing the makefile patch to the one I'll
attach, and duplicating the call to make in %build.  For some reason the package
wants make to be called twice; the second invocation actually builds the
software.  Before, everything was being built in %install.
Comment 4 Jason Tibbitts 2006-09-16 11:57:57 EDT
Created attachment 136435 [details]
Replacement patch which doesn't override OPT.
Comment 5 Till Maas 2006-09-16 12:57:56 EDT
You should use macros here (%{_sbindir} and %{_mandir}):
mkdir -p $RPM_BUILD_ROOT/usr/{sbin,share/man/man{8,5}}
Comment 6 Jason Tibbitts 2006-10-05 21:09:26 EDT
Comment 7 Paul P Komkoff Jr 2006-10-06 15:15:36 EDT
Sorry for the delays.

and http://stingr.net/l/fe/rpld.spec
Comment 8 Jason Tibbitts 2006-10-06 17:48:40 EDT
Thanks.  Builds in mock; rpmlint has this to say:
   W: rpld mixed-use-of-spaces-and-tabs (spaces: line 1, tab: line 10)
Fix if you like.

  W: rpld incoherent-version-in-changelog 1.8-1 1.8-3.fc6
You should make a changelog entry at minimum for each release.  The current
release is 1.8-3 but the last changelog entry is from 1.8-1.

Hmm, not that I look at things, this is a beta release, isn't it?  In that case
you should follow the naming guidelines for prereleases:

So this should be named something like rpmd-1.8-0.1.beta1.  If you revise it, go
to 1.8-0.2.beta1.  When beta2 comes out, then go to 1.8-0.3.beta2.  And when
it's finally released (if it ever is), you can go to 1.8-1.  Basically you can
put anything you want after the second number in the release, even if things
don't sort alphabetically, as long as you keep incrementing the second number.

I'm still not seeing the proper flags being passed to the compiler:
   gcc      -c -o protocol.o protocol.c
   gcc      -c -o rpld.o rpld.c
You're using the patch, but for some reason you need a second make call in the
%build section as the first one doesn't actually build the software.  I just
duplicated the "make" line verbatim.

* source files match upstream:
   08a020c08a466378a99edb88ea98ba35  rpld-1.8-beta-1.tar.gz
X package meets naming guidelines (needs to use prerelease naming conventions).
* specfile is properly named, is cleanly written and uses macros consistently.
* dist tag is present.
* build root is correct.
* license field matches the actual license.
* license is open source-compatible.  License text included in package.
* latest version is being packaged.
* BuildRequires are proper.
X compiler flags are appropriate.
* %clean is present.
* package builds in mock (development, x86_64).
* package installs properly
X debuginfo package is busted.
X rpmlint has valid complaints.
* final provides and requires are sane (no non-glibc requirements).
* %check is not present; no test suite upstream.  It is not possible for me to
test this package.
* no shared libraries are added to the regular linker search paths.
* owns the directories it creates.
* doesn't own any directories it shouldn't.
* no duplicates in %files.
* file permissions are appropriate.
* no scriptlets present.
* code, not content.
* documentation is small, so no -docs subpackage is necessary.
* %docs are not necessary for the proper functioning of the package.
* no headers.
* no pkgconfig files.
* no libtool .la droppings.
Comment 10 Jason Tibbitts 2006-10-15 00:21:25 EDT

The package naming looks good.
The compilation flags are proper.
The debuginfo package looks fine.
rpmlint is quiet.

Comment 11 Paul P Komkoff Jr 2011-11-28 06:28:42 EST
Package Change Request
Package Name: rpld
New Branches: el4 el5 el6
Owners: stingray
Comment 12 Jon Ciesla 2011-11-28 08:06:39 EST
Git done (by process-git-requests).

Note You need to log in before you can comment on or make changes to this bug.