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
Summary: | Review Request: smarteiffel - The GNU Eiffel Compiler and Libraries | ||
---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Gérard Milmeister <gemi> |
Component: | Package Review | Assignee: | Jason Tibbitts <j> |
Status: | CLOSED NEXTRELEASE | QA Contact: | Fedora Package Reviews List <fedora-package-review> |
Severity: | medium | Docs Contact: | |
Priority: | medium | ||
Version: | rawhide | CC: | j, panemade |
Target Milestone: | --- | ||
Target Release: | --- | ||
Hardware: | All | ||
OS: | Linux | ||
Whiteboard: | |||
Fixed In Version: | Doc Type: | Bug Fix | |
Doc Text: | Story Points: | --- | |
Clone Of: | Environment: | ||
Last Closed: | 2006-09-23 17:17:55 UTC | Type: | --- |
Regression: | --- | Mount Type: | --- |
Documentation: | --- | CRM: | |
Verified Versions: | Category: | --- | |
oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |
Cloudforms Team: | --- | Target Upstream Version: | |
Embargoed: | |||
Bug Depends On: | |||
Bug Blocks: | 163779 |
Description
Gérard Milmeister
2006-06-16 15:46:37 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 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 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 Above is Not an official review as I'm not yet sponsored 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.) (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... 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. 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. 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. 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 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. 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 This looks good to me. APPROVED Built on FC5 and devel. Made entries in owners.list and comps-fe5.xml.in and comps-fe6.xml.in. Thanks for the review! |