Spec URL: http://www.auroralinux.org/people/spot/review/ddd.spec SRPM URL: http://www.auroralinux.org/people/spot/review/ddd-3.3.11-8.fc6.src.rpm Description: The Data Display Debugger (DDD) is a popular GUI for command-line debuggers like GDB, DBX, JDB, WDB, XDB, the Perl debugger, and the Python debugger. DDD allows you to view source texts and provides an interactive graphical data display, in which data structures are displayed as graphs. You can use your mouse to dereference pointers or view structure contents, which are updated every time the program stops. DDD can debug programs written in Ada, C, C++, Chill, Fortran, Java, Modula, Pascal, Perl, and Python. DDD provides machine-level debugging; hypertext source navigation and lookup; breakpoint, watchpoint, backtrace, and history editors; array plots; undo and redo; preferences and settings editors; program execution in the terminal emulation window, debugging on a remote host, an on-line manual, extensive help on the Motif user interface, and a command-line interface with full editing, history and completion capabilities. This package is moving from Fedora Core to Fedora Extras.
Taking this review, will have feedback tonight.
just a couple of quickie: 1. Buildroot does not seems to be the "preferred" extras one 2. gcc-c++ should be safely removed from BRs
* desktop-file-install vendor should be fedora * many BR are allready indirectly required: lesstif-devel requires libXext-devel libXp-devel libXt-devel libXt-devel requires libX11-devel libSM-devel libSM-devel requires libICE-devel * is the .gz needed in the install info snippet? Wouldn't it be better without?
Fixed up in -9: New SRPM: http://www.auroralinux.org/people/spot/review/ddd-3.3.11-9.fc6.src.rpm New SPEC: http://www.auroralinux.org/people/spot/review/ddd.spec
Now missing a br on libtool. Otherwise passes rpmlint other than mixed tabs/spaces and info.gz files not being in utf8 format.
Added the BR: libtool Fixed the mixed tab spaces. Ignored the info.gz files. New SRPM: http://www.auroralinux.org/people/spot/review/ddd-3.3.11-10.fc6.src.rpm New SPEC: http://www.auroralinux.org/people/spot/review/ddd.spec
Ok, approved.
Icon related snippets are missing (and they are now mandatory unless I'm wrong).
Some issues with that package (some are blockers, other are minor but most are serious): * spec should be utf8 encoded * missing Requires for proper functionning: gdb, /usr/bin/xfontsel, xterm, gnuplot and an html viewer, I propose htmlview * html viewing (in Help->DDD WWW Page) is broken for me, since konqueror is used but incorrectly called. a possible fix would be to Requires: htmlview, and then make a patch such that in the installed Ddd file in /usr/share/ddd-3.3.11/ddd/Ddd there is Ddd*wwwCommand: \ htmlview @URL@ instead of the default broken list * desktop category X-Red-Hat-Extra should be removed * Many interesting files missing from %docs. * instead of patching configure.ac and rerunning autotools (and doing it implicitly, wich is bad) a diff to configure should be better. I attach a simple one. * It seems to me that defaults come from Ddd.in, currently installed at /usr/share/ddd-3.3.11/ddd/Ddd so the programs required by ddd should be detected by ./configure, and therefore in BuildRequires. gdb is a fallback, xterm is a fallback but the following are tested: xterm kterm dtterm hpterm xfontsel is the fallback. You should make sure that builds are reproducible even if additional packages are required for the buildrequires but at the same time you may also want to avoid unneeded buildrequires. You can even change the defaults, and even patch configure for their detection (and adapt Requires asked above accordingly). * /usr/share/ddd-3.3.11/ddd/Ddd is a config file. In the INSTALL file, however, unless I didn't understood it right, it is warned against letting it under the user control because it may change between version. I don't have a firm opinion on that subject. Maybe it could be left where it is and flagged with %config? maybe it could be moved to %_sysconfdir, but not to the app-default dir? (it seems to be find by ddd/resources.C: static string own_app_defaults_file = resolvePath(string(ddd_NAME) + "/" + app_class); and if not there the standard X app-default file is tried). Maybe the best thing would be to do a directory in sysconfdir with %{name}-%{version} and put Ddd in it, and change the code such that resolvePath isn'called and instead a modified version that looks in HOME, then in sysconfdir/ddd-3.3.11/. * it would be possible to BuildRequire readline-devel and pass --with flag to configure, I don't know the implications though. * --disable-dependency-tracking could be passed to configure I tried to have a look at theme stuff and vsl stuff, it didn't seemed to work for me, but I am not a ddd user. Maybe somebody who knows better ddd and vsl could have a look at what is required to have that part work properly.
Created attachment 137182 [details] modified xprint patch to patch onlt configure If this is applied, I think libtool don't need to be a BuldRequires anymore
OK, here's a new SRPM which fixes the following: - Enable readline support - disable-dependency-tracking - Mark Ddd as a config file (not very comfortable patching new config file behavor here) - add configure defaults to BuildRequires - add all program callout defaults to Requires - use icon cache scriptlet - rpmlint doesn't complain about spec not being utf-8 anymore (this was all thanks to Trond's last name having foreign characters back in 2003) - add lots of %doc files - use htmlview as default WWW viewer - don't use autotools, use new patch to prevent it - X-Red-Hat-Extra removed from desktop New SRPM: http://www.auroralinux.org/people/spot/review/ddd-3.3.11-11.fc6.src.rpm New SPEC: http://www.auroralinux.org/people/spot/review/ddd.spec
* The files %{_datadir}/%{name}-%{version}/COPYING and %{_datadir}/%{name}-%{version}/NEWS are needed at runtime, they shouldn't be rm even though they are duplicates. * I think that COPYING.LIB shouldn't be shipped since there is no file covered by the LGPL (if I'm not wrong). * I think that libtool isn't needed as BR now. * libXmu-devel and libXpm-devel are dependencies of libXaw-devel. apart from the non-UTF8 warnings for info files, there is an error: E: ddd file-in-usr-marked-as-conffile /usr/share/ddd-3.3.11/ddd/Ddd I agree with you that it is sufficient and although it would be better to have Ddd below %_sysconfdir, this is something for upstream, and certainly not a blocker. When I start ddd I have the following warnings on the console: Warning: Cannot convert string "-*-symbol-*-*-*-*-*-120-*-*-*-*-adobe-*" to type FontStruct (Annoyed? Try 'Edit->Preferences->General->Suppress X Warnings'!) Warning: XmStringGetNextComponent: unknown type 164801264 Warning: XmStringGetNextComponent: unknown type 164801312 Any idea on what cause them, and how harmfull the corresponding issues are? (in general I tend to ignore such warnings, they are most of the time caused by something else than the package).
-12 has: - got rid of duplicate doc file removal - got rid of COPYING.LIB from %doc - no need for libtool BR - no need for libXmu-devel, libXpm-devel BR Dunno about those console warnings, I see them too, but they don't seem harmful (ddd works fine with them), and the program offers a mechanism for suppressing them. New SRPM: http://www.auroralinux.org/people/spot/review/ddd-3.3.11-12.fc6.src.rpm New SPEC: http://www.auroralinux.org/people/spot/review/ddd.spec
Now there are some files missing from %files %{_datadir}/%{name}-%{version}/COPYING %{_datadir}/%{name}-%{version}/NEWS Otherwise things seems fine for me (I don't post a summary, but I checked the review items).
Built, thanks for the reviewing.