Bug 429450

I've got two issues here
a) mock build fails with:

Executing(%build): /bin/sh -e /var/tmp/rpm-tmp.26053
+ umask 022
+ cd /builddir/build/BUILD
+ cd gstream16
+ LANG=C
+ export LANG
+ unset DISPLAY
+ make -j3 -f Makefile.unx MAKEDOC=/usr/bin/allegro-makedoc 'OFLAGS=-O2 -g -pipe
-Wall -Wp,-D_FORTIFY_SOURCE=2 -fexception
s -fstack-protector --param=ssp-buffer-size=4 -m64 -mtune=generic -fPIC'
Missing the makedoc utility from Allegro! Please copy the makedoc executable
from allegro/docs to this directory.
+ rm test.o
rm: cannot remove `test.o': No such file or directory
error: Bad exit status from /var/tmp/rpm-tmp.26053 (%build)

This happens despite having allegro-devel installed.

b) I've noticed that you pass some custom flags to gcc when building
libgstrm.so.0. Due to the failed build, I did not reach this point so I could
not see what flags are actually used, so I figured I'd better ask, just in case:
shouldn't RPM_OPT_FLAGS be used here, too ? 

(In reply to comment #1)

Thanks for taking a look at this!

> I've got two issues here
> a) mock build fails with:
> 
> Executing(%build): /bin/sh -e /var/tmp/rpm-tmp.26053
> + umask 022
> + cd /builddir/build/BUILD
> + cd gstream16
> + LANG=C
> + export LANG
> + unset DISPLAY
> + make -j3 -f Makefile.unx MAKEDOC=/usr/bin/allegro-makedoc 'OFLAGS=-O2 -g -pipe
> -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexception
> s -fstack-protector --param=ssp-buffer-size=4 -m64 -mtune=generic -fPIC'
> Missing the makedoc utility from Allegro! Please copy the makedoc executable
> from allegro/docs to this directory.
> + rm test.o
> rm: cannot remove `test.o': No such file or directory
> error: Bad exit status from /var/tmp/rpm-tmp.26053 (%build)
> 
> This happens despite having allegro-devel installed.
> 

Erm, oops, my bad, allegro-devel was "missing" makedoc (not install by make
install, but appearantly needed by some apps). Since I'm also the allegro
maintainer I fixed this, but I never committed let alone build my changes (other
then a local build).

I'm currently working on fixing allegro <-> pulseaudio interaction, when I'm
done with that I'll push a new allegro to devel, F-8 and F-7 which does include
makedoc (which I've name allegro-makedoc as makedoc is a bit generic).

I'll add a note here once the new allegro is done.

> b) I've noticed that you pass some custom flags to gcc when building
> libgstrm.so.0. Due to the failed build, I did not reach this point so I could
> not see what flags are actually used, so I figured I'd better ask, just in case:
> shouldn't RPM_OPT_FLAGS be used here, too ? 

Yes, you're right I'll fix this after a full review is done, as I would like to
keep the number of iterations (== work for both of us) to a minimum.

>I'm currently working on fixing allegro <-> pulseaudio interaction, 
>when I'm done with that I'll push a new allegro [...]
> which does include makedoc (which I've name allegro-makedoc as makedoc is a
bit generic).
>I'll add a note here once the new allegro is done.

Excellent approach. I'll be here.


>Yes, you're right I'll fix this after a full review is done, as I would like
> to keep the number of iterations (== work for both of us) to a minimum.

Which is exactly why I've asked now and not after solving the makedoc issue :)

The new allegro is ready, no idea when it will hit the repo though:

http://koji.fedoraproject.org/koji/taskinfo?taskID=365007
https://admin.fedoraproject.org/updates/F8/pending/allegro-4.2.2-7.fc8
https://admin.fedoraproject.org/updates/F7/pending/allegro-4.2.2-7.fc7

Something is fishy here:
Executing(%build): /bin/sh -e /var/tmp/rpm-tmp.91582
+ umask 022
+ cd /builddir/build/BUILD
+ cd gstream16
+ LANG=C
+ export LANG
+ unset DISPLAY
+ make -j3 -f Makefile.unx MAKEDOC=/usr/bin/allegro-makedoc 'OFLAGS=-O2 -g -pipe
-Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-
protector --param=ssp-buffer-size=4 -m64 -mtune=generic -fPIC'
c++    -Wall -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions
-fstack-protector --param=ssp-buffer-size=4 -m64 -mtune=generic
 -fPIC -c gsintfac.cc
c++    -Wall -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions
-fstack-protector --param=ssp-buffer-size=4 -m64 -mtune=generic
 -fPIC -c gsvirtul.cc
c++    -Wall -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions
-fstack-protector --param=ssp-buffer-size=4 -m64 -mtune=generic
 -fPIC -c gsmanip.cc
