Bug 207839 - Review Request: lush - An object-oriented Lisp interpreter and compiler
Review Request: lush - An object-oriented Lisp interpreter and compiler
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Jason Tibbitts
Fedora Package Reviews List
:
Depends On:
Blocks: FE-ACCEPT
  Show dependency treegraph
 
Reported: 2006-09-24 07:46 EDT by Gérard Milmeister
Modified: 2007-11-30 17:11 EST (History)
2 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2006-12-29 05:57:21 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)

  None (edit)
Description Gérard Milmeister 2006-09-24 07:46:59 EDT
Spec URL: http://math.ifi.unizh.ch/fedora/spec/lush.spec
SRPM URL: http://math.ifi.unizh.ch/fedora/5/i386/SRPMS.gemi/lush-1.2-1.src.rpm
Description:
Lush is an object-oriented programming language designed for
researchers, experimenters, and engineers interested in large-scale
numerical and graphic applications. Lush is designed to be used in
situations where one would want to combine the flexibility of a
high-level, loosely-typed interpreted language, with the efficiency of
a strongly-typed, natively-compiled language, and with the easy
integration of code written in C, C++, or other languages.
Comment 1 Parag AN(पराग) 2006-09-25 01:30:02 EDT
{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
Comment 2 Gérard Milmeister 2006-09-25 10:33:18 EDT
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
Comment 3 Jason Tibbitts 2006-10-04 21:42:01 EDT
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.
Comment 4 Gérard Milmeister 2006-10-05 11:55:54 EDT
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.
Comment 5 Jason Tibbitts 2006-10-06 00:48:19 EDT
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?
Comment 6 Paul Howarth 2006-10-06 05:50:39 EDT
(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.
Comment 7 Jason Tibbitts 2006-10-06 12:24:29 EDT
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.
Comment 8 Gérard Milmeister 2006-10-06 18:06:12 EDT
(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.
Comment 9 Jason Tibbitts 2006-10-06 18:14:58 EDT
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.
Comment 10 Gérard Milmeister 2006-10-06 18:18:35 EDT
I also thought of something like that. Have you got an idea how to do this in
the simplest possible manner?
Comment 11 Jason Tibbitts 2006-10-06 18:31:15 EDT
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.
Comment 12 Gérard Milmeister 2006-10-07 05:40:41 EDT
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
Comment 13 Jason Tibbitts 2006-10-15 21:57:33 EDT
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.
Comment 14 Gérard Milmeister 2006-11-22 19:15:07 EST
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.
Comment 15 Jason Tibbitts 2006-12-27 20:31:22 EST
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
Comment 16 Gérard Milmeister 2006-12-29 05:57:21 EST
Build for FC-6 and FC-7.
Added entries in owners.list and comps files.
Thanks for the review!

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