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 174377
Summary: | Review Request: gnu-smalltalk - GNU Smalltalk | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Jochen Schmitt <jochen> | ||||||||||
Component: | Package Review | Assignee: | Mamoru TASAKA <mtasaka> | ||||||||||
Status: | CLOSED NEXTRELEASE | QA Contact: | Fedora Package Reviews List <fedora-package-review> | ||||||||||
Severity: | medium | Docs Contact: | |||||||||||
Priority: | medium | ||||||||||||
Version: | rawhide | CC: | gemi, jima, mtasaka, pbonzini, toshio | ||||||||||
Target Milestone: | --- | Keywords: | Reopened | ||||||||||
Target Release: | --- | Flags: | kevin:
fedora-cvs+
|
||||||||||
Hardware: | All | ||||||||||||
OS: | Linux | ||||||||||||
URL: | http://www.herr-schmitt.de/pub/gnu-smalltalk | ||||||||||||
Whiteboard: | |||||||||||||
Fixed In Version: | Doc Type: | Bug Fix | |||||||||||
Doc Text: | Story Points: | --- | |||||||||||
Clone Of: | Environment: | ||||||||||||
Last Closed: | 2007-05-23 04:48:56 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 | ||||||||||||
Attachments: |
|
Description
Jochen Schmitt
2005-11-28 17:15:17 UTC
SRPM still not there? Thank you for your comment. I have corrected it. It's seems that something was going wrong during the upload. Best Regards: Jochen Schmitt Good: - rpmlint clean - tarball matches upstream - installinfo Bad: - Package naming: Upstream name is just smalltalk. Debian has gnu-smalltalk, so I'd suggest renaming the package to gnu-smalltalk (for clarity: gst brings to mind gstreamer, not smalltalk; consistency with Debian and to leave room for other Smalltalk variants) - BuildRequires on gtk+-devel. This should be gtk2-devel. No need to specify a minimum version as there are no gtk2 <= 2.0 on any Fedora release. - gtk2-devel depends on glib2-devel and atk-devel already, you can remove them - Likewise with tcl-devel and tk-devel. tk-devel depends on tcl-devel already. They have been at versions >= 8 for a long time so no need to specify a minimum version - Want to add paragraph breaks in description? Space out the three paragraphs a bit. - Use %makeinstall instead of manually doing make DESTDIR=$RPM_BUILD_ROOT install *** Bug 174375 has been marked as a duplicate of this bug. *** * BuildRequires: readline-devel * Requires(post): /sbin/install-info Requires(postun): /sbin/install-info * %configure belongs to to %build section * rename the package "el" to "emacs", it's clearer * there are two additional info entries: /sbin/install-info %{_infodir}/gst-base.info %{_infodir}/dir 2>/dev/null || : /sbin/install-info %{_infodir}/gst-libs.info %{_infodir}/dir 2>/dev/null || : * libgst.so, libgst.a, gnu-smalltalk.pc and gst-config should be in the devel package * "make DESTDIR=$RPM_BUILD_ROOT install" is used in the skeleton produced fedora-newrpmspec, so it is ok. I have uploaded a updated version of the package: SPEC: http://www.herr-schmitt.de/pub/gnu-smalltalk/gnu-smalltalk.spec SRPM: http://www.herr-schmitt.de/pub/gnu-smalltalk/gnu-smalltalk-2.2-2.src.rpm Best Regards: Jochen Schmitt mockbuild fails: ... Patch #1 (gst-2.2-am.patch): + patch -p1 -b --suffix .org -s + exit 0 Executing(%build): /bin/sh -e /var/tmp/rpm-tmp.31985 + umask 022 + cd /builddir/build/BUILD + cd smalltalk-2.2 + LANG=C + export LANG + unset DISPLAY + automake configure.ac:20: version mismatch. This is Automake 1.9.5, configure.ac:20: but the definition used by this AM_INIT_AUTOMAKE configure.ac:20: comes from Automake 1.9.6. You should recreate configure.ac:20: aclocal.m4 with aclocal and run automake again. error: Bad exit status from /var/tmp/rpm-tmp.31985 (%build) Possibly patch "configure" directly. When I make a aclocal as described in the error message I got a strange error message which refer to a package called lightning. So I have decide to create a new package called lightning, which will be a build require for gnu-smalltalk. You may find the review request for this package on bug #163776 Best Regards: Jochen Schmitt Correction: the Bug number is #174952. Sorry for any inconvinience. Best Regards: Jochen Schmitt I have uploaded the packages which refer to lightning to: SRPM: http://www.herr-schmitt.de/pub/gnu-smalltalk/gnu-smalltalk-2.2-3.src.rpm SPEC: http://www.herr-schmitt.de/pub/gnu-smalltalk/gnu-smalltalk.spec Best Regards: Jochen Schmitt Lightning seems to be used for jit compiling. I once compiled gst with jit enabled and this would not work. In your spec file, there is --enable-jit to the configure script (in any way a package would not be required to enable the jit). Thus there should not be a requirement on lightning. I think you should patch the configure script and Makefile.in directly. Corrected version: SPEC: http://www.herr-schmitt.de/pub/gnu-smalltalk//gnu-smalltalk.spec SRPM: http://www.herr-schmitt.de/pub/gnu-smalltalk//gnu-smalltalk-2.2-4.src.rpm Best Regards: Jochen Schmitt - gnu-smalltalk-devel should depends on %{name}-%{version} not just %{name} - I'm not sure why the *.a, *.la, sigsegv.h files are removed. Should they not be put in -devel? - Likewise, the libgst.so is OK in -devel, but libgst.so.* should be in the main gnu-smalltalk package. Almost ready to go otherwise. Gerard, anything I missed? - Michel On http://www.fedoraproject.org/wiki/PackagingGuidelines#head-e3f33b737a91aa22385d1cb155bb8d611558ae22 you can read, that *.a and *.la should be not included in any package. AFAIK sigsegv is provided by glibc-devel. Best Regards: Jochen Schmitt *.la files shouldn't be included if possible, however I disagree with the packaging guidelines about the *.a. It is useful for developers to be able to link against certain static libraries, so they can distribute their programs without the libraries. Sometimes however libraries require more than just the library code itself. Then this is not possible. In the case of gnu smalltalk I am not sure if it useful to include *.a or not, you may do as you want. "sigsegv.h" is part of libsigsegv-devel. This library is included with gst and linked-in. The header file is not used, so it can be left out. I agree with the rest of Michel's comments. If I wrote 'Requires: %{name}-%{version} in the spec file, I got the following messages during installing the packages: [root@myhome pclinux]# cd redhat/RPMS/i686/ [root@myhome i686]# rpm -i gnu-smalltalk-2.2-5.i686.rpm gnu-smalltalk-devel-2.2-5.i686.rpm gnu-smalltalk-emacs-2.2-5.i686.rpm Fehler: Failed dependencies: gnu-smalltalk-2.2 is needed by gnu-smalltalk-devel-2.2-5.i686 gnu-smalltalk-2.2 is needed by gnu-smalltalk-emacs-2.2-5.i686 [root@myhome i686]# Question: What should I do? I'm using rpm-4.4.1. Best Regards: Jochen Schmitt Created attachment 122189 [details]
Spec file which coused troubled during installation
(In reply to comment #16) > If I wrote 'Requires: %{name}-%{version} in the spec file, I got the following > messages during installing the packages: > > [root@myhome pclinux]# cd redhat/RPMS/i686/ > [root@myhome i686]# rpm -i gnu-smalltalk-2.2-5.i686.rpm > gnu-smalltalk-devel-2.2-5.i686.rpm gnu-smalltalk-emacs-2.2-5.i686.rpm > Fehler: Failed dependencies: > gnu-smalltalk-2.2 is needed by gnu-smalltalk-devel-2.2-5.i686 > gnu-smalltalk-2.2 is needed by gnu-smalltalk-emacs-2.2-5.i686 Requires: %{name} = %{version} or even: Requires: %{name} = %{version}-%{release} Thank you for your tip. The next release of the package can you found at: SRPM: http://www.herr-schmitt.de/pub/gnu-smalltalk/gnu-smalltalk-2.2-5.src.rpm SPEC: http://www.herr-schmitt.de/pub/gnu-smalltalk/gnu-smalltalk.spec Best Regards: Jochen Schmitt Hello, it will be nice, if any may approve the package. Best Regards: Jochen Schmitt (In reply to comment #20) > it will be nice, if any may approve the package. Well, IMO, this package still needs quite some love before it can be approved. I tried to build your *-5.rpm and am still seeing quite a lot of arguable details: 1. /usr/bin/gst contains a invalid rpath. 2. Disable building the static libs. 3. Many of the *st scripts in /usr/share carry a broken SHEBANG and broken permissions. 4. Please explain the purpose of the *-am.patch. In its core, the package renames the package (comprising the tarball's name!) and should not be applied. IMO this patch should not be applied. 5. make check should be moved to %check ... > 1. /usr/bin/gst contains a invalid rpath. Added --disable-rpath > 2. Disable building the static libs. Added --enable-static=no > 3. Many of the *st scripts in /usr/share carry a broken SHEBANG and broken > permissions. I have fixed the boken shebangs. If a *.st file have a shebang, a+x will be set. > 4. Please explain the purpose of the *-am.patch. > In its core, the package renames the package (comprising the tarball's name!) > and should not be applied. IMO this patch should not be applied. I must refer you to comment #3 of this bug. The debian guys have change the package name in the same way. I think, that a directory should be better named /usr/share/gnu-smalltalk instead of '/usr/share/smalltalk'. > 5. make check should be moved to %check Done. download: SrPM: http://www.herr-schmitt.de/pub/gnu-smalltalk/gnu-smalltalk-2.2-6.src.rpm SPEC: http://www.herr-schmitt.de/pub/gnu-smalltalk/gnu-smalltalk.spec Best Regards: Jochen Schmitt Hello, it will be nice if anyone get his eyes to the corrected package in comment #22. Thank you for your patient. Best Regards: Jochen Schmitt * make check currently fails on FC5-devel/x86-64. The errors go away if the patch is not applied; the intmath test is causing the problem. * run rpmlint on it: the binary RPM produced has the errors below. Not sure what to do about the rpath error, as for the others, make sure all the scripts that begin with #!/bin/sh have 0755 permission. E: gnu-smalltalk binary-or-shlib-defines-rpath /usr/bin/gst ['/home/build/redhat/tmp/smalltalk-2.2/libgst/.libs', '/usr/lib64'] E: gnu-smalltalk non-executable-script /usr/share/gnu-smalltalk/vfs/uar 0644 E: gnu-smalltalk non-executable-script /usr/share/gnu-smalltalk/examples/shell 0644 E: gnu-smalltalk non-executable-script /usr/share/gnu-smalltalk/vfs/mailfs 0644 E: gnu-smalltalk non-standard-executable-perm /usr/share/gnu-smalltalk/Test.st 0744 E: gnu-smalltalk non-executable-script /usr/share/gnu-smalltalk/examples/pepe.sed 0644 E: gnu-smalltalk non-executable-script /usr/share/gnu-smalltalk/vfs/uzip 0644 E: gnu-smalltalk non-executable-script /usr/share/gnu-smalltalk/vfs/utar 0644 E: gnu-smalltalk non-executable-script /usr/share/gnu-smalltalk/vfs/urar 0644 E: gnu-smalltalk non-standard-executable-perm /usr/share/gnu-smalltalk/Load.st 0744 E: gnu-smalltalk postin-without-ldconfig /usr/lib64/libgst.so.4.1.1 E: gnu-smalltalk library-without-ldconfig-postun /usr/lib64/libgst.so.4.1.1 W: gnu-smalltalk devel-file-in-non-devel-package /usr/share/gnu-smalltalk/unsupported/binary.c E: gnu-smalltalk non-executable-script /usr/share/gnu-smalltalk/vfs/patchfs 0644E: gnu-smalltalk non-standard-executable-perm /usr/share/gnu-smalltalk/Reload.st 0744 E: gnu-smalltalk non-executable-script /usr/share/gnu-smalltalk/vfs/deb 0644 E: gnu-smalltalk non-executable-script /usr/share/gnu-smalltalk/vfs/ucpio 0644 E: gnu-smalltalk non-executable-script /usr/share/gnu-smalltalk/examples/xml.sed 0644 E: gnu-smalltalk non-executable-script /usr/share/gnu-smalltalk/vfs/uzoo 0644 E: gnu-smalltalk non-executable-script /usr/share/gnu-smalltalk/vfs/lslR 0644 E: gnu-smalltalk non-executable-script /usr/share/gnu-smalltalk/vfs/ulha 0644 Thank you for your respone. For the first point, I need more input. Did you get any error message for the failed test? For the seconed point, I have done some fixed but I will get the following error messages from rpmlint E: gnu-smalltalk binary-or-shlib-defines-rpath /usr/bin/gst ['/home/pclinux/redhat/BUILD/smalltalk-2.2/libgst/.libs'] E: gnu-smalltalk shlib-with-non-pic-code /usr/lib/libgst.so.4.1. It will be nice if you have any hint to solve this problem. For testing I have uploaded a current version at: SrPM: http://www.herr-schmitt.de/pub/gnu-smalltalk/gnu-smalltalk-2.2-7.src.rpm SPEC: http://www.herr-schmitt.de/pub/gnu-smalltalk/gnu-smalltalk.spec Best Regards: Jochen Schmitt Only the .i686 RPM is uploaded on that address. I rebuilt from the new .spec file, and on i386 gets the same errors you get. The shlib-with-non-pic-code is probably not that bad; I just recompiled libid3tag to compare and it produced the same error with rpmlint. So you're down to one error (rpath) plus compilation failure on x86_64 if the patch is applied. The 'make check' log is attached; look for this line in it: ./run-test: line 9: 6984 Segmentation fault $top_builddir/gst -rI $top_builddir/gst.im ${base}.st >$build_base.log 2>&1 At this point it's probably advisable to contact upstream developers and ask about the rpath problem (also, why renaming the package and regenerating the build scripts cause some libraries to fail on some platforms). Once we know more about the rpath problem, a quick fix would be for you to exclude x86_64 from the supported architecture, pending a fix for that, and the package can then be approved. Created attachment 124254 [details]
x86_64 make check failure log
Similar rpath problem discussed on a Debian mailing list: http://lists.debian.org/debian-mentors/2005/10/msg00080.html I did a cursory scan and a lot of the Makefiles have -rpath in them. Perhaps you can automatically remove the references and try and see if it rebuilds fine and passes the test suite.. there is the possibility that the rpath's are all required, but in that case, the --disable-rpath switch should not be there in configure. For the PIC problem, just add --with-pic=yes to %configure. For some reason by default it uses both PIC and non-PIC object When I look at the build output I saw a lot oth -rpath. But when I look in the Makefiles I have trouble to find the rpath options: Perhaps someone have any idea, how the rpath will be generated in the Makefile. I'd help but there's no SRPM. Could you roll a new one with Michel's --with-pic=yes suggestion and upload so I can take a look at what's going on? The glanced at the tarball and RPATH is probably being snuck in via libtool. This might work for you: BuildRequires: libtool [...] make %{?_smp_mflags} LIBTOOL=/usr/bin/libtool Thank you for your comment. But unfortunately now I will got the following error message: SMALLTALK_KERNEL="`cd ./kernel; pwd`" \ SMALLTALK_IMAGE="`pwd`" \ ./gst -iQ /dev/null a Smalltalk string:1: Aborted (ip 0)UndefinedObject>>#executeStatements (ip 0)<bottom> make[2]: *** [gst.im] Aborted but when I remove the LIBTOOL=/usr/bin/libtool from the make step then it's works. I emailed Paolo Bonzini, the maintainer; hopefully he can take a look into this. Created attachment 125851 [details]
Fixed specfile
Comment on attachment 125851 [details] Fixed specfile Summary: GNU Smalltalk Name: smalltalk Version: 2.2 Release: 8%{?dist} Source: ftp://ftp.gnu.org/gnu/smalltalk/smalltalk-%{version}.tar.gz License: GPL Group: Development/Languages URL: http://www.gnu.org/software/smalltalk/smalltalk.html BuildRoot: %{_tmppath}/%{name}-%{version}-root-%(%{__id_u} -n) Requires(post): /sbin/install-info Requires(preun): /sbin/install-info BuildRequires: tk-devel BuildRequires: gtk2-devel BuildRequires: gdbm-devel BuildRequires: gmp-devel BuildRequires: readline-devel BuildRequires: emacs %description GNU Smalltalk is an implementation that closely follows the Smalltalk-80 language as described in the book `Smalltalk-80: the Language and its Implementation' by Adele Goldberg and David Robson. The Smalltalk programming language is an object oriented programming language. Unlike other Smalltalks (including Smalltalk-80), GNU Smalltalk emphasizes Smalltalk's rapid prototyping features rather than the graphical and easy-to-use nature of the programming environment. Therefore, even though we have a nice GUI environment including a class browser, the goal of the GNU Smalltalk project is currently to produce a complete system to be used to write your scripts in a clear, aesthetically pleasing, and philosophically appealing programming language. %package emacs Summary: Emacs mode for GNU Smalltalk Group: Applications/Editors Requires: %{name} = %{version}, emacs %description emacs This Package contains the Smalltalk mode for Emacs. %package devel Summary: Development Stuff for the GNU Smalltalk package Group: Development/Libraries Requires: %{name} = %{version} %description devel This Package contains header files and other stuff provided by GNU Smalltalk. You will need this package, if you want to extent GNU Smalltalk with functions written in C. %prep %setup -q -n smalltalk-%{version} %build %configure --with-tcl=%{_libdir} --with-tk=%{_libdir} \ # --enable-static=no --with-pic=yes make %{?_smp_mflags} %install rm -rf $RPM_BUILD_ROOT make DESTDIR=$RPM_BUILD_ROOT install rm -rf $RPM_BUILD_ROOT/%{_includedir}/sigsegv.h find $RPM_BUILD_ROOT/%{_libdir}/smalltalk -name '*.st' \ -print >files.list find $RPM_BUILD_ROOT/%{_datadir}/smalltalk -type f \ -print >>files.list # # Fix broken Shebangs # cat files.list | ( while read file; do sed 's|@bindir@|%{_bindir}|g' <$file >$file.new mv -f $file.new $file if grep '^#!' $file; then chmod 0755 $file fi done ) %check make check %clean rm -rf $RPM_BUILD_ROOT %post /sbin/install-info %{_infodir}/gst.info %{_infodir}/dir || : /sbin/install-info %{_infodir}/gst-base.info %{_infodir}/dir || : /sbin/install-info %{_infodir}/gst-libs.info %{_infodir}/dir || : /sbin/ldconfig %post devel -p /sbin/ldconfig %preun if [ $1 = 0 ]; then /sbin/install-info --delete %{_infodir}/gst.info %{_infodir}/dir || : /sbin/install-info --delete %{_infodir}/gst-base.info %{_infodir}/dir || : /sbin/install-info --delete %{_infodir}/gst-libs.info %{_infodir}/dir || : fi %postun -p /sbin/ldconfig %postun devel -p /sbin/ldconfig %files %defattr(-,root,root,-) %{_bindir}/gst %{_bindir}/gst-load %{_bindir}/gst-package %{_bindir}/gst-reload %{_bindir}/gst-sunit %{_libdir}/libgst.so.* %dir %{_libdir}/smalltalk/ %{_libdir}/smalltalk/*.so %{_datadir}/smalltalk/ %{_infodir}/gst.info* %{_infodir}/gst-*.info* %{_mandir}/man1/gst.1* %doc AUTHORS COPYING COPYING.DOC COPYING.LIB ChangeLog %doc NEWS README THANKS TODO %files devel %defattr(-,root,root,-) %{_bindir}/gst-config %{_libdir}/libgst.so %{_libdir}/libgst.a %{_libdir}/smalltalk/*.a %ghost %{_libdir}/*.la %ghost %{_libdir}/smalltalk/*.la %{_libdir}/pkgconfig/gnu-smalltalk.pc %{_datadir}/aclocal/*.m4 %{_includedir}/gst.h %{_includedir}/gstpub.h %doc COPYING %files emacs %defattr(-,root,root,-) %{_datadir}/emacs/site-lisp/* %doc COPYING %changelog * Wed Mar 8 2006 Michel Salim <michel.salim> 2.2-8 - Reverted name to smalltalk to avoid rpath oddities and test failures - Spec cleanup * Tue Jan 31 2006 Jochen Schmitt <jochen herr-schmitt de> 2.2-7 - Fix rpmlint errors * Tue Jan 10 2006 Jochen Schmitt <Jochen herr-schmitt de> 2.2-6 - Added --disable-rpath - Added --enable-static=no - fix broken Shebangs * Tue Dec 13 2005 Jochen Schmitt <Jochen herr-schmitt de> 2.2-5 - Deps from -devel and -emacs more strict - Move libgst.so.* to main package * Wed Dec 7 2005 Jochen Schmitt <Jochen herr-schmitt de> 2.2-4 - remove dep to lightning * Sun Dec 4 2005 Jochen Schmitt <Jochen herr-schmitt de> 2.2-3 - Add aclocal - Add depend to lightning * Tue Nov 29 2005 Jochen Schmitt <Jochen herr-schmitt de> 2.2-2 - Rename package - install-info for gst-base and gst-libs - move libgst.so to devel package * Thu Nov 24 2005 Jochen Schmitt <Jochen herr-schmitt de> 2.2-1 - Initial RPM Seems like something definitely breaks when the renaming of smalltalk -> gnu-smalltalk is performed. Attached is a spec file that does not require *any* funny rpath and pic toggle to pass rpmlint tests. The renaming can perhaps be scrapped since the chance of a second smalltalk implementation wanting to be installed to the same directory is pretty slim anyway. Created attachment 125853 [details]
Actual fixed specfile
It strange, rpmlint complaints $ rpmlint smalltalk-2.2-8.i686.rpm E: smalltalk shlib-with-non-pic-code /usr/lib/libgst.so.4.1.1 when I used the attached SPEC file. Might want to add the --with-pic=yes back to configure. I neglected to test it on i386; x86-64 does not exhibit that problem. I have used --with-pic=yes. This review appears to be stalled; consider this the one-week warning as described here: http://fedoraproject.org/wiki/Extras/Policy/StalledReviews If Michel doesn't respond within one week, we can revert this review to FE-NEW and another reviewer may take over. The shlib-with-non-pic-code is necessary to get decent performance on i386 due to the low number of registers. If it is stalled, might as well restart from the GNU Smalltalk CVS repository on savannah. Great, I forgot about this. Reverting to FE-NEW and changing owner back to "nobody." If the submitter wants to do as Paolo suggested in Comment #42, go ahead. There is a new prerlease of gnu-smalltalk which solved all the issue reported in this review SRPM: http://www.herr-schmitt.de/pub/gnu-smalltalk/gnu-smalltalk-2.2c-1.src.rpm SPEC: http://www.herr-schmitt.de/pub/gnu-smalltalk/gnu-smalltalk.spec Well, very quick look at this: * Why isn't %configure macro used in spec file? * Is exporting 'CCFLAGS' variable required? * make %{_smp_mflags} CFLAGS="$RPM_OPT_FLAGS" The description CFLAGS="$RPM_OPT_FLAGS" is not needed when %configure macro is used. * -devel package should not require ldconfig call. * %{_bindir}/gst-config The content of this file is incorrect as this contains --------------------------------------------- gst_libs='@LDPATH@ -lnsl -lm ' ---------------------------------------------- The last item is fixed by this upstream patch: --- orig/gst-config.in +++ mod/gst-config.in @@ -2,7 +2,6 @@ : ${prefix:=@prefix@} : ${exec_prefix:=@exec_prefix@} -gst_libs='@LDPATH@ @LIBS@' exec_prefix_set=no usage() @@ -69,6 +68,9 @@ while test $# -gt 0; do shift done +: ${pkglibdir:="@libdir@/@PACKAGE@"} +gst_libs="${pkglibdir} @LIBS@" + if test "$echo_prefix" = "yes"; then echo $prefix fi Next Release: SRPM: http://www.herr-schmitt.de/pub/gnu-smalltalk/gnu-smalltalk-2.2c-2.src.rpm SPEC: http://www.herr-schmitt.de/pub/gnu-smalltalk/gnu-smalltalk.spec Okay, I will review this package. 0. spec file description : * %prep - applying patches I usually recommend to add some suffix every time you apply a patch, e.g. %patch1 -p1 -b .am * %build section - export CFLAGS="$RPM_OPT_FLAGS" This is not necessary because %configure macro sets this. - make %{?_smp_mflags} LIBTOOL="/usr/bin/libtool" Use macro. i.e. LIBTOOL="%{_bindir}/libtool" * %install section - Fix broken Shebangs (Please see "timestamp" section below for this). * %files section - COPYING Why does every subpackage have same COPYING file? Only for rpmlint issue? If so, please remove these as this rpmlint can be ignored. - *.st files Well, actually I first saw this utility so I don't quite know how to use this package, however, are these *.st really required for 'minimal' use of smalltalk? Or are these somewhat 'add-on' files for smalltalk? (The judgment as of what is 'minimal' use is up to you.) - /usr/share/gnu-smalltalk/unsupported/binary.c rpmlint complains about this file as 'devel-file-in-non-devel-package'. Would you explain why this file should be in main package? Then: 1. From http://fedoraproject.org/wiki/Packaging/Guidelines : * Timestamps This package contains lots of text files (mainly *.st files), so keeping timestamps is highly preferable because - it shows when the file is written by upstream or vendor (as you) - it may suggest whether vendor (you) have changed original content. So please keep timestamps for text files included. Then: A. Try: 'make DESTDIR=$RPM_BUILD_ROOT INSTALL="install -c -p" install' B. For "Fix broken Shebangs" section: B-1 check if this section is still needed. B-2 If so (i.e. this section is still needed), change the description of this section so that the files which are actually not needed to be fixed _WON'T_ change their timestamps. * File and Directory Ownership - %{_datadir}/aclocal/ This directory is owned only by automake and so -devel package should require automake. 2. From http://fedoraproject.org/wiki/Packaging/ReviewGuidelines : (okay) I can reply as far as the upstream package is concerned... > * %install section > - Fix broken Shebangs > (Please see "timestamp" section below for this). Should not be necessary anymore. > - *.st files > Well, actually I first saw this utility so I don't quite know > how to use this package, however, are these *.st really required > for 'minimal' use of smalltalk? Or are these somewhat 'add-on' > files for smalltalk? (The judgment as of what is 'minimal' use > is up to you.) They are needed to load the optional packages (e.g. sockets, GUI bindings, ...). > - /usr/share/gnu-smalltalk/unsupported/binary.c > rpmlint complains about this file as 'devel-file-in-non-devel-package'. > Would you explain why this file should be in main package? I'm going to remove this from the upstream package too. Sorry, but the Shebang fixing stuff is necessary anymore. Please see: /usr/share/gnu-smalltalk/Test.st (In reply to comment #50) > Sorry, but the Shebang fixing stuff is necessary anymore. > > Please see: /usr/share/gnu-smalltalk/Test.st Well, ------------------------------------------ [tasaka1@dhcp158 smalltalk-2.2c]$ for name in `find . -name \*.st` ; do if grep -q '@bindir@' $name ; then echo $name ; fi ; done ./scripts/Load.st ./scripts/Reload.st ./scripts/Test.st ------------------------------------------ Other files may contain broken shebang, so fixing is necessary, it is correct. So, please fix _only_ the files which requires to be fixed as I have commented in comment #48 and don't change other files. i.e. don't apply ------------------------------------------- sed 's|@bindir@|%{_bindir}|g' <$file >$file.new mv -f $file.new $file ------------------------------------------- to the files which don't need fixing to avoid timestamps change. These three files need not be installed; you can rm the three files identified by Mamoru Tasaka safely. It will be fixed upstream in 2.3. Next Release: SRPM: http://www.herr-schmitt.de/pub/gnu-smalltalk/gnu-smalltalk-2.2c-4.src.rpm SPEC: http://www.herr-schmitt.de/pub/gnu-smalltalk/gnu-smalltalk.spec Well, ------------------------------------------------------- E: gnu-smalltalk non-executable-script /usr/share/gnu-smalltalk/vfs/uar 0644 E: gnu-smalltalk non-executable-script /usr/share/gnu-smalltalk/examples/shell 0644 E: gnu-smalltalk non-executable-script /usr/share/gnu-smalltalk/vfs/mailfs 0644 E: gnu-smalltalk wrong-script-interpreter /usr/share/gnu-smalltalk/Test.st "@bindir@/gst" E: gnu-smalltalk non-executable-script /usr/share/gnu-smalltalk/Test.st 0644 E: gnu-smalltalk non-executable-script /usr/share/gnu-smalltalk/examples/pepe.sed 0644 E: gnu-smalltalk non-executable-script /usr/share/gnu-smalltalk/vfs/uzip 0644 E: gnu-smalltalk non-executable-script /usr/share/gnu-smalltalk/vfs/utar 0644 E: gnu-smalltalk non-executable-script /usr/share/gnu-smalltalk/vfs/urar 0644 E: gnu-smalltalk wrong-script-interpreter /usr/share/gnu-smalltalk/Load.st "@bindir@/gst" E: gnu-smalltalk non-executable-script /usr/share/gnu-smalltalk/Load.st 0644 W: gnu-smalltalk devel-file-in-non-devel-package /usr/share/gnu-smalltalk/unsupported/binary.c E: gnu-smalltalk wrong-script-interpreter /usr/share/gnu-smalltalk/Reload.st "@bindir@/gst" E: gnu-smalltalk non-executable-script /usr/share/gnu-smalltalk/Reload.st 0644 E: gnu-smalltalk non-executable-script /usr/share/gnu-smalltalk/vfs/patchfs 0644 E: gnu-smalltalk non-executable-script /usr/share/gnu-smalltalk/vfs/deb 0644 E: gnu-smalltalk non-executable-script /usr/share/gnu-smalltalk/vfs/ucpio 0644 E: gnu-smalltalk non-executable-script /usr/share/gnu-smalltalk/examples/xml.sed 0644 E: gnu-smalltalk non-executable-script /usr/share/gnu-smalltalk/vfs/uzoo 0644 E: gnu-smalltalk non-executable-script /usr/share/gnu-smalltalk/vfs/lslR 0644 E: gnu-smalltalk non-executable-script /usr/share/gnu-smalltalk/vfs/ulha 0644 ----------------------------------------------------------------------------- * _Permission_ fix is necessary (and permission fix won't change _timestamp_) ----------------------------------------------------------------- rm -rf $RPM_BUILD_ROOT/{_datadir}/gnu-smalltalk/Load.st ----------------------------------------------------------------- should be: ----------------------------------------------------------------- rm -rf $RPM_BUILD_ROOT/%{_datadir}/gnu-smalltalk/Load.st ----------------------------------------------------------------- > E: gnu-smalltalk non-executable-script /usr/share/gnu-smalltalk/vfs/uar 0644
I fixed this upstream.
Jochen, you may want to wait for the final 2.3 release at this point.
Now here the RPMs for the new final 2.3 release of gnu-smalltalk: SRPM: http://www.herr-schmitt.de/pub/gnu-smalltalk/gnu-smalltalk-2.3-1.src.rpm SPEC: http://www.herr-schmitt.de/pub/gnu-smalltalk/gnu-smalltalk.spec Well, again reviewing this package: From http://fedoraproject.org/wiki/Packaging/Guidelines : * Requires: - automake requirement Well, that I wrote that 'automake' is needed for Requires is for -devel package, not -emacs package, because -devel package includes .m4 files, while -emacs packages does not. - pkgconfig requirement This is also for -devel package. * Timestamps - Keeping timestamps is not treated completely. -------------------------------------------------------------- make install-data-hook make[3]: Entering directory `/builddir/build/BUILD/smalltalk-2.3' rm -f /var/tmp/gnu-smalltalk-2.3-root-mockbuild/usr/share/gnu-smalltalk/packages.xml ./gst-package --srcdir="." --no-load --destdir="/var/tmp/gnu-smalltalk-2.3-root-mockbuild" packages.xml Merging packages.xml into /var/tmp/gnu-smalltalk-2.3-root-mockbuild/usr/share/gnu-smalltalk/packages.xml... mkdir /var/tmp/gnu-smalltalk-2.3-root-mockbuild/usr/share/gnu-smalltalk/browser /usr/bin/install -c -m 644 browser/test.st /var/tmp/gnu-smalltalk-2.3-root-mockbuild/usr/share/gnu-smalltalk/browser/test.st /usr/bin/install -c -m 644 browser/ChangeLog /var/tmp/gnu-smalltalk-2.3-root-mockbuild/usr/share/gnu-smalltalk/browser/Chang eLog mkdir /var/tmp/gnu-smalltalk-2.3-root-mockbuild/usr/share/gnu-smalltalk/gtk /usr/bin/install -c -m 644 gtk/Structs.st /var/tmp/gnu-smalltalk-2.3-root-mockbuild/usr/share/gnu-smalltalk/gtk/Structs.st /usr/bin/install -c -m 644 gtk/Enums.st /var/tmp/gnu-smalltalk-2.3-root-mockbuild/usr/share/gnu-smalltalk/gtk/Enums.st /usr/bin/install -c -m 644 gtk/Funcs.st /var/tmp/gnu-smalltalk-2.3-root-mockbuild/usr/share/gnu-smalltalk/gtk/Funcs.st ...... -------------------------------------------------------------- Here 'install -c' is used, not 'install -c -p'. This is due to './gst-package' content. Fix this file so that timestamps are correctly kept. Umm.. I think that this file is a bit strange and somewhat incorrect. Why does this file echo: ----------------------------------------------------------------- echo "/usr/bin/install -c -m 755 $file $dest" ----------------------------------------------------------------- which prints out 'install' but actually do: ----------------------------------------------------------------- $dry_run || (rm -f "$dest" && cp "$file" "$dest" && chmod 755 "$dest") ----------------------------------------------------------------- not using install but using cp? This script is wrong when when we set @INSTALL@ environment to others such as 'install -p' (which I strongly recommend) by --------------------------------------------------------------- export INSTALL='install -p' %configure .... --------------------------------------------------------------- This case gst-package prints out the message as if this script keep timestamps (by 'install -p') , while the actual behaviour of this file is not to keep timestamps (by executing 'cp'). The above complaints should be fixed: SRPM: http://www.herr-schmitt.de/pub/gnu-smalltalk/gnu-smalltalk-2.3-2.src.rpm SPEC: http://www.herr-schmitt.de/pub/gnu-smalltalk/gnu-smalltalk.spec Moving to final check. For 2.3-2: * gst-config - "gst-config --libs" returns a wrong result (I should have noticed this...) -------------------------------------------- [tasaka1@localhost ~]$ gst-config --libs -L/usr/lib /usr/lib/gnu-smalltalk -lnsl -lm -lgst -------------------------------------------- This should be: -L/usr/lib -L/usr/lib/gnu-smalltalk ..... Fix the issue above. I will approve this package after that. Should be fixed: SRPM: http://www.herr-schmitt.de/pub/gnu-smalltalk/gnu-smalltalk-2.3-3.src.rpm SPEC: http://www.herr-schmitt.de/pub/gnu-smalltalk/gnu-smalltalk.spec Okay!! ------------------------------------------------ This package (gnu-smalltalk) is APPROVED by me. Paolo, Jochen tried to rebuild 2.3-3, however, it failed with x86_64. Testing intmath.st fails with segv. Would you check this problem? Reference: http://buildsys.fedoraproject.org/logs/fedora-development-extras/23119-gnu-smalltalk-2.3-3.fc7/x86_64/build.log Well, * If rebuilding this package has already succeeded (for at least one arch), please close this bug as CLOSED NEXTRELEASE. * If rebuilding this package fails for a arch (for example, on x86_64) * disable x86_64 arch * open a new bugzilla entry which tells that this package does not build on x86_64 and make it block Bug 179259 * then close this bug as CLOSED NEXTRELEASE . Becouse build fails on x86_64, I have open Bug #219096. Becouse build fails on x86_64, I have open Bug #219096. Package Change Request ====================== Package: gnu-smalltalk Updated Fedora Ownders: Jochen,lxtnow Package Change Request ====================== Package Name: gnu-smalltalk New Branches: EL-4 EL-5 cvs done. |