Bug 245561 - Review Request: ht - File editor/viewer/analyzer for executables
Summary: Review Request: ht - File editor/viewer/analyzer for executables
Keywords:
Status: CLOSED RAWHIDE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Till Maas
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2007-06-25 12:45 UTC by Andreas Bierfert
Modified: 2008-11-11 06:27 UTC (History)
4 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2008-11-11 06:27:48 UTC
Type: ---
Embargoed:
opensource: fedora-review+
dennis: fedora-cvs+


Attachments (Terms of Use)

Description Andreas Bierfert 2007-06-25 12:45:29 UTC
Spec URL: http://fedora.lowlatency.de/review/ht.spec
SRPM URL: http://fedora.lowlatency.de/review/ht-2.0.8-1.fc7.src.rpm
Description:
HT is a file editor/viewer/analyzer for executables. The goal is to combine
the low-level functionality of a debugger and the usability of IDEs. We plan
to implement all (hex-)editing features and support of the most important
file formats.

Comment 1 Jason Tibbitts 2007-06-29 04:00:19 UTC
This package conflicts with the tetex-tex4ht package, which contains a
/usr/bin/ht executable.

Comment 2 Jason Tibbitts 2007-07-28 02:56:29 UTC
Any response to that conflict?  I don't know whether this will still be an issue
with the proposed texlive packages; I guess you should check.  But any conflict
need to either be made explicit (with "Conflicts: tetex-tex4ht") or handled with
alternatives.

Comment 3 Andreas Bierfert 2007-07-28 07:29:28 UTC
Hm, must have missed your first comment. Sorry. I guess a Conflicts:
tetex-tex4ht would be sufficient because it is not really an alternative ...

Comment 4 Till Maas 2007-09-08 08:00:06 UTC
- version 2.0.10 is now released
- License should be GPLv2 not GPL
- Source0: should be http://download.sourceforge.net/hte/ht-%{version}.tar.bz2

TexLive seems not to obolete tetex-tex4ht

| - BR ruby, don't obsolete tetex-tex4ht
Source: http://people.redhat.com/jnovy/files/texlive/SPECS/texlive.spec



Comment 5 Patrice Dumas 2007-09-08 16:57:13 UTC
I am the tetex-tex4ht maintainer, and indeed using ht as a program
name is not a good idea. tetex-tex4ht in fact contains many 
executables, some with trivial names, I'll try to talk to 
upstream. In the mean time I don't know exactly what could be 
done. In debian tex4ht there are much less programs, but 
still ht.

This is also true for ht, upstream should try to find 
a more descriptive name. The %{_bindir} namespace has to be shared
among all the applications, more care should be taken.

Comment 6 Patrice Dumas 2007-11-28 10:12:38 UTC
In devel and F-8 I renamed ht to tex4ht-ht. I don't want to modify
the other releases, so just add a Conflict for older releases.

Comment 7 Andreas Bierfert 2007-11-28 12:36:51 UTC
In that case I guess it would be easiest to just build for >= f-8 to avoid
confusion. Thanks anyway. I will upload the new version asap.

Comment 8 Andreas Bierfert 2008-01-07 08:01:00 UTC
Here is an updated version:

http://fedora.lowlatency.de/review/ht-2.0.10-1.fc8.src.rpm

Comment 9 Till Maas 2008-07-04 22:52:19 UTC
I changed the version in the spec to 2.0.13 and the rpm built fine. Here is my
review:

- rpmlint: good enough: 
ht.i386: W: file-not-utf8 /usr/share/doc/ht-2.0.13/TODO
This should probably be reported to upstream because file says:
$ file TODO
TODO: Non-ISO extended-ASCII English text
- naming: ok
- license: ok, GPLv2, included
- builds on koji for dist-f10: ok
- no libraries / subpackages: ok
- not a gui tool, no .desktop needed: ok
- %install: ok
- %clean: ok
- Honoring %optflags: NOT OK:
build.log shows usage of -O3 -fomit-frame-pointer and the Fedora optflags are
not used
- BuildRequires: NOT OK:
It should BR: lzo-devel, because otherwise it uses a local copy of minilzo
instead of the Fedora lzo library

Comment 10 Till Maas 2008-07-13 17:41:00 UTC
- Source Tag: NOT OK:
I was wrong in comment #4, the correct sourceforge url is
downloads.sourceforge... not download.sourceforge (it needs to be in plural).

Comment 11 Rakesh Pandit 2008-09-03 14:49:18 UTC
@Andreas
Any updates here?
It will closed if no update within a week is provided.

Comment 12 Andreas Bierfert 2008-09-03 21:01:11 UTC
http://fedora.lowlatency.de/review/ht-2.0.14-1.fc9.src.rpm
http://fedora.lowlatency.de/review/ht.spec

I will contact upstream about the TODO file so I guess this is not a major blocker.

Comment 13 Till Maas 2008-11-06 21:04:07 UTC
Sorry for the delay. The %optflags are still not OK:

NOT OK:
There is still fomit-frame-pointer and -O3 in the compiler flags, e.g.:

