[GRASS-dev] Re: testing native winGRASS

[Paul Kelly]

On Mon, 19 Mar 2007, Moritz Lennert wrote:

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

Leave it as motivation I think - although that instance too also has 
prompts printing to stderr and pager output going to stdout so we're just 
keeping the same bug.

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

I thought we decided that terminal interaction was OK if the user 
specifically requested it, in this case by specifying the -i "interactive" 
flag. Looks like G_spawn_ex() with appropriate flags would solve the 
problem though, so it can be a motivation to develop that on Windows.

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

Well I see this point yes but I don't think we should do it to the extent 
that it breaks things that worked before. But looking at G_spawn_ex() for 
Windows is probably the best way out here.

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