Problems with paths in the TCL/TK Windows GUI

[marmai]

Within the tcltk-GUI it is not possible to select a map within the project: When clicking 'Add vector layer', then on the icon in the lower part of the GIS Manager (tcltk-GUI) 'vector map to display'. Within the following pop-up window there are no maps listed. With the 'g.list type=vect' command entered in the GIS.m window, all vector maps are listed as expected. And it is also possible to show the maps by entering the names of the maps in the 'Vector map'- field in the lower part of the GIS Manager. Just selecting with the icon is not possible.

Furthermore another path related problem with the tcltk-GUI: Using the 'Show attribute data' button within the 'GIS Manager' window the following command is sent to be executed:

db.select table=veg_buffer_1_dissolve database=D:\daten\GISDataBase/demolocation/PERMANENT/dbf/ driver=dbf

For this command I get the following error in the output:

BMI-DBF driver error: Cannot create dbf database: D:datenGISDataBase/demolocation/PERMANENT/dbf/ Unable to open database <D:datenGISDataBase/demolocation/PERMANENT/dbf/>

Notice: the \ are gone!

By correcting the command manually (backslash to slash) to: db.select table=veg_buffer_1_dissolve database=D:/daten/GISDataBase/demolocation/PERMANENT/dbf/ driver=dbf everything works fine.

All was tested with the GRASS6.4.0RC3 and GRASS6.4svn-2 (OSGeo4W installer)

[hamish]

Replying to marmai:

Within the tcltk-GUI it is not possible to select a map within the project: When clicking 'Add vector layer', then on the icon in the lower part of the GIS Manager (tcltk-GUI) 'vector map to display'. Within the following pop-up window there are no maps listed. With the 'g.list type=vect' command entered in the GIS.m window, all vector maps are listed as expected. And it is also possible to show the maps by entering the names of the maps in the 'Vector map'- field in the lower part of the GIS Manager. Just selecting with the icon is not possible.

