{Not Official Reviewer}

- Mockbuild is Failed for i386 FC6 
unix.c:1072: warning: ignoring return value of 'fgets', declared with attribute
warn_unused_result
gcc  -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector
--param=ssp-buffer-size=4 -m32 -march=i386 -mtune=generic
-fasynchronous-unwind-tables -DHAVE_CONFIG_H -DNO_DEBUG -Wall -O3 -march=i686
-mmmx -msse -I../include -pthread -I/usr/include/freetype2 -c dldbfd.c
dldbfd.c: In function 'init_global_symbol_table':
dldbfd.c:686: error: too few arguments to function 'bfd_hash_table_init'
dldbfd.c: In function 'link_archive_members':
dldbfd.c:2725: error: too few arguments to function 'bfd_hash_table_init'
dldbfd.c: In function 'dld_find_executable':
dldbfd.c:3188: warning: ignoring return value of 'getcwd', declared with
attribute warn_unused_result
make[1]: *** [dldbfd.o] Error 1
make[1]: Leaving directory `/builddir/build/BUILD/lush-1.2/src'
make: *** [all] Error 2
error: Bad exit status from /var/tmp/rpm-tmp.32198 (%build)

- rpmlint on source rpm is not silent 
W: tclabc mixed-use-of-spaces-and-tabs
The specfile mixes use of spaces and tabs for indentation, which is a
cosmetic annoyance.  Use either spaces or tabs for indentation, not both.

Use sed -i -e 's|\t| |g' lush.spec 
to remove that rpmlint warning

+ dist tag is present
+ Buildroot is correct
+ source URL is correct
+ License used is GPL
+ License file COPYING is included
+ MD5 sum on tarball is matching upstream tarball
95010c360350bf0a489ddb4d4cfa089f  lush-1.2.tar.gz

Now builds in FC6 mock (this SRPM will not work on FC5).
Spaces/Tab fixed.

http://math.ifi.unizh.ch/fedora/5/i386/SRPMS.gemi/lush-1.2-2.src.rpm

On x86_64 rawhide, the compilation completes but the build fails at the rather
new check-buildroot stage:

+ /usr/lib/rpm/check-buildroot
Binary file
/var/tmp/lush-1.2-2.fc6-root-mockbuild/usr/share/lush/sys/stdenv.dump matches
Found '/var/tmp/lush-1.2-2.fc6-root-mockbuild' in installed files; aborting
error: Bad exit status from /var/tmp/rpm-tmp.59323 (%install)

So the buildroot is leaking into the installed files somehow.

Also, if you really care about it, the spaces/tabs thing is still there.  The
tabs are on the Patch0: line.

Lush needs to set a local load path in order to load the source files needed to
create the dump. Unfortunately, this load path is stored into the dump file,
although it is not needed, since the runtime works correctly. I suggest
therefore to ignore the check and disable it.

Well, to disable it, you put
   export QA_SKIP_BUILD_ROOT=1
at the end of %install.

This does get things building, and I think it's acceptable to do this if there's
certainty that the location of the buildroot isn't placed anywhere where it
might affect the execution of any code.

Now, for rpmlint:
  W: lush mixed-use-of-spaces-and-tabs (spaces: line 1, tab: line 11)

Loads of these:
  W: lush devel-file-in-non-devel-package /usr/share/lush/packages/svm/lasvm/lasvm.c
It's a compiler, so devel-file-in-non-devel-package warnings are OK.

What remains is a load of non-executable-script errors.  These four:
   E: lush non-executable-script /usr/share/lush/etc/compile-all 0644
   E: lush non-executable-script /usr/share/lush/etc/lush-find-string 0644
   E: lush non-executable-script /usr/share/lush/etc/make-html-manual 0644
   E: lush non-executable-script /usr/share/lush/etc/make-latex-manual 0644
are the only ones that aren't demos or examples.  Should they even be packaged?
 They look like build-time utilities.

Should the demos and examples be packaged as documentation?

(In reply to comment #5)
> Well, to disable it, you put
>    export QA_SKIP_BUILD_ROOT=1
> at the end of %install.
> 
> This does get things building, and I think it's acceptable to do this if there's
> certainty that the location of the buildroot isn't placed anywhere where it
> might affect the execution of any code.

Might it be possible to generate the problematic file in %post in order to avoid
this problem? If possible, that would seem to me to be a cleaner solution.

I don't think basically compiling the package in %post is a good idea.

Seriously, this is one of those things that check-buildroot is just going to
fail on.  It remains to decide whether this is acceptable; I believe it is in
this case, but it needs more scrutiny.

(In reply to comment #7)
> I don't think basically compiling the package in %post is a good idea.
I thought of the idea too, and the building is fast, but I think it is really a
little overkill just to get rid of an aesthetic issue. BTW, I could also copy
the dump file in the build directory, which is exactly the same as the one in
the build root, except that it contains the build dir path instead of the build
root path.

> Seriously, this is one of those things that check-buildroot is just going to
> fail on.  It remains to decide whether this is acceptable; I believe it is in
> this case, but it needs more scrutiny.
I straced the running of lush on one of the demos (which triggers some compiling
through gcc), and there was not once a reference to the build root.

One other possibility (besides the imminently reasonable one of just leaving
things the way they are) is to just edit it out, change the string to spaces or
something.  If it's really not used, it shouldn't make any difference, and if it
were used, it would prevent whatever problematic behavior the check in question
is supposed to protect against.

I also thought of something like that. Have you got an idea how to do this in
the simplest possible manner?

You could use sed, I suppose.  It is imperative, however, that the replacement
be the same length as the original string.  But this is so fragile that, while
it might be a fun exercise, it's almost certainly worse than just leaving things
alone.  The name of the build root changes depending on who builds the package,
so you have to handle the length of the string changing.

I moved the demos to docdir, as well as those etc files that are not related to
installation. The buildroot check is disabled.
http://math.ifi.unizh.ch/fedora/5/i386/SRPMS.gemi/lush-1.2-3.src.rpm

There are still a bunch of files under /usr/share/lush/packages/sn28/examples
which prompt non-executable-script errors from rpmlint.  What do you think
should happen with these?

The compiler is called with an odd set of flags.  Some of them look to be the
normal ones, but then '-O3' is used and '-g' is missing.  This causes the
debuginfo package to be mostly empty.  I tried passing --enable-debug to
%configure and this turns on debugging (and fixes the debuginfo subpackage) but
drops the optimization flag.  I'm not sure what to do here, other than hacking
the configure script.

* source files match upstream:
   95010c360350bf0a489ddb4d4cfa089f  lush-1.2.tar.gz
   3838fc7de8367a63349635766c657fbf  lush-manual.pdf
* package meets naming and packaging guidelines.
* 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 aren't correct.
* %clean is present.
* package builds in mock (development, x86_64).
* package installs properly
X debuginfo package is mostly empty
X rpmlint has some complaints.
* final provides and requires are sane:
   lush = 1.2-3.fc6
  =
   libX11.so.6()(64bit)
   libXft.so.2()(64bit)
   libXrender.so.1()(64bit)
   libfontconfig.so.1()(64bit)
   libfreetype.so.6()(64bit)
   libstdc++.so.6()(64bit)

* %check is not present; no test suite upstream.
* 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 (except those used internally)
* no pkgconfig files.
* no libtool .la droppings.

Here is the latest version:
http://math.ifi.unizh.ch/fedora/6/i386/SRPMS.gemi/lush-1.2.1-1.fc6.src.rpm

I fixed the compiler flags and some file permissions.

My sincere apologies; I seem to have completely forgotten about this one.

The compiler flags certainly look correct now.
The debuginfo package looks better as a result.
Only the acceptable (since this is a compiler) "devel-file-in-non-devel-package"
rpmlint complaints remain.

Those were the only problems I was, and they're all fixed.

APPROVED

Build for FC-6 and FC-7.
Added entries in owners.list and comps files.
Thanks for the review!