[Bug 214124] Review Request: bogl - a graphics library and an Unicode terminal emulator

Thu Nov 9 23:23:13 UTC 2006

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.

Summary: Review Request: bogl - a graphics library and an Unicode terminal emulator

https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=214124

------- Additional Comments From mitr at redhat.com  2006-11-09 18:23 EST -------
Thanks for the comments!

(In reply to comment #1)
> * rpmlint is not silent. 
[SNIP]
> I can see from the comment in the specthat there really aren't any URL
> presently, but I think that one should be provided/created.
packages.debian.org URL added.

> * There is no use of the %find_lang macro in the spec. There are no locale files
> so maybe this is not needed anyway?
Exactly.

> * /usr/share/bogl is not owned by any package (use %dir)
AFAICS this directory is owned by bogl-bterm.

> * /usr/include/bogl is not owned by any package (use %dir)
... and this one by bogl-devel.

> * You are using: "Requires: bogl = %{epoch}:%{version}-%{release}".
>   Why not: "Requires: %{name} = %{version}-%{release}" ? Se also my comment
> about the epoch tag below.
Because the package does have Epoch: 0.  Even if 0 behaves exactly the same as
"not present" (which I'm not sure is true), it seems better not to rely on this.

> From http://fedoraproject.org/wiki/Packaging/Guidelines:
> * Timestamps: Consider using "install -p" and "cp -p". You could use
> INSTALL="install -c -p" in your make install command
Changed for the header files, the other files are created during the build.

> Other issues:
> * I can't see any resaon why you need to use the epoch tag. Version numbers like
> 0.1.18 are not hard for RPM to parse at all. See e.g. here:
>
http://www.rpm.org/max-rpm-snapshot/s1-rpm-inside-tags.html#S3-RPM-INSIDE-REQUIRES-TAG
The epoch is already in the old Fedora / RHEL packages, and
http://fedoraproject.org/wiki/Tools/RPM/VersionComparison seems to say removing
the epoch could break upgrades.

> * There is no COPYING file in the top-level source directory. There should be.
Too bad.

I have added the debian/copyright files, although I don't think they can really
replace the licence.

> * The *.bdf files are not under the GPL, but they appear to be free enough

> * bogl does not use autoconf/automake. I really do find that the old Makefile
> way is to inflexible.
This should be fixed upstream, not in Fedora packaging.

> * Please use "Release: 12%{?dist}" not "Release: 12"
AFAIK the dist tag is not mandatory, and I'd rather not use it for the main
branch if possible.

> * There is a lot of compile warnings. These warning should be reviewd for
> seriousness. I know this is just me being overly strict, but I would prefer the
> Werror compile flag to be mandatory for all F[C,E] packages.
I have reviewed them about a year ago, and IIRC all the remaining warnings are
harmless.

> - I recommend using %_datadir instead of /usr/share.
Both /usr/share/terminfo and /usr/share/bogl are hardcoded in the bogl
installation scripts and the source, respectively, so the spec file should use
/usr/share as well.

> - fedora specific compilation flags are not passed.
Fixed.

> - Well, Japanese people (including me) always complain about bterm
>   as this software (bterm) is not useful for non-root users because
>   *it seems* bterm requires device access right to /dev/tty0 .
>  (I have not checked the whole source code of bterm and perhaps it is
>   impossible for me).
It fails on opening /dev/tty0, but even if that were removed, bogl needs root
privileges anyway to drive the VGA hardware.  I have looked at the kernel code a
bit and I couldn't find any alternative, although I'm not very familiar with the
framebuffer API.

I'd prefer leaving the program as is, I don't think it was audited for running
by hostile users.

http://people.redhat.com/mitr/extras/bogl-0.1.18-13.src.rpm :
* Fri Nov 10 2006 Miloslav Trmac <mitr at redhat.com> - 0:0.1.18-13
- Add URL:
- Preserve modification date of header files
- Ship debian/copyright
- Compile all files with $RPM_OPT_FLAGS

-- 
Configure bugmail: https://bugzilla.redhat.com/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug, or are watching the QA contact.