Bug 168228 - Review Request: gforth - Fast and portable implementation of the ANS Forth language
Review Request: gforth - Fast and portable implementation of the ANS Forth la...
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Adrian Reber
David Lawrence
http://math.ifi.unizh.ch/fedora/4/i38...
:
Depends On:
Blocks: FE-ACCEPT
  Show dependency treegraph
 
Reported: 2005-09-13 13:53 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: 2005-09-15 04:45:44 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)
adds a harmless comment to siteinit.fs thus making it non-empty (817 bytes, patch)
2005-09-14 09:38 EDT, Vadim Nasardinov
no flags Details | Diff
gforth.spec-siteinit.patch incorprates gforth-siteinit.patch into gforth.spec (551 bytes, patch)
2005-09-14 09:42 EDT, Vadim Nasardinov
no flags Details | Diff
ghosting the files in the triggers (1002 bytes, patch)
2005-09-14 16:50 EDT, Adrian Reber
no flags Details | Diff

  None (edit)
Description Gérard Milmeister 2005-09-13 13:53:41 EDT
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 14:43:18 EDT
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-13 21:07:50 EDT
* 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 09:38:48 EDT
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 09:42:22 EDT
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 13:16:03 EDT
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@bluewin.ch> - 0.6.2-4
- use triggers to install (x)emacs files
- create not-empty siteinit.fs
Comment 6 Adrian Reber 2005-09-14 16:50:24 EDT
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 17:24:02 EDT
Spec: http://math.ifi.unizh.ch/fedora/spec/gforth.spec
* Wed Sep 14 2005 Gerard Milmeister <gemi@bluewin.ch> - 0.6.2-5
- changes to the trigger code
Comment 8 Adrian Reber 2005-09-14 18:09:00 EDT
* 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

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