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 168228 - Review Request: gforth - Fast and portable implementation of the ANS Forth language
Summary: Review Request: gforth - Fast and portable implementation of the ANS Forth la...
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Adrian Reber
QA Contact: David Lawrence
URL: http://math.ifi.unizh.ch/fedora/4/i38...
Whiteboard:
Depends On:
Blocks: FE-ACCEPT
TreeView+ depends on / blocked
 
Reported: 2005-09-13 17:53 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: 2005-09-15 08:45:44 UTC
Type: ---
Embargoed:


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

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


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