For some reason the FE-NEW blocker was removed from this ticket.  It will never
get reviewed if it doesn't show up in the FE-NEW list, so I'll re-add it.

I removed the FE-NEW blocker because I didn't see the link between ELSA and the
BUG #163776. As nobody reviewed ELSA I thought that it was due to the blocker. I
was wrong and thank you for re-adding it.

1). 
Source0:        elsa-1.3.0.tar.bz2

You should use %{version}-%{release} instead of hard-coded numbers

2).
%build
make %{?_smp_mflags} USER_INCLUDES=-I%{cn_proc_dir}

You are not using %{optflags} which can provide extra compile flags for Fedora
packages. 

To pass them along you can use the CFLAGS env variable, as so:

CFLAGS="%{optflags}"  make %{?_smp_mflags} USER_INCLUDES=-I%{cn_proc_dir}

3)

/bin/*
/sbin/*
/usr/share/man/man1/*

Should be:
%{_bindir}/<files or globs>
%{_sbindir)/<files or globs>
${_mandir}/man1/elsa*

4)
%doc -P ChangeLog COPYING README HOWTO utils/test_elsa_installation.sh

Is the .sh file executable in the source tar ball?

5).
if [ ! -f %{cn_proc_file} ]; then
  echo " ERROR: %{cn_proc_file} not found"
  echo " EXIT "
  exit 1
fi

Is this check needed?  You have:
BuildRequires:  kernel-devel >= 2.6.15

which will provide that package and that header file.
Consequently, these will not be required:

%define kversion `uname -r`
%define cn_proc_dir /lib/modules/%{kversion}/build/include
%define cn_proc_file %{cn_proc_dir}/linux/cn_proc.h

Also, the 'uname -r' is not a good check - as the build machines
does not have to be neccesarily the same version as what the Fedora release
kernel would have. (FC6 is actually compiled on RHEL4 machines)

And lastly this  "USER_INCLUDES=-I%{cn_proc_dir}" wouldn't be needed either.

Let me find out from other third-party spec files what the correct way of
finding kernel header files is.

(In reply to comment #3)
> 1). 
> Source0:        elsa-1.3.0.tar.bz2
> 
> You should use %{version}-%{release} instead of hard-coded numbers

Done

> 2).
> To pass them along you can use the CFLAGS env variable, as so:
> 
> CFLAGS="%{optflags}"  make %{?_smp_mflags} USER_INCLUDES=-I%{cn_proc_dir}

Done.

> 3)
> 
> /bin/*
> /sbin/*
> /usr/share/man/man1/*
> 
> Should be:
> %{_bindir}/<files or globs>
> %{_sbindir)/<files or globs>
> ${_mandir}/man1/elsa*

Replaced. So now elsa and jobmng are installed in the '/usr/bin' directory and
jobd is installed in the '/usr/sbin' directory.


> 4)
> %doc -P ChangeLog COPYING README HOWTO utils/test_elsa_installation.sh
> 
> Is the .sh file executable in the source tar ball?

Yes it is. 

> 5).
> if [ ! -f %{cn_proc_file} ]; then
>   echo " ERROR: %{cn_proc_file} not found"
>   echo " EXIT "
>   exit 1
> fi
> 
> Is this check needed?  You have:
> BuildRequires:  kernel-devel >= 2.6.15

Yes, you're right. I removed this test.

> %define kversion `uname -r`
> %define cn_proc_dir /lib/modules/%{kversion}/build/include
> %define cn_proc_file %{cn_proc_dir}/linux/cn_proc.h

I replaced this one:

%define kversion `rpm -q kernel-devel | cut --delimiter='-' -f 3,4` 

and thus, in %build stage I have:

CFLAGS="%{optflags}" make %{?_smp_mflags}  \
USER_INCLUDES=-I/usr/src/kernels/%{kversion}-%{_host_cpu}/include

I'm not sure if this is the right way to proceed?

Spec URL and SRPM URL are the same so to check changes you can still use them.
Thank you very much for all your comments,

Guillaume

> 
> CFLAGS="%{optflags}" make %{?_smp_mflags}  \
> USER_INCLUDES=-I/usr/src/kernels/%{kversion}-%{_host_cpu}/include
> 
> I'm not sure if this is the right way to proceed?
> 
I am not sure either, let me ask one of the engineers who is a guru at this.

I am putting Jon Masters on the CC. Jon, please take a look at #comment #5. Your
guidance in this would be much appreciated.

Guillaume,

I tried to build your package on FC5, and I get this:
++ cut --delimiter=- -f 3,4
+ CFLAGS='-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'
+ make -j3 USER_INCLUDES=-I/usr/src/kernels/2.6.15-1.2054_FC5
2.6.17-1.2187_FC5-i686/include
make: *** No rule to make target `2.6.17-1.2187_FC5-i686/include'.  Stop.
error: Bad exit status from /var/tmp/rpm-tmp.7878 (%build)


RPM build errors:
    Bad exit status from /var/tmp/rpm-tmp.7878 (%build)

The build machine apparently has several devel packages so the value in kversion
has whitespace in it. Here are the relevant .spec snippets:

%define kversion `rpm -q kernel-devel | cut --delimiter='-' -f 3,4`

...

%build
CFLAGS="%{optflags}" make %{?_smp_mflags}
USER_INCLUDES=-I/usr/src/kernels/%{kversion}-%{_host_cpu}/include

As a quick fix you could try to replace kversion's define with:
%define kversion `rpm -q kernel-devel | head -n 1 | cut --delimiter='-' -f 3,4`

The question is if there's a better way to select which kernel devel RPM to use.

(In reply to comment #7)
> The build machine apparently has several devel packages so the value in kversion
> has whitespace in it. Here are the relevant .spec snippets:
-----8<-----

I didn't word the original reply correctly -- I'm *guessing* that this is the
problem.

Matt,

That was the problem. It is further aggrevated by the fact that I have lots of
kernels in that directory. A clean build machine shouldn't have this problem as
it installs all the RPMs in a chroot-ed location (for more details refer to mock
RPM).

I asked Jon Masters to take a look at this BZ, since he has a macro that
determines the latest kernel and sets the kernel path in an environment.

Build on FC6 build system failed, with this error:
Installing /builddir/build/SRPMS/elsa-1.3.0-2.fc6.src.rpm
Building target platforms: i386
Building for target i386
Executing(%prep): /bin/sh -e /var/tmp/rpm-tmp.1379
+ umask 022
+ cd /builddir/build/BUILD
+ LANG=C
+ export LANG
+ unset DISPLAY
+ cd /builddir/build/BUILD
+ rm -rf elsa-1.3.0
+ /usr/bin/bzip2 -dc /builddir/build/SOURCES/elsa-1.3.0.tar.bz2
+ tar -xf -
+ STATUS=0
+ '[' 0 -ne 0 ']'
+ cd elsa-1.3.0
++ /usr/bin/id -u
+ '[' 250 = 0 ']'
++ /usr/bin/id -u
+ '[' 250 = 0 ']'
+ /bin/chmod -Rf a+rX,u+w,g-w,o-w .
+ exit 0
Executing(%build): /bin/sh -e /var/tmp/rpm-tmp.1379
+ umask 022
+ cd /builddir/build/BUILD
+ cd elsa-1.3.0
+ LANG=C
+ export LANG
+ unset DISPLAY
++ rpm -q kernel-devel
++ head -n 1
++ cut --delimiter=- -f 3,4
+ CFLAGS='-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'
+ make -j4 USER_INCLUDES=-I/usr/src/kernels/2.6.18-1.2798.fc6-i686/include
make -C job_daemon USER_INCLUDES=-I/usr/src/kernels/2.6.18-1.2798.fc6-i686/include
make[1]: Entering directory `/builddir/build/BUILD/elsa-1.3.0/job_daemon'
gcc -g -O2 -Wall -I/usr/include/glib-2.0 -I/usr/lib/glib-2.0/include  -L/lib
-lglib-2.0   -I/usr/src/kernels/2.6.18-1.2798.fc6-i686/include   jobd.c   -o jobd
jobd.c: In function 'cn_fork_listen':
jobd.c:96: error: invalid application of 'sizeof' to incomplete type 'struct
cn_msg' 
jobd.c:104: error: dereferencing pointer to incomplete type
jobd.c:104: error: 'CN_IDX_PROC' undeclared (first use in this function)
jobd.c:104: error: (Each undeclared identifier is reported only once
jobd.c:104: error: for each function it appears in.)
jobd.c:105: error: dereferencing pointer to incomplete type
jobd.c:105: error: 'CN_VAL_PROC' undeclared (first use in this function)
jobd.c:106: error: dereferencing pointer to incomplete type
jobd.c:107: error: dereferencing pointer to incomplete type
jobd.c:108: error: dereferencing pointer to incomplete type
jobd.c:109: error: dereferencing pointer to incomplete type
jobd.c: In function 'cn_fork_ignore':
jobd.c:129: error: invalid application of 'sizeof' to incomplete type 'struct
cn_msg' 
jobd.c:137: error: dereferencing pointer to incomplete type
jobd.c:137: error: 'CN_IDX_PROC' undeclared (first use in this function)
jobd.c:138: error: dereferencing pointer to incomplete type
jobd.c:138: error: 'CN_VAL_PROC' undeclared (first use in this function)
jobd.c:139: error: dereferencing pointer to incomplete type
jobd.c:140: error: dereferencing pointer to incomplete type
jobd.c:141: error: dereferencing pointer to incomplete type
jobd.c:142: error: dereferencing pointer to incomplete type
jobd.c: In function 'recv_sk_nl':
jobd.c:596: error: dereferencing pointer to incomplete type
jobd.c: In function 'main':
jobd.c:768: error: 'CN_IDX_PROC' undeclared (first use in this function)
make[1]: *** [jobd] Error 1
make[1]: Leaving directory `/builddir/build/BUILD/elsa-1.3.0/job_daemon'
make: *** [all] Error 2
error: Bad exit status from /var/tmp/rpm-tmp.1379 (%build)


RPM build errors:
    Bad exit status from /var/tmp/rpm-tmp.1379 (%build)

On FC5 build:
Installing /builddir/build/SRPMS/elsa-1.3.0-2.fc5.src.rpm
Building target platforms: x86_64
Building for target x86_64
Executing(%prep): /bin/sh -e /var/tmp/rpm-tmp.86186
+ umask 022
+ cd /builddir/build/BUILD
+ LANG=C
+ export LANG
+ unset DISPLAY
+ cd /builddir/build/BUILD
+ rm -rf elsa-1.3.0
+ /usr/bin/bzip2 -dc /builddir/build/SOURCES/elsa-1.3.0.tar.bz2
+ tar -xf -
+ STATUS=0
+ '[' 0 -ne 0 ']'
+ cd elsa-1.3.0
++ /usr/bin/id -u
+ '[' 250 = 0 ']'
++ /usr/bin/id -u
+ '[' 250 = 0 ']'
+ /bin/chmod -Rf a+rX,u+w,g-w,o-w .
+ exit 0
Executing(%build): /bin/sh -e /var/tmp/rpm-tmp.86186
+ umask 022
+ cd /builddir/build/BUILD
+ cd elsa-1.3.0
+ LANG=C
+ export LANG
+ unset DISPLAY
++ rpm -q kernel-devel
++ head -n 1
++ cut --delimiter=- -f 3,4
+ CFLAGS='-O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions
-fstack-protector --param=ssp-buffer-size=4 -m64 -mtune=generic'
+ make -j4 USER_INCLUDES=-I/usr/src/kernels/2.6.18-1.2200.fc5-x86_64/include
make -C job_daemon USER_INCLUDES=-I/usr/src/kernels/2.6.18-1.2200.fc5-x86_64/include
make[1]: Entering directory `/builddir/build/BUILD/elsa-1.3.0/job_daemon'
gcc -g -O2 -Wall -I/usr/include/glib-2.0 -I/usr/lib64/glib-2.0/include 
-lglib-2.0   -I/usr/src/kernels/2.6.18-1.2200.fc5-x86_64/include   jobd.c   -o jobd
jobd.c: In function 'cn_fork_listen':
jobd.c:96: error: invalid application of 'sizeof' to incomplete type 'struct
cn_msg' 
jobd.c:104: error: dereferencing pointer to incomplete type
jobd.c:104: error: 'CN_IDX_PROC' undeclared (first use in this function)
jobd.c:104: error: (Each undeclared identifier is reported only once
jobd.c:104: error: for each function it appears in.)
jobd.c:105: error: dereferencing pointer to incomplete type
jobd.c:105: error: 'CN_VAL_PROC' undeclared (first use in this function)
jobd.c:106: error: dereferencing pointer to incomplete type
jobd.c:107: error: dereferencing pointer to incomplete type
jobd.c:108: error: dereferencing pointer to incomplete type
jobd.c:109: error: dereferencing pointer to incomplete type
jobd.c: In function 'cn_fork_ignore':
jobd.c:129: error: invalid application of 'sizeof' to incomplete type 'struct
cn_msg' 
jobd.c:137: error: dereferencing pointer to incomplete type
jobd.c:137: error: 'CN_IDX_PROC' undeclared (first use in this function)
jobd.c:138: error: dereferencing pointer to incomplete type
jobd.c:138: error: 'CN_VAL_PROC' undeclared (first use in this function)
jobd.c:139: error: dereferencing pointer to incomplete type
jobd.c:140: error: dereferencing pointer to incomplete type
jobd.c:141: error: dereferencing pointer to incomplete type
jobd.c:142: error: dereferencing pointer to incomplete type
jobd.c: In function 'recv_sk_nl':
jobd.c:596: error: dereferencing pointer to incomplete type
jobd.c: In function 'main':
jobd.c:768: error: 'CN_IDX_PROC' undeclared (first use in this function)
make[1]: *** [jobd] Error 1
make[1]: Leaving directory `/builddir/build/BUILD/elsa-1.3.0/job_daemon'
make: *** [all] Error 2
error: Bad exit status from /var/tmp/rpm-tmp.86186 (%build)


RPM build errors:
    Bad exit status from /var/tmp/rpm-tmp.86186 (%build)

Isn't it inappropriate to use kernel-devel for building userland things, and the
public userlan stuff in kernel-headers should be used instead?  Eg.

$ rpm -qf /usr/include/linux/cn_proc.h
kernel-headers-2.6.18-1.2798.fc6.x86_64

Ville,

I had no idea that such package even existed. And you are correct:

[konrad@dyn448114 ~]$ rpm -qi kernel-devel | tail -2
This package provides kernel headers and makefiles sufficient to build modules
against the kernel package.

So I tried to install the kernel-headers and no luck on FC5.

That package is one the FC6 kernels though. Time to upgrade to FC6 for me :-)

I replaced the kernel-devel dependancy by kernel-headers in the elsa.spec file.
I also added the quick fix to define kversion. 

I also fixed the compilation problem by adding the <linux/connector.h> header in
jobd.c. 

The new URLs are:

Spec URL: http://www.bullopensource.org/elsa-srpm/elsa.spec
SRPM URL: http://www.bullopensource.org/elsa-srpm/elsa-1.3.0-3.src.rpm

Excellent. Your package ran through the build system without any trouble.

Josh,

The package is in good shape. Builds with internal Red Hat build system on FC6.
The spec file looks good and problems with it have been fixed. 

What is your thoguht? Is it ready to go in CVS? 

Correction:

MUSTFIX:
 - md5sum don't match. Teh one in the SRPM is different from the one on SF:
->SF:
bab9a2b966f6a997dac717b773ab1507  elsa-1.3.0.tar.bz2
->SRPM:
af98c61eab62c7be0d2b47c1c9d4b0c2

 - also, the version-release-build-release is not used. For example, you have
  -1.3.0-3. Why not use 1.3-3 ?

You're right for the version-release-build-release. The new package is now
available from SF and the md5sum should match now. So, the new version is
elsa-1.4.tar.bz2.

Spec URL: http://www.bullopensource.org/elsa-srpm/elsa.spec
SRPM URL: http://prdownloads.sourceforge.net/elsa/elsa-1.4-1.src.rpm?download

It seems that there is a problem with sourceforge. The size of the SRPM package
and source file are 0Kb. Maybe it's just because files were recently released.

If it doesn't work, here are alternative URL to download files:

SRPM URL: http://www.bullopensource.org/elsa-srpm/elsa-1.4-1.src.rpm
SOURCE URL: http://www.bullopensource.org/elsa-srpm/elsa-1.4.tar.bz2

Good:
 - MD5 sums match ( from bullopensource.org and from SF),
 - Builds on all platforms (brew using FC6),
 - Good usage of version and release tags,
 - License (COPYING) is the same in source code,
 - No compile warnings,
 - No rpmlint errors and warnings,
 - Documentation include.
 - Clear english in spec file.
 - Consistent use of macros.

Needs work:
 - NEWS file, add a reference about release 1.4 and being submitted for FC6
 - Take out from the TODO, the reference to webmin since that is a part of
another package (from reading the ChangeLog)

Do these and it is ready. 

I modified the NEWS file and removed the TODO file. There was only one entry in
the TODO list and it was about the webmin part so I removed the file.

SRPM URL: http://prdownloads.sourceforge.net/elsa/elsa-1.4-2.src.rpm?download
SOURCE URL: http://prdownloads.sourceforge.net/elsa/elsa-1.4-2.tar.bz2?download

Josh,

I have completed my review. Throwing over to you :-)

(In reply to comment #22)
> Josh,
> 
> I have completed my review. Throwing over to you :-)

Sorry for such a delay.  Things around here have been a bit hectic.  I'll try
and take a look at this tomorrow or Monday and get the ball rolling again.

This looks good.  Guillaume, have you applied for access to the Extras CVS?

(In reply to comment #24)
> This looks good.  Guillaume, have you applied for access to the Extras CVS?

Excellent. I will apply for access to Extras CVS today.

You should have access now with me as your sponsor.  Moving to FE-ACCEPT.  Have
fun and thanks for packaging for Fedora!

Yes I have access, thanks Josh. 

Thank you very much everybody for your help.

Guillaume,

If the package is in Fedora Core, can you close this BZ as MODIFIED?

I think that ELSA is not in Fedora Core. When I look component in Fedora Core
from https://bugzilla.redhat.com/bugzilla/request.cgi, the package is not
present, it is still in Fedora Extras.

So should I let the BZ as NEW?

Uh-uh. I have no idea what the next step is. Josh: Help!?

This review was for Extras, so it obviously won't be in Core.  And with Fedora
7, there really isn't a Core/Extras distinction anymore.

All that needs to be done here is to build the package with 'make tag build' in
package directory and close this bug as Resolved: RAWHIDE

So(In reply to comment #31)

> 
> All that needs to be done here is to build the package with 'make tag build' in
> package directory and close this bug as Resolved: RAWHIDE
> 

Who has that privilage to do that?

(In reply to comment #32)
> So(In reply to comment #31)
> 
> > 
> > All that needs to be done here is to build the package with 'make tag build' in
> > package directory and close this bug as Resolved: RAWHIDE
> > 
> 
> Who has that privilage to do that?

Guillaume does.  If both of you would like, I could add Konrad as a co-maintainer.

Oops, sorry for the delay... I will build the package with 'make tag build' and
close the bug. If Konrad agree to be a co-maintainer it's ok for me with great
pleasure.

(In reply to comment #34)
> Oops, sorry for the delay... I will build the package with 'make tag build' and
> close the bug. If Konrad agree to be a co-maintainer it's ok for me with great
> pleasure.

Wooho! err, The answer is 'Yes'. Thank you :-)

I tried a 'make tag build' but it failed. I got yhe following error:

guill@fermat:~/tmp/elsa/devel$ make tag build
cvs tag  -c elsa-1_4-2_fc8
ERROR: Tag elsa-1_4-2_fc8 has been already created.
The following tags have been created so far
elsa-1_4-2:devel:thouveng:1169564002
elsa-1_4-2_fc8:devel:thouveng:1180267354
cvs tag: Pre-tag check failed
cvs [tag aborted]: correct the above errors first!
make: *** [tag] Error 1

Any idea?

Guillaume: Did you use cvs-import.sh to import your package? If so, it already
tagged it, so all you need to do is: 'make build' and it should build it for
you. ;) Can you try that?

I have some problems with koji. I tried the 'make build' and here is the problem:
guill@fermat:~/tmp/elsa/devel$ make build 
<type 'exceptions.AttributeError'>: 'Krb5Error' object has no attribute 'err_code'
make: *** [koji] Error 1

koji doesn't work at all. All common use failed. For example:

guill@fermat:~/tmp/elsa/devel$ koji list-tasks      
<type 'exceptions.AttributeError'>: 'Krb5Error' object has no attribute 'err_code'

I have installed Fedora 7

See: http://fedoraproject.org/wiki/Koji/UsingKoji

In particular you need to run: 

/usr/bin/fedora-packager-setup.sh

Thank you very much Kevin. It works better now ;)
So the build is done
http://koji.fedoraproject.org/koji/buildinfo?buildID=8152

I close this bug as Resolved: RAWHIDE