Bug 168228

Summary: Review Request: gforth - Fast and portable implementation of the ANS Forth language
Product: [Fedora] Fedora Reporter: Gérard Milmeister <gemi>
Component: Package ReviewAssignee: Adrian Reber <adrian>
Status: CLOSED NEXTRELEASE QA Contact: David Lawrence <dkl>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: fedora-extras-list, vnasardinov
Target Milestone: ---   
Target Release: ---   
Hardware: All   
OS: Linux   
URL: http://math.ifi.unizh.ch/fedora/4/i386/SRPMS.gemi/gforth-0.6.2-3.src.rpm
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2005-09-15 08:45:44 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 Flags
adds a harmless comment to siteinit.fs thus making it non-empty
none
gforth.spec-siteinit.patch incorprates gforth-siteinit.patch into gforth.spec
none
ghosting the files in the triggers none

Description Gérard Milmeister 2005-09-13 17:53:41 UTC
Spec: http://math.ifi.unizh.ch/fedora/spec/gforth.spec
SRPM: http://math.ifi.unizh.ch/fedora/4/i386/SRPMS.gemi/gforth-0.6.2-3.src.rpm
Description:
Gforth is a fast and portable implementation of the ANS Forth
language. It works nicely with the Emacs editor, offers some nice
features such as input completion and history, backtraces, a
decompiler and a powerful locals facility, and it even has a
manual. Gforth combines traditional implementation techniques with
newer techniques for portability and performance performance: its
inner innerpreter is direct threaded with several optimizations, but
you can also use a traditional-style indirect threaded interpreter.

Comment 1 Vadim Nasardinov 2005-09-13 18:43:18 UTC
Although I'm not involved in the Fedora Extras process, I'd like to
put in a good word for Gérard's gforth SRPM.  I first looked at it a
few weeks ago when I picked up a copy of
http://thinking-forth.sourceforge.net/ and was looking for a free
Forth implementation packaged as an RPM that I could play with.
Gérard's SRPM built out of the box and the only two issues that I ran
into were the following:

  - The binary RPM built for x86_64 didn't work on my AMD64 box due to
    the fact that the Makefile variable "libdir" was being set to the
    incorrect value /usr/lib rather than /usr/lib64;

  - The Emacs forth-mode wasn't getting activated upon opening a Forth
    source file due to a typo in the spec file: the alist
    auto-mode-alist was being extended with the dotted cons 
    ("\\.gs$" . forth-mode) rather than the correct value of
    ("\\.fs$" . forth-mode).

Both of these issues have been corrected.

Gérard's latest SRPM works fine on my AMD64:

| $ curl -O http://math.ifi.unizh.ch/fedora/4/i386/SRPMS.gemi/gforth-0.6.2-3.src.rpm
| $ rpm -ivh gforth-0.6.2-3.src.rpm 
| $ cd /var/vadim/scratch/rpm-topdir/SPECS/
| $ rpmbuild -ba gforth.spec     
| $ sudo rpm -Uvh ../RPMS/x86_64/gforth{-,-debuginfo-}0.6.2-3.x86_64.rpm
| $ gforth
| Gforth 0.6.2, Copyright (C) 1995-2003 Free Software Foundation, Inc.
| Gforth comes with ABSOLUTELY NO WARRANTY; for details type `license'
| Type `bye' to exit
| 3 4 + 5 * .
| 3 4 + 5 * . 35  ok
| bye
| bye 

(3+4)*5 is, indeed 35.  The Emacs forth mode kicks in upon opening an
.fs file.  Info pages work.  The map page works.


Comment 2 Adrian Reber 2005-09-14 01:07:50 UTC
* builds in mock
* spec looks good
* clean installation and removal
* scripts look good and have the necessary requirements
* source matches upstream
* patches are sane

* rpmlint isn't quite happy:
E: gforth zero-length /usr/share/gforth/site-forth/siteinit.fs

This isn't really a big issue, but gforth seems to work even if the file is
deleted. So if the file isn't necessary rpmlint could be made happy by deleting it.

Maybe the *emacs files could be handled like in the fedora-rpmdevtools package,
because from my point of view the installation of these files is not really
necessary and the directories for the files are created and never removed.