if g++ -DHAVE_CONFIG_H -I. -I. -I. -I./analyser -I./asm -I./info -I./io/posix -I./io -I./output -I./eval -I.    -DNOMACROS -pipe -O3 -fomit-frame-pointer -Wall -fsigned-char -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 -Woverloaded-virtual -Wnon-virtual-dtor -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector --param=ssp-buffer-size=4 -m32 -march=i386 -mtune=generic -fasynchronous-unwind-tables -MT htcfg.o -MD -MP -MF ".deps/htcfg.Tpo" -c -o ht


[NEEDS WORK] rpmlint output:
ht.i386: W: file-not-utf8 /usr/share/doc/ht-2.0.14/TODO
The file should be converted to UTF, e.g. with BR: recode and this in %prep:
recode latin1..utf8 TODO
Or some iconv/touch lines.

ht-debuginfo.i386: E: non-standard-executable-perm /usr/src/debug/ht-2.0.14/endianess.cc 0775
ht-debuginfo.i386: E: script-without-shebang /usr/src/debug/ht-2.0.14/endianess.cc
endianess.cc is executable in the tarball, the group writable comes from my
setup. Imho should rpmbuild create sane permissions for debuginfo contents.
But you could also ask upstream to remove the executable bit from the source
file.


[OK] Spec in %{name}.spec format

[OK] license allowed: GPLv2
[OK] license matches shortname in License
[OK] license in tarball and included in %doc: COPYING

[OK] package is code or permissive content
{OK} patches sent to upstream and commented
[OK] Source0 is a working URL
{OK} Sourceforge URL is Source0: http://downloads.sourceforge.net/%{name}/%{name}-%{version}.tar.gz
[OK] Source0 matches Upstream:
b3721a82532ba24bbfbc7f812b218217  ht-2.0.14.tar.bz2

[OK] Package builds on all platforms:
http://koji.fedoraproject.org/koji/taskinfo?taskID=920089
[OK] BuildRequires are complete (mock builds)
(OK) No file dependencies outside of /etc /bin /sbin /usr/bin /usr/sbin 
[OK] %find_lang used for locales

[N/A] Every (sub)package containing libraries runs ldconfig
%post -p /sbin/ldconfig
%postun -p /sbin/ldconfig
[N/A] .h (header) files are in -devel subpackage
[N/A] .a (static libraries) are in -static subpackage
[N/A] contains .pc (pkgconfig) files and has Requires: pkgconfig
(N/A) .pc files are in -devel subpackage
[N/A] contains .so.X(.Y) files and .so is in -devel
[N/A] -devel subpackage has Requires: %{name} = %{version}-%{release}
[N/A] .la files (libtool) are not included


[N/A] Has GUI and includes %{name}.desktop
[N/A] .desktop file installed with desktop-file-install in %install

[OK] Prefix: /usr not used (not relocatable)

[OK] Owns all created directories
[OK] no duplicates in %files
[OK] %defattr(-,root,root,-) is in every %files section
[OK] Does not own files or dirs from other packages
[OK] included filenames are in UTF-8

[OK] %clean is rm -rf %{buildroot} or $RPM_BUILD_ROOT 
[OK] %install starts with rm -rf %{buildroot} or $RPM_BUILD_ROOT 

[OK] Consistent macro usage
[OK] large documentation is -doc subpackage
[OK] %doc does not affect runtime

{OK} no pre-built binaries (.a, .so*, executable)
{OK} well known BuildRoot
%{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)
{OK} PreReq not used
{OK} no duplication of system libraries
{OK} no rpath
{OK} Timestamps preserved with cp and install
{OK} Uses parallel make (%{?_smp_mflags})
{OK} Requires(pre,post) style notation not used
{OK} only writes to tmp /var/tmp $TMPDIR %{_tmppath} %{_builddir} (and %{buildroot} on %install and %clean)
{OK} no Conflicts
{OK} nothing installed in /srv
{OK} Changelog in allowed format
{OK} does not use Scriptlets
<OK> Sane Provides: and Requires:

{OK} Follows Naming Guidelines

Comment 14 Till Maas 2008-11-06 21:36:25 UTC
I studied the package a little and found this out:

Use this to disable the -O3 and -fomit-frame-pointer:

%configure --enable-maintainermode
make %{?_smp_mflags}

Beware, the --enable-maintainermode is chosen by upstream and not the well known --enable-maintainer-mode option that autotools provide, which is described here:
http://www.gnu.org/software/automake/manual/html_node/maintainer_002dmode.html

Maybe you can get upstream to choose a better name for this option. :-)

Given that the remaining change is very simple, I APPROVE this package, but please update the spec/srpm before import to make it build with the right compiler flags.  Thank you very much for packaging this. :-)

Comment 15 Andreas Bierfert 2008-11-06 22:20:39 UTC
Thanks for the review. I will adjust the package accordingly before the import. Since there will be branching also already with F-10 branch.


New Package CVS Request
=======================
Package Name: ht
Short Description: File editor/viewer/analyzer for executables
Owners: awjb
Branches: F-8 F-9 F-10

Comment 16 Dennis Gilmore 2008-11-07 02:24:31 UTC
CVS Done

Comment 17 Andreas Bierfert 2008-11-11 06:27:48 UTC
Thanks for the review. Imported and build.


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