c++    -Wall -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions
-fstack-protector --param=ssp-buffer-size=4 -m64 -mtune=generic
 -fPIC -c gswrirea.cc
c++    -Wall -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions
-fstack-protector --param=ssp-buffer-size=4 -m64 -mtune=generic
 -fPIC -c gsfunc.cc
c++    -Wall -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions
-fstack-protector --param=ssp-buffer-size=4 -m64 -mtune=generic
 -fPIC -c test.cc
/usr/bin/allegro-makedoc gstream._tx
makeinfo --no-split gstream.txi
make: makeinfo: Command not found
make: *** [gstream.info] Error 127
make: *** Waiting for unfinished jobs....
test.cc: In function 'int main()':
test.cc:36: warning: 'int text_mode(int)' is deprecated (declared at
/usr/include/allegro/alcompat.h:155)
test.cc:36: warning: 'int text_mode(int)' is deprecated (declared at
/usr/include/allegro/alcompat.h:155)
gswrirea.cc: In member function 'void gbuf::output_chars(const char*)':
gswrirea.cc:33: warning: 'void textout(BITMAP*, const FONT*, const char*, int,
int, int)' is deprecated (declared at /usr/include/al
legro/alcompat.h:157)
gswrirea.cc:33: warning: 'void textout(BITMAP*, const FONT*, const char*, int,
int, int)' is deprecated (declared at /usr/include/al
legro/alcompat.h:157)
gswrirea.cc: In member function 'void Schar::draw()':
gswrirea.cc:257: warning: 'void textout(BITMAP*, const FONT*, const char*, int,
int, int)' is deprecated (declared at /usr/include/a
llegro/alcompat.h:157)
gswrirea.cc:257: warning: 'void textout(BITMAP*, const FONT*, const char*, int,
int, int)' is deprecated (declared at /usr/include/a
llegro/alcompat.h:157)
error: Bad exit status from /var/tmp/rpm-tmp.91582 (%build)

I could not locate where should makeinfo come from.

makeinfo is part of texinfo, so I'm missing a BuildRequires: texinfo, sorry
about that.

Package Review
==============

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:
source RPM: none
binary RPM: 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 Guidelines.
 [x] License field in the package spec file matches the actual license.
     License type:Giftware
 [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: 6d5548495da8d693ab5f39c648421541b9059279 gstrm16.zip
 [x] Package is not known to require ExcludeArch
 [!] All build dependencies are listed in BuildRequires, except for any that are
listed in the exceptions section of Packaging Guidelines.
See note 1
 [x] The spec file handles locales properly.
 [x] ldconfig called in %post and %postun if required.
 [x] Package must own all directories that it creates.
 [x] 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).
 [!] Package consistently uses macros.
See Note 2
 [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.
 [x] Header files in -devel subpackage, if present.
 [-] Static libraries in -devel subpackage, if present.
 [-] Package requires pkgconfig, if .pc files are present.
 [x] Development .so files in -devel subpackage, if present.
 [x] 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: F7 and devel / x86_64
 [x] Package should compile and build into binary rpms on all supported
architectures.
     Tested on: F7 and devel / x86_64
 [x] Package functions as described.
Allows building of maze, bz #429451
 [x] Scriptlets must be sane, if used.
 [-] The placement of pkgconfig(.pc) files is correct.
 [x] File based requires are sane.


=== Issues ===
1. Missing BR: texinfo
2. Maybe
  make %{?_smp_mflags} -f Makefile.unx MAKEDOC=/usr/bin/allegro-makedoc \
should be replaced with
  make %{?_smp_mflags} -f Makefile.unx MAKEDOC=%{_bin}/allegro-makedoc \
Not a big deal, but for consistency sake
3. Fedora compiler flags are not used in the sequence
# makefile makes a .a file, make a .so ourselves
gcc -shared -o libgstrm.so.0 -Wl,-soname,libgstrm.so.0 *.o

=== Final Notes ===
1. MUSTFIX: Missing BR for texinfo
2. MUSTFIX: proper compiler flags must be used in the command listed as issue no. 3


================
*** APPROVED *** with the condition of fixing at least the 2 problems mentioned
under Final Notes (+ issue 2 from Issues if possible)
================

Thanks for the review! I'll fix all 3 mentioned issues before import.

New Package CVS Request
=======================
Package Name:      gstream
Short Description: Simplified stream output/input for Allegro
Owners:            jwrdegoede
Branches:          F-7 F-8 devel
InitialCC:         <empty>
Cvsextras Commits: Yes

cvs done.

allegro-4.2.2-7.fc8 has been pushed to the Fedora 8 stable repository.  If problems still persist, please make note of it in this bug report.

allegro-4.2.2-7.fc7 has been pushed to the Fedora 7 stable repository.  If problems still persist, please make note of it in this bug report.

Imported and build for F-7 F-8 and devel, updates for F-7 and F-8 are on their
way, closing.