confirmed with the latest wingrass installer (Colin's stand alone version is much newer than the current osgeo4w build)

this is a close relative of bug #593.

Furthermore another path related problem with the tcltk-GUI: Using the 'Show attribute data' button within the 'GIS Manager' window the following command is sent to be executed:

db.select table=veg_buffer_1_dissolve database=D:\daten\GISDataBase/demolocation/PERMANENT/dbf/ driver=dbf

For this command I get the following error in the output:

BMI-DBF driver error: Cannot create dbf database: D:datenGISDataBase/demolocation/PERMANENT/dbf/ Unable to open database <D:datenGISDataBase/demolocation/PERMANENT/dbf/>

Notice: the \ are gone!

By correcting the command manually (backslash to slash) to: db.select table=veg_buffer_1_dissolve database=D:/daten/GISDataBase/demolocation/PERMANENT/dbf/ driver=dbf everything works fine.

this is slightly different, but probably a close relative of error seen in ​https://trac.osgeo.org/grass/ticket/629#comment:5

All was tested with the GRASS6.4.0RC3 and GRASS6.4svn-2 (OSGeo4W installer)

please could you try with the latest stand-alone installer? a lot has been fixed there. (you can have both installed without conflicts)

Hamish

[hamish]

probably G_convert_dirseps_to_host() is needed somewhere in the code.

[glynn]

Replying to hamish:

probably G_convert_dirseps_to_host() is needed somewhere in the code.

Did you mean G_convert_dirseps_from_host()?

It's only necessary to convert / to \ if the path is being passed to Windows' command interpreter. The rest of the OS understands both / and \.

OTOH, some GRASS functions only recognise /, so any \ characters need to be converted.

[marmai]

I just tested with the latest stand-alone version (wingrass-6.4.0SVN-r37703-1-setup.exe). Here everything works fine on my W2k system, so probably everything is already fixed. Thanks a lot.

[hamish]

Replying to glynn:

Did you mean G_convert_dirseps_from_host()?

umm, yeah.

It's only necessary to convert / to \ if the path is being passed to Windows' command interpreter. The rest of the OS understands both / and \.

Ok, so the code we need to check then is the bit that launches e.g. dbf.exe child from lib/db/dbmi_client/start.c ???

OTOH, some GRASS functions only recognise /, so any \ characters need to be converted.

Hamish

[glynn]

Replying to hamish:

It's only necessary to convert / to \ if the path is being passed to Windows' command interpreter. The rest of the OS understands both / and \.

Ok, so the code we need to check then is the bit that launches e.g. dbf.exe child from lib/db/dbmi_client/start.c ???

I don't think so. The backslashes are getting lost. Windows' command interpreter doesn't interpret backslashes, so this looks more like Tcl's doing. It looks like something is (implicitly or explicitly) using "eval" or "subst" on the string, causing backslash-<char> to be replaced with <char>.

[hamish]

Replying to hamish:

Ok, so the code we need to check then is the bit that launches e.g. dbf.exe child from lib/db/dbmi_client/start.c ???

Replying to glynn:

I don't think so. The backslashes are getting lost.

(comment:4 of this ticket reports that fixed now)

Windows' command interpreter doesn't interpret backslashes, so this looks more like Tcl's doing. It looks like something is (implicitly or explicitly) using "eval" or "subst" on the string, causing backslash-<char> to be replaced with <char>.

maybe fixed by the new eval [list ] stuff?

anyway I was more thinking about the Bad file descriptor problems of unquoted spaces from ​https://trac.osgeo.org/grass/ticket/629#comment:5, not the missing '\'s.

Hamish

[hellik]

Replying to hamish:

Replying to hamish:

Ok, so the code we need to check then is the bit that launches e.g. dbf.exe child from lib/db/dbmi_client/start.c ???

Replying to glynn:

I don't think so. The backslashes are getting lost.

(comment:4 of this ticket reports that fixed now)

Windows' command interpreter doesn't interpret backslashes, so this looks more like Tcl's doing. It looks like something is (implicitly or explicitly) using "eval" or "subst" on the string, causing backslash-<char> to be replaced with <char>.

maybe fixed by the new eval [list ] stuff?

anyway I was more thinking about the Bad file descriptor problems of unquoted spaces from ​https://trac.osgeo.org/grass/ticket/629#comment:5, not the missing '\'s.

Hamish

WinVista 32 Grass65-svn - self-compiled today in the osgeo4w-tree nc-dataset

i can confirm the problem i'm not able to select a vector/raster map in the tcltk-gui, but it's possible to enter the name for displaying the maps

helli

[marisn]

If I didn't broke anything, this issue is fixed in 6.5 r38296

Please test. Candidate for 6.4.0.

[hellik]

Replying to marisn:

If I didn't broke anything, this issue is fixed in 6.5 r38296

Please test. Candidate for 6.4.0.

WinVista32 grass65-svn self compiled at rev38296 in osgeo4w-tree nc_spm_08-dataset

in the bug-report, there seems to be two issues

(1) selecting raster/vector within the gui: => is working

(2) showing attribute data

db.select table=boundary_county database=C:/gisdata/grassdata/nc_spm_08/PERMANENT/dbf driver=dbf

at|AREA|PERIMETER|FIPS|NAME|NAME_LOCAS|DOT_DISTRI|DOT_DIVISI|DOT_COUNTY|COUNTY_100|DOT_GROUP_|ACRES|ABBR_5CHAR|ABBR_4CHAR|ABBR_2CHAR|Z_MEAN|Z_MIN|Z_MAX|Z_ZONE|CO_CENSUS|DIV_CONTAC|DIST_CONTA|CO_WIKIPED|Shape_Leng|Shape_Area 1|11920322825.1|521691.436874|9|ASHE|Ashe|3|11|4|5|ASHE,WILK|273888|ASHE|ASHE|AS|3203|2136|5169|5000|​http://quickfacts.census.gov/qfd/states/37/37009.html|http://apps01.dot.state. [...]

=> attribute data is shown

so both issues seem to be fixed

Helli

[hamish]

Replying to marisn:

If I didn't broke anything, this issue is fixed in 6.5 r38296

Please test. Candidate for 6.4.0.

Hi,

I have reviewed the patch, it looks good. Thanks!

One thing I notice though - gis.m/vector.tcl line ~ 273 (as seen in r38296 diff):

   if {![catch {open "|v.db.connect map=$mapname layer=$layernum -g" r} vdb]} {
...
   set db [file normalize [join [lrange $vdblist 3 end-1]]]

example output of v.db.connect -g:

v.db.connect map=archsites@PERMANENT layer=1 -g fs='|'
1|archsites|cat|/home/hamish/grassdata/spearfish60/PERMANENT/dbf/|dbf

instead of [lrange ... 3 end-1] to catch the spaces in pathnames it might be cleaner to use the new fs='|' option to v.db.connect. Or would you just have to convert any fs= to spaces to please Tcl's list logic, so may as well stick with the status quo?

your thoughts?

grep shows that chart.tcl and thematic.tcl also use v.db.connect -g with the same default space separator method & so will also fail with spaces in the path name.

and source:grass/trunk/gui/wxpython/gui_modules/sqlbuilder.py@#L46 may suffer a similar bug for DB paths containing spaces.

also, crossing over to the other bug ticket for a sec, is there any reason not to "quote" that $path in lib/gtcltk/select.tcl? even if it seems to work regardless, to be safe.

otherwise your fixes seem ready to backport to 6.4 & I'll do that soon.

regards, Hamish

[glynn]

Replying to hamish:

instead of [lrange ... 3 end-1] to catch the spaces in pathnames it might be cleaner to use the new fs='|' option to v.db.connect. Or would you just have to convert any fs= to spaces to please Tcl's list logic, so may as well stick with the status quo?

fs=| should be passed to v.db.connect, to more robustly distinguish field separators from spaces in the filename field.

Tcl lists should be constructed using Tcl's list operators: list, lappend, concat, etc, not by using string operators and textual substitution and hoping that you got the syntax right.

also, crossing over to the other bug ticket for a sec, is there any reason not to "quote" that $path in lib/gtcltk/select.tcl? even if it seems to work regardless, to be safe.

Tcl isn't Bourne shell; variable substitutions aren't re-parsed, and don't need to be quoted. $foo is always one word, regardless of whether it contains spaces. You only need quotes for literal strings which contain spaces.

[marisn]

Replying to hamish:

instead of [lrange ... 3 end-1] to catch the spaces in pathnames it might be cleaner to use the new fs='|' option to v.db.connect. Or would you just have to convert any fs= to spaces to please Tcl's list logic, so may as well stick with the status quo?

your thoughts?

It may look cleaner at first, but it still will require some cleaver string to list convertation, as on some (insane) system there may be some path with any fs in path and thus broke plain string parsing by fs parameter. lrange approach isn't beautiful, but it works.

grep shows that chart.tcl and thematic.tcl also use v.db.connect -g with the same default space separator method & so will also fail with spaces in the path name.

My fault - I didn't grep all gis.m source for similar bugs.

otherwise your fixes seem ready to backport to 6.4 & I'll do that soon.

regards, Hamish

Nice to hear that one of 6.4 stoppers seems to be solved :)

[glynn]

Replying to marisn:

your thoughts?

It may look cleaner at first, but it still will require some cleaver string to list convertation, as on some (insane) system there may be some path with any fs in path and thus broke plain string parsing by fs parameter.

This isn't worth worrying about. There are always cases which don't work; you could have a newline in the pathname, which will break the code which splits the file into records before you even get to worrying about splitting the records into fields.

We only need to worry about the ones which might actually happen in reality. Otherwise, we would have to quote/escape the field separator (and also quote/escape the quote/escape characters), then change everything which parses the output accordingly. On Unix, any character except NUL can occur in a pathname.

I suppose that we could add support for fs=nul (analogous to "find -print0" for use with "xargs -0"); that should be parseable in Python, but I wouldn't be too sure about other languages (bash doesn't seem to like it). This would have to be implemented by the module; you can't have embedded NULs in argv[].

[hamish]

sqlbuilder.py in the wxGUI now updated to safely handles spaces in the DB path and spaces in the SQL query.

Hamish

[marisn]

Should be fixed in 6.4 as of r39149.

[hamish]

Replying to marisn:

Should be fixed in 6.4 as of r39149.

thanks,

[hamish]

tested & fixed with latest (WinGRASS-6.4.0SVN-r39271-1-Setup.exe) native installer.

still get _spawnl error if map is not in the current mapset (both wx and tcltk), but that's a different matter.

thanks, Hamish