This doesn't build on Rawhide

if 
gcc -DHAVE_CONFIG_H -I. -I. -I../..  -I./../../common/src -D_GNU_SOURCE -I./../../libfreeipmi/include   -Wall -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector --param=ssp-buffer-size=4 -m32 -march=i386 -mtune=generic -fasynchronous-unwind-tables -MT 
bmc-config-utils.o -MD -MP -MF ".deps/bmc-config-utils.Tpo" -c -o 
bmc-config-utils.o bmc-config-utils.c; \
        then mv -f ".deps/bmc-config-utils.Tpo" ".deps/bmc-config-utils.Po"; 
else rm -f ".deps/bmc-config-utils.Tpo"; exit 1; fi
In function 'open',
    inlined from 'bmc_config_args_validate' at bmc-config-argp.c:326:
/usr/include/bits/fcntl2.h:51: error: call to '__open_missing_mode' declared 
with attribute error: open with O_CREAT in second argument needs 3 arguments
make[3]: *** [bmc-config-argp.o] Error 1

Relationship to OpenIPMI?

FreeIPMI is the userland only implementation of IPMI, no relation to OpenIPMI
except that both are IPMI implementations.

Read ya, Phil

And FreeIPMI contains several features of IPMI that OpenIPMI doesn't provide.

Read ya, Phil

MUSTFIX:
# /etc/init.d/freeipmi-bmc-watchdog start
prints out the bmc-watchdog help instead of starting it.

a koji build attempt failed: 
http://koji.fedoraproject.org/koji/taskinfo?taskID=228149 
gcc -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector --param=ssp-buffer-size=4 -m64 -mminimal-toc -o .libs/bmc-watchdog 
bmc_watchdog-bmc-watchdog.o  ../../libfreeipmi/src/.libs/libfreeipmi.so -lgcrypt -lm 
../../libfreeipmi/src/.libs/libfreeipmi.so: undefined reference to `inb'
../../libfreeipmi/src/.libs/libfreeipmi.so: undefined reference to `outb'
../../libfreeipmi/src/.libs/libfreeipmi.so: undefined reference to `iopl'

The spec file and the package looks good now as all verbal feedback has been 
added. So this package will be approved when the two remaining issues have 
been fixed.

First issue fixed.

Second one seems to that sys/io.h is missing on ppc64 from glibc-headers,
opening a bug against glibc and using an ExludeArch: ppc64 ppc  until this is
cleared up.

Read ya, Phil

PS: Fixed several other minor issues as well in the last version (removed static
libs and added several needed Requires(post|preun) among others).

Added new glibc bug as depends to this bugzilla.

Read ya, Phil

review complete

because the header is missing from ppc64  is no reason to exclude ppc also.  

can you provide links to newer srpms ?

According to Jakubs comment here
https://bugzilla.redhat.com/show_bug.cgi?id=368541#c2 i've now added the
following line to the specfile with comment and reference:

ExclusiveArch: %{ix86} x86_64 ia64 alpha

Newest specfile and SRPM have just been uploaded.

Read ya, Phil

again post links to updated spec and srpm  you must bump release for each 
revision. even during review.

Can you please reference this in the ReviewGuidelines? I read through them and
didn't find any mention of this there.

It's not a problem to do so, but i wasn't aware that this is a MUST requirement.

Thanks,

Read ya, Phil

New Package CVS Request
=======================
Package Name: freeipmi
Short Description: Free IPMI userland implementation
Owners: pknirsch
Branches: F-8
InitialCC:
Cvsextras Commits: yes

Im not satisfied this has received a proper review

Can i get an answer to my question from comment #12 please?

Thanks,

Read ya, Phil

rpmlint output of current packages:

[pknirsch@hamburg rpm]$ ls /home/devel/pknirsch/rpm/rpms/i386/
freeipmi-0.4.6-1.fc7.i386.rpm
freeipmi-bmc-watchdog-0.4.6-1.fc7.i386.rpm
freeipmi-debuginfo-0.4.6-1.fc7.i386.rpm
freeipmi-devel-0.4.6-1.fc7.i386.rpm
freeipmi-ipmidetectd-0.4.6-1.fc7.i386.rpm
freeipmi-ipmimonitoring-0.4.6-1.fc7.i386.rpm
[pknirsch@hamburg rpm]$ rpmlint -i /home/devel/pknirsch/rpm/rpms/i386/*
[pknirsch@hamburg rpm]$ 

So no warning nor error from rpmlint.

Read ya, Phil

Ok, here's the list of what I've checked, although this list isn't required by 
the ReviewGuidelines, it just has to be followed (which I did):
- rpmlint: freeipmi.i386: W: incoherent-version-in-changelog 0.4.6-1.fc7 
0.4.6-1
  I think this one can be neglected, rpmlint should ignore dist tags
- package follows package nameing guidelines
- spec file matches base package name
- Licenses of all files have been checked
- no pre-built binaries included
- package follows FHS
- changelog looks sane
- spec file tags are ok
- buildroot is ok
- buildroot cleaned before install, spec hat a %clean section
- Buildrequirements are ok, no regressions between builds in mock and in a 
full environment, configure output has been checked
- Requires post/preun have been added for programs used in scriptlets
- static libraries have been disabled
- check-rpaths doesn't complain
- config files are noreplace
- initscripts aren't config files
- file permissions look sane
- %makeinstall isn't used
- tarball sha1sum matches upstream's tarball
- smp flags are used for build
- package owns oll directories it creates or requires other packages who own 
them
- package build in mock and in koji, spec file has an ExclusiveArch because of
  glibc not supporting some required functions on s390x, sparc, ppc* with a 
comment pointing at a bugzilla for the reasons
- freeipmi packages don't have any localizes stuff
- packages run ldconfig in their scriptlets
- no duplicate files in the files sections
- header files are in a -devel subpackage
- packages don't have any .la or pkgconfig files

Btw: it would have been faster if you actually would have taken a quick look 
at the spec file instead of simply rejecting it. Be assured that I didn't 
simply waive it just because Phil happens to sit at the other side of my desk. 
He really had to fix quite a few issues.

My fedora-review+ still stands and if you have any issues with that, take it 
to FESCO instead of mistrusting my review of this package

my main concern was that we have no idea what your issues were and how they 
were fixed because they were not documented properly 

disttag values should not be put into changelogs 

CVS-Done

Ok, removing distag from changelog then for the final package.

Thanks Karsten and Dennis.

Read ya, Phil