[GRASS-dev] Re: testing native winGRASS

[Moritz Lennert]

On 19/03/07 01:02, Paul Kelly wrote:
> 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.

Ok, so, should I revert this, or should we leave it as a motivation to 
develop a GRASS_PAGER batch script ?

>>
>> 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.

So, should I revert or is there a better fix ? Unless we decide that any 
form of prompting is "illegal" in modules ?

> 
>>>> 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.

Glynn seems to argue that we should try to avoid any calls to a shell...

> 
>>>> 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.


At least some good ;-)


Moritz