Comment 3 Vadim Nasardinov 2005-09-14 13:38:48 UTC
Created attachment 118794 [details]
adds a harmless comment to siteinit.fs thus making it non-empty

Another option is to keep the siteinit.fs file but make it non-empty.
The attached patch accomplishes this by putting the following comment
into siteinit.fs at build time:

\ If you change this file, you need to recompile gforth.fi

I'm not sure if siteinit.fs is useful when gforth is packaged as an
RPM.  Here's what gforth-0.6.2/INSTALL has to say about it under the
section "Preloading installation-specific code":


 | If you want to have some installation-specific files loaded when
 | Gforth starts (e.g., an assembler for your processor), put commands
 | for loading them into
 | /usr/local/share/gforth/site-forth/siteinit.fs (if the commands
 | work for all architectures) or
 | /usr/local/lib/gforth/site-forth/siteinit.fs (for
 | architecture-specific commands);
 | /usr/local/lib/gforth/site-forth/siteinit.fs takes precedence if
 | both files are present (unless you change the search path). The
 | file names given above are the defaults; if you have changed the
 | prefix, you have to replace "/usr/local" in these names with your
 | prefix.
 | 
 | By default, the installation procedure creates an empty
 | /usr/local/share/gforth/site-forth/siteinit.fs if there is no such
 | file.
 | 
 | If you change the siteinit.fs file, you should run "make install"
 | again for the changes to take effect (Actually, the part of "make
 | install" starting with "rm gforth.fi" is sufficient).

As for the following x/emacs files:

    /usr/share/emacs/site-lisp/gforth.el
    /usr/share/emacs/site-lisp/site-start.d/gforth-init.el
    /usr/share/xemacs/site-packages/lisp/gforth.el
    /usr/share/xemacs/site-packages/lisp/site-start.d/gforth-init.el

I'd much rather you kept them in the gforth RPM rather than splitting
them off into a different package.  While it's annoying that
directories may be left behind upon uninstall, it's not a big deal in
the grand scheme of things.

Comment 4 Vadim Nasardinov 2005-09-14 13:42:22 UTC
Created attachment 118795 [details]
gforth.spec-siteinit.patch incorprates gforth-siteinit.patch into gforth.spec

Here's a patch against gforth.spec to incorporate
gforth-siteinit.patch included in attachment #118794 [details], comment #3

Comment 5 Gérard Milmeister 2005-09-14 17:16:03 UTC
Spec: http://math.ifi.unizh.ch/fedora/spec/gforth.spec
SRPM: http://math.ifi.unizh.ch/fedora/4/i386/SRPMS.gemi/gforth-0.6.2-4.src.rpm

* Wed Sep 14 2005 Gerard Milmeister <gemi> - 0.6.2-4
- use triggers to install (x)emacs files
- create not-empty siteinit.fs


Comment 6 Adrian Reber 2005-09-14 20:50:24 UTC
Created attachment 118816 [details]
ghosting the files in the triggers

I have made some changes to the spec file to be more like the
fedora-rpmdevtools and thus removing the "rm"s from the postun scritplet.
I am now a bit unsure what the best solution for *emacs files is. Triggers are
usually considered as evil and that is why I thought that it might be better
without them like it was before. On the other hand the current implementation
with the triggers looks really nice and does exactly what I expect it to do.

I leave it up to you what to do, but from my point of the view the package is
approved no matter how you do it.

Comment 7 Gérard Milmeister 2005-09-14 21:24:02 UTC
Spec: http://math.ifi.unizh.ch/fedora/spec/gforth.spec
* Wed Sep 14 2005 Gerard Milmeister <gemi> - 0.6.2-5
- changes to the trigger code


Comment 8 Adrian Reber 2005-09-14 22:09:00 UTC
* builds in mock
* rpmlint is almost happy (only warnings)
  W: gforth dangerous-command-in-%trigger ln
  W: gforth dangerous-command-in-%trigger ln
  W: gforth dangerous-command-in-%trigger rm
  W: gforth dangerous-command-in-%trigger rm
* clean installation and removal
* spec looks good
* source matches upstream

APPROVED