Note: This is a public test instance of Red Hat Bugzilla. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback at bugzilla.redhat.com.
Bug 195683 - Review Request: smarteiffel - The GNU Eiffel Compiler and Libraries
Summary: Review Request: smarteiffel - The GNU Eiffel Compiler and Libraries
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Jason Tibbitts
QA Contact: Fedora Package Reviews List
URL:
Whiteboard:
Depends On:
Blocks: FE-ACCEPT
TreeView+ depends on / blocked
 
Reported: 2006-06-16 15:46 UTC by Gérard Milmeister
Modified: 2007-11-30 22:11 UTC (History)
2 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2006-09-23 17:17:55 UTC
Type: ---
Embargoed:


Attachments (Terms of Use)

Description Gérard Milmeister 2006-06-16 15:46:37 UTC
Spec URL: http://math.ifi.unizh.ch/fedora/spec/smarteiffel.spec
SRPM URL: http://math.ifi.unizh.ch/fedora/5/i386/SRPMS.gemi/smarteiffel-2.2-3.src.rpm
Description:
SmartEiffel is a small, portable implementation of the Eiffel OO
programming language.  Eiffel cleanly implements all the important
concepts of OO programming, including: multiple inheritance,
genericity, polymorphism, and encapsulation.  Eiffels unique feature
is Design By Contract, which increases the reusability and reliability
of program modules.

Comment 1 Parag AN(पराग) 2006-06-18 10:48:33 UTC
Review for this package:-
      Mock build for i386 compiled package successfully with showinf errors as
cpio: SmartEiffel/bin/clean.c: No such file or directory
cpio: SmartEiffel/bin/clean.h: No such file or directory
cpio: SmartEiffel/bin/compile.c: No such file or directory
cpio: SmartEiffel/bin/compile.h: No such file or directory
cpio: SmartEiffel/bin/compile_to_c.h: No such file or directory
cpio: SmartEiffel/bin/compile_to_c1.c: No such file or directory
cpio: SmartEiffel/bin/compile_to_c10.c: No such file or directory
cpio: SmartEiffel/bin/compile_to_c11.c: No such file or directory
cpio: SmartEiffel/bin/compile_to_c12.c: No such file or directory
cpio: SmartEiffel/bin/compile_to_c13.c: No such file or directory
cpio: SmartEiffel/bin/compile_to_c14.c: No such file or directory
cpio: SmartEiffel/bin/compile_to_c15.c: No such file or directory
cpio: SmartEiffel/bin/compile_to_c16.c: No such file or directory
cpio: SmartEiffel/bin/compile_to_c17.c: No such file or directory
cpio: SmartEiffel/bin/compile_to_c18.c: No such file or directory
cpio: SmartEiffel/bin/compile_to_c19.c: No such file or directory
cpio: SmartEiffel/bin/compile_to_c2.c: No such file or directory
cpio: SmartEiffel/bin/compile_to_c20.c: No such file or directory
cpio: SmartEiffel/bin/compile_to_c21.c: No such file or directory
cpio: SmartEiffel/bin/compile_to_c22.c: No such file or directory
cpio: SmartEiffel/bin/compile_to_c23.c: No such file or directory
cpio: SmartEiffel/bin/compile_to_c24.c: No such file or directory
cpio: SmartEiffel/bin/compile_to_c25.c: No such file or directory
cpio: SmartEiffel/bin/compile_to_c26.c: No such file or directory
cpio: SmartEiffel/bin/compile_to_c27.c: No such file or directory
cpio: SmartEiffel/bin/compile_to_c28.c: No such file or directory
cpio: SmartEiffel/bin/compile_to_c29.c: No such file or directory
cpio: SmartEiffel/bin/compile_to_c3.c: No such file or directory
cpio: SmartEiffel/bin/compile_to_c30.c: No such file or directory
cpio: SmartEiffel/bin/compile_to_c31.c: No such file or directory
cpio: SmartEiffel/bin/compile_to_c32.c: No such file or directory
cpio: SmartEiffel/bin/compile_to_c33.c: No such file or directory
cpio: SmartEiffel/bin/compile_to_c34.c: No such file or directory
cpio: SmartEiffel/bin/compile_to_c35.c: No such file or directory
cpio: SmartEiffel/bin/compile_to_c36.c: No such file or directory
cpio: SmartEiffel/bin/compile_to_c37.c: No such file or directory
cpio: SmartEiffel/bin/compile_to_c4.c: No such file or directory
cpio: SmartEiffel/bin/compile_to_c5.c: No such file or directory
cpio: SmartEiffel/bin/compile_to_c6.c: No such file or directory
cpio: SmartEiffel/bin/compile_to_c7.c: No such file or directory
cpio: SmartEiffel/bin/compile_to_c8.c: No such file or directory
cpio: SmartEiffel/bin/compile_to_c9.c: No such file or directory

