[postgis-devel] State of shp2pgsql-gui in trunk

[Mark Cave-Ayland]

Hi folks,

Having just been applying fixes to shp2pgsql-gui in both trunk and 1.5, 
I am getting quite concerned about the state of shp2pgsql-gui in trunk. 
Here are a few of the issues I have noticed with current trunk:


- Multiple warnings at build-time

gcc -g -O2  -fPIC -DPIC  -Wall -Wmissing-prototypes 
-I/home/pg83/rel-8.3.7/include -D_REENTRANT -I/usr/include/gtk-2.0 
-I/usr/lib/gtk-2.0/include -I/usr/include/atk-1.0 -I/usr/include/cairo 
-I/usr/include/pango-1.0 -I/usr/include/glib-2.0 
-I/usr/lib/glib-2.0/include -I/usr/include/pixman-1 
-I/usr/include/freetype2 -I/usr/include/directfb -I/usr/include/libpng12 
    -o shp2pgsql-gui.o -c shp2pgsql-gui.c
shp2pgsql-gui.c:155: warning: no previous prototype for ‘pgui_log_va’
shp2pgsql-gui.c: In function ‘compare_columns’:
shp2pgsql-gui.c:1000: warning: enumeration value ‘FTInvalid’ not handled 
in switch
shp2pgsql-gui.c: In function ‘pgui_create_file_table’:
shp2pgsql-gui.c:2369: warning: unused variable ‘bin_window’
shp2pgsql-gui.c: At top level:
shp2pgsql-gui.c:1675: warning: ‘pgui_action_null’ defined but not used
shp2pgsql-gui.c: In function ‘pgui_action_shape_file_set’:
shp2pgsql-gui.c:1713: warning: ‘status’ may be used uninitialized in 
this function
shp2pgsql-gui.c: In function ‘pgui_action_handle_tree_edit’:
shp2pgsql-gui.c:666: warning: ‘status’ may be used uninitialized in this 
function


- Multiple GTK assertions triggered at run time

e.g.

(shp2pgsql-gui:29945): GLib-GObject-WARNING **: 
/build/buildd-glib2.0_2.22.4-1~bpo50+1-amd64-SJ3EDw/glib2.0-2.22.4/gobject/gsignal.c:2270: 
signal `changed' is invalid for instance `0x6becc0'

(shp2pgsql-gui:29945): GLib-GObject-WARNING **: IA__g_object_set_valist: 
object class `GtkCellRendererToggle' has no property named `editable'

(shp2pgsql-gui:29945): Gtk-CRITICAL **: gtk_list_store_get_value: 
assertion `column < list_store->n_columns' failed

(shp2pgsql-gui:29945): GLib-GObject-CRITICAL **: g_object_set_property: 
assertion `G_IS_VALUE (value)' failed

(shp2pgsql-gui:29945): GLib-GObject-CRITICAL **: g_value_unset: 
assertion `G_IS_VALUE (value)' failed


- Bitrot/regressions compared to 1.5 branch

It looks as if the merge into trunk has removed some of the later 
changes from the 1.5 branch, e.g. take a look at pgui_read_connection(). 
The version in trunk has now regressed to a much older version, and is 
totally different from the logic in the 1.5 branch. Does that mean that 
there are other instances of this from the merge?


- Test connection always displays "Connection Succeeded" even when it 
doesn't?!


- General code quality issues:

Random bits of code are commented out with no explanation, e.g 
shp2pgsql-gui lines 43-46, and lines 1176-1179. These sections need to 
either be fixed up or removed. Also what does the sqlform query do? It's 
not obvious and needs a comment. Some sections of code are still marked TBD.


To summarise, my feeling is that in its current state this code is not 
ready for the prime time, and the maintenance is going upwards due to 
trunk not being based on a recent version of 1.5 branch. I think someone 
needs to step up to fix/maintain this code ASAP or we should simply 
revert the changes and re-synchronise with the 1.5 branch :(


ATB,

Mark.

-- 
Mark Cave-Ayland - Senior Technical Architect
PostgreSQL - PostGIS
Sirius Corporation plc - control through freedom
http://www.siriusit.co.uk
t: +44 870 608 0063

Sirius Labs: http://www.siriusit.co.uk/labs