[GRASS-dev] Re: testing native winGRASS

[Paul Kelly]

Hello Moritz,
A few comments below.

On Mon, 19 Mar 2007, Moritz Lennert wrote:

>>> lib/gis/gishelp.c:55:	system(buffer) ;
>>> sprintf(buffer, "%%GRASS_PAGER%% %s", file) ; /*ifdef __MINGW32__*/
>>> sprintf(buffer, "$GRASS_PAGER %s", file) ;/*else*/
>> 
>> 	G_spawn(getenv("GRASS_PAGER"), getenv("GRASS_PAGER"), file, NULL);
>
> done, but I get:

Note that that now won't work on Windows (while it presumably did before!) 
because GRASS_PAGER is a shell built-in and we haven't done changed that 
yet...
Same goes for the other usages of GRASS_PAGER.

> I also see the following code in ls_groups.c (line 37ff):
>
> strcpy (buf, "cd ");
> G__file_name (buf+strlen(buf), element, "", G_mapset());
> strcat (buf, ";ls");
> if (!full) strcat (buf, " -C");
> if ((ls = popen (buf, "r")))
>
> Isn't this somewhat equivalent to a system() call ?

We can maybe do something with G_ls() or G__ls().

>>> lib/init/mke_loc.c:179:    system(buf);
>>> sprintf (buf, "echo %s >  \"%s/%s/%s/MYNAME\"", myname, gisdbase, 
>>> location_name, mapset);  &  G_convert_dirseps_to_host(buf);
>> 
>> 	sprintf(path, "%s/%s/%s/MYNAME", gisdbase, location_name, mapset);
>
> No need to run G_convert_dirseps_to_host(path) here ?

No because the fopen() function on Windows can handle Unix file separator 
characters OK.

>> 	fp = fopen(path, "w");
>> 	fputs(myname, fp);
>> 	fclose(fp);
>> 
>>> lib/proj/datum.c:300:                G_system(buff);
>>> sprintf(buff,"%s \"%s\" 1>&2", pager, 
>>> G_convert_dirseps_to_host(Tmp_file));
>> 
>> As for other pager uses above.
>
> replaced by
>
> G_spawn(getenv("GRASS_PAGER"), getenv("GRASS_PAGER"),
>    G_convert_dirseps_to_host(Tmp_file), NULL);

As well as not working on Windows as mentioned above, this will also be 
broken on Unix now for some circumstances as it will print the list of 
datum choices to stdout but print the prompts to stderr. So if you run 
something like
g.proj -wi epsg=29900 > wkt.prj
to write a WKT description of EPSG co-ordinate system 29900 to a text 
file, but want g.proj to interactively prompt you on the datum names, the 
list of datum names will be written into the file rather than printed to 
the screen with the other prompts.

> Again: is G_convert_dirseps_to_host(Tmp_file) necessary ?

Actually yes here - because you are passing a pathname to an external 
program and it needs to have native system separator characters.

>>> raster/r.average/main.c:94:    if ((stat = system(command)))
>>> sprintf (command, "%s -anC input=%s,%s fs=space > \"%s\"", STATS, 
>>> basemap->answer, covermap->answer, tempfile1); & #define STATS "r.stats"
>> 
>> 	sprintf(input_arg, "input=%s,%s", basemap->answer, covermap->answer);
>> 	G_spawn_ex(STATS, STATS, "-anC", input_arg, "fs=space", 
>> SF_REDIRECT_FILE, 1, O_WRONLY|O_CREAT, tempfile, NULL);
>> 
>>> raster/r.average/main.c:154:    stat = system(command);
>>> sprintf (command, "%s input=%s output=%s < \"%s\"", RECODE, 
>>> basemap->answer, outputmap->answer, tempfile2); & #define RECODE 
>>> "r.recode"
>> 
>> 	sprintf(input_arg, "input=%s", basemap->answer);
>> 	sprintf(output_arg, "output=%s", outputmap->answer);
>> 	G_spawn_ex(RECODE, RECODE, input_arg, output_arg, SF_REDIRECT_FILE, 
>> 0, O_RDONLY, tempfile2, NULL);
>
> Should we use G_spawn_ex even though it is not ported to Windows ?

Well seeing the point of this audit was to fix occurences of system() that 
were broken on Windows, and that this one (like a lot of them) as far as I 
can see isn't actually broken, but the change would break it, I should 
think not! ;) It could do with being tidied up obviously but needs a bit 
more work.

>>> raster/r.coin/inter.c:32:    G_system("clear");
>>> 
>>> raster/r.coin/inter.c:51:	G_system("clear");
>> 
>> 	G_spawn("clear", "clear", NULL);
>
> Should be G_clear_screen(). I guess I can just go ahead with this and we can 
> deal with the internals of G_clear_screen() later, or ?

I think so. This actually is a case of something that currently is broken 
on Windows and will be fixed by the change.

>>> raster/r.coin/inter.c:87:	G_system(command);
>>> sprintf(command,"$GRASS_PAGER %s",dumpname);
>> 
>> As for other pager uses above.

Still won't work on Windows but wouldn't have before anyway so nothing 
lost. If we get GRASS_PAGER working as a batch file we can get it working 
without having to revisit this though.

Paul