MUST Items:
     - MUST: rpmlint shows error 
      E: smarteiffel unknown-key GPG#3321270a
     - MUST: The package is named according to the Package Naming Guidelines.
     - MUST: The spec file name matching the base package osgcal, in the format
osgcal.spec
      - MUST: This package meets the Packaging Guidelines.
      - MUST: The package is licensed with an open-source compatible license GPL.
      - MUST: The License field in the package smarteiffel.spec file matches the
actual license file GNU_LICENSE in tarball.
      - MUST: The sources used to build the package matches the upstream source,
as provided in the spec URL. md5sum is correct.
      - MUST: This package owns all directories that it creates. 
      - MUST: This package did not contain any duplicate files in the %files
listing.
      - MUST: This package  have a %clean section, which contains rm -rf
$RPM_BUILD_ROOT.
      - MUST: This package used macros.
      - MUST: Document files are included like READ_ME.txt
 

Comment 2 Parag AN(पराग) 2006-06-18 11:17:12 UTC
Correction to above review sorry for mistakenly writing package name osgcal
 - MUST: The spec file name matching the base package smarteiffel, in the format
smarteiffel.spec

Comment 3 Gérard Milmeister 2006-06-18 19:09:23 UTC
cpio errors:
The generated C files are removed after compilation. Since they
are not useful for debugging anyway, I propose to disable the
debuginfo package.

E: smarteiffel unknown-key GPG#3321270a
This my GPG key:
http://math.ifi.unizh.ch/fedora/RPM-GPG-KEY-gemi


Comment 4 Parag AN(पराग) 2006-06-19 11:00:18 UTC
Above is Not an official review as I'm not yet sponsored

Comment 5 Jason Tibbitts 2006-06-24 02:32:01 UTC
I agree about the debug package; there's no point in it.

I can't seem to reach upstream to download the source tarball at the moment.

Builds fine in mock on x86_64, development; no parallel build, but perhaps that
can't be helped as there's not really a makefile.  The whole install process is
pretty bizarre.  (The tarball even includes a windows exe.  Weird.)

rpmlint spews a pile of warnings, but I'll grep out all of the
"devel-file-in-non-devel-package" warnings as this is a compiler.  That leaves:

W: smarteiffel conffile-without-noreplace-flag /etc/serc
Not sure what's up here.  Is it expected that the user will need to edit this
file?  If upgrading risks overwriting expected local configuration then
noreplace is indeed the proper thing to do, but I suppose this may be an
occasion where it's not warranted.

E: smarteiffel zero-length /usr/share/SmartEiffel/short/tex3/hook832
Generally zero-length files shouldn't be packaged, but I think in this case the
file needs to be there.  (If I understand correctly, it's a formatting hook,
where some text could be inserted at various points in some generated output.)

W: smarteiffel non-standard-dir-in-usr libexec
Seems acceptable to me, or at least in line with other accepted uses of libexec,
although I wonder how this would work with multilib.

W: smarteiffel doc-file-dependency
/usr/share/doc/smarteiffel-2.2/contrib/htmldoc/htmlshort /usr/bin/env
(and several others)
I don't think these should be executable.  (Wow, a program written in Ruby which
handles running the SmartEiffel compiler under gdb.)


Comment 6 Gérard Milmeister 2006-06-24 10:46:03 UTC
(In reply to comment #5)
> I agree about the debug package; there's no point in it.
> 
> I can't seem to reach upstream to download the source tarball at the moment.
I will use the current download url:
http://gforge.inria.fr/frs/download.php/586/SmartEiffel-2-2.tar.bz2

> W: smarteiffel conffile-without-noreplace-flag /etc/serc
> Not sure what's up here.  Is it expected that the user will need to edit this
> file?  If upgrading risks overwriting expected local configuration then
> noreplace is indeed the proper thing to do, but I suppose this may be an
> occasion where it's not warranted.
I don't think it makes much sense to change the settings in /etc/serc.

> E: smarteiffel zero-length /usr/share/SmartEiffel/short/tex3/hook832
> Generally zero-length files shouldn't be packaged, but I think in this case the
> file needs to be there.  (If I understand correctly, it's a formatting hook,
> where some text could be inserted at various points in some generated output.)
yes.

> W: smarteiffel non-standard-dir-in-usr libexec
> Seems acceptable to me, or at least in line with other accepted uses of libexec,
> although I wonder how this would work with multilib.
I always thought that /usr/libexec is the standard directory for executables
that are called as subprocesses and not directly by the user.
I really don't understand the warning, /usr/libexec seems pretty standard to
me...

> W: smarteiffel doc-file-dependency
> /usr/share/doc/smarteiffel-2.2/contrib/htmldoc/htmlshort /usr/bin/env
> (and several others)
> I don't think these should be executable.  (Wow, a program written in Ruby which
> handles running the SmartEiffel compiler under gdb.)
Hmm, if I chmod 0644 these files, I must also remove the shebang.
Maybe I should move the contrib directory to /usr/share/SmartEiffel.
There are also a few files to patch, paths etc...


Comment 7 Jason Tibbitts 2006-07-08 22:07:02 UTC
Were you going to cut a new package?

(In reply to comment #6)
> I don't think it makes much sense to change the settings in /etc/serc.

Seems reasonable to me.

> I always thought that /usr/libexec is the standard directory for executables
> that are called as subprocesses and not directly by the user.

It is; the packaging committee codified this and hopefully rpmlint will be
patched to stop warning about this soon.

> Hmm, if I chmod 0644 these files, I must also remove the shebang.

Not as I understand things.

> Maybe I should move the contrib directory to /usr/share/SmartEiffel.

I'm not sure it would be worth it.  Just remove execute permission and anyone
who wants to play with those files can copy them.

Comment 8 Jason Tibbitts 2006-09-12 19:16:47 UTC
Ping?  I'm happy to give this a full review if you'd care to cut a new package
and decided what you'd like to do with the contrib directory.

Comment 9 Gérard Milmeister 2006-09-12 20:07:43 UTC
Sorry, I currently quite busy with the final stage of my PhD studies.
But I will work on it, as soon as I find more time.

Comment 10 Gérard Milmeister 2006-09-19 22:14:26 UTC
Ok, I removed the execute bits from the files in docdir.
http://math.ifi.unizh.ch/fedora/5/i386/SRPMS.gemi/smarteiffel-2.2-4.src.rpm

Comment 11 Jason Tibbitts 2006-09-22 03:54:01 UTC
After running through my checklist, the only remaining issue I have is that the
installed package is about 91MB, but 68MB of that is documentation which does
bring up the question of whether the documentation should be in a subpackage. 
What do you think?  (Honestly I'd prefer not to have to build this again, but
there really is a big pile of documentation there.)

68128   ./usr/share/doc
81508   ./usr/share
91228   ./usr
91244   .

* source files match upstream:
   77b3ab3895c6fced2cb1649b4ca80547  SmartEiffel-2-2.tar.bz2
* 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 (none)
* compiler flags are appropriate.
* %clean is present.
* package builds in mock (development, x86_64).
* package installs properly
* debuginfo would be empty and is disabled.
* rpmlint has only acceptable complaints.
* final provides and requires are sane:
   config(smarteiffel) = 2.2-4.fc6
   smarteiffel = 2.2-4.fc6
  =
   /bin/sh
   config(smarteiffel) = 2.2-4.fc6
* %check is not present; no runnable test suite.
* no shared libraries are added to the regular linker search paths.
* package is not relocatable.
* 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.
* headers (and various source files) are installed as appropriate for a compiler.
* no pkgconfig files.
* no libtool .la droppings.

Comment 12 Gérard Milmeister 2006-09-22 15:16:39 UTC
I split off a doc package with the contents of the www directory:
http://math.ifi.unizh.ch/fedora/5/i386/SRPMS.gemi/smarteiffel-2.2-5.src.rpm

Comment 13 Jason Tibbitts 2006-09-22 19:14:42 UTC
This looks good to me.

APPROVED

Comment 14 Gérard Milmeister 2006-09-23 17:17:55 UTC
Built on FC5 and devel.
Made entries in owners.list and comps-fe5.xml.in and comps-fe6.xml.in.
Thanks for the review!


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