[GRASS-dev] g.mapsets use of sleep()

Paul_Kelly · December 5, 2006, 9:40pm

Since $(VASKLIB) was removed from the g.mapsets Makefile I get the following error on Windows:
gcc -L/c/grass/grass6/dist.i686-pc-mingw32/lib -Wl,--export-dynamic,--enable-run
time-pseudo-reloc -L/c/grass/lib -DPACKAGE=\""grassmods"\" -o /c/grass/
grass6/dist.i686-pc-mingw32/bin/g.mapsets.exe OBJ.i686-pc-mingw32/main_cmd.o OBJ
.i686-pc-mingw32/dsply_path.o OBJ.i686-pc-mingw32/dsply_maps.o OBJ.i686-pc-mingw
32/get_maps.o /c/grass/grass6/lib/gis/OBJ.i686-pc-mingw32/fmode.o -lgrass_gis -l
grass_datetime -lxdr -liberty -lws2_32 -lz -lxdr -liberty -lws2_32 -l
z
OBJ.i686-pc-mingw32/main_cmd.o(.text+0x427): In function `main':
c:/grass/grass6/general/g.mapsets/main_cmd.c:128: undefined reference to `sleep'

collect2: ld returned 1 exit status
make: *** [/c/grass/grass6/dist.i686-pc-mingw32/bin/g.mapsets.exe] Error 1

Not at all sure what's the most elegant way of fixing it???

Paul

Glynn_Clements1 · December 6, 2006, 12:16am

Paul Kelly wrote:

Since $(VASKLIB) was removed from the g.mapsets Makefile I get the
following error on Windows:
gcc -L/c/grass/grass6/dist.i686-pc-mingw32/lib -Wl,--export-dynamic,--enable-run
time-pseudo-reloc -L/c/grass/lib -DPACKAGE=\""grassmods"\" -o /c/grass/
grass6/dist.i686-pc-mingw32/bin/g.mapsets.exe OBJ.i686-pc-mingw32/main_cmd.o OBJ
.i686-pc-mingw32/dsply_path.o OBJ.i686-pc-mingw32/dsply_maps.o OBJ.i686-pc-mingw
32/get_maps.o /c/grass/grass6/lib/gis/OBJ.i686-pc-mingw32/fmode.o -lgrass_gis -l
grass_datetime -lxdr -liberty -lws2_32 -lz -lxdr -liberty -lws2_32 -l
z
OBJ.i686-pc-mingw32/main_cmd.o(.text+0x427): In function `main':
c:/grass/grass6/general/g.mapsets/main_cmd.c:128: undefined reference to `sleep'

collect2: ld returned 1 exit status
make: *** [/c/grass/grass6/dist.i686-pc-mingw32/bin/g.mapsets.exe] Error 1

Not at all sure what's the most elegant way of fixing it???

- sprintf (command, "ls -C %s/%s 1>&2", G_gisdbase(), G_location());
+ sprintf (command, "ls -C %s/%s 1>&2 ; sleep 3", G_gisdbase(), G_location());
system (command);
- sleep (3);

?

OK, so it's not exactly "elegant", but assuming the existence of a
"sleep" command doesn't seem to be any more of a Unix-ism than
assuming the existence of an "ls" command.

Alternatively, use G_sleep() instead:

From lib/gis/sleep.c:

  unsigned int G_sleep (unsigned int seconds)
  {
  #ifdef __MINGW32__
      /* TODO: no sleep for now */
      return 0;
  #else /* __MINGW32__ */
      return sleep(seconds);
  #endif /* __MINGW32__ */
  }

On MinGW, it doesn't actually sleep, but at least you don't get a link
error.

--
Glynn Clements <glynn@gclements.plus.com>

H_B · December 6, 2006, 1:43am

Glynn Clements wrote:

Paul Kelly wrote:

> Since $(VASKLIB) was removed from the g.mapsets Makefile I get the
> following error on Windows:
> gcc -L/c/grass/grass6/dist.i686-pc-mingw32/lib -Wl,--export-dynamic,--enable-run
> time-pseudo-reloc -L/c/grass/lib -DPACKAGE=\""grassmods"\" -o /c/grass/
> grass6/dist.i686-pc-mingw32/bin/g.mapsets.exe OBJ.i686-pc-mingw32/main_cmd.o OBJ
> .i686-pc-mingw32/dsply_path.o OBJ.i686-pc-mingw32/dsply_maps.o OBJ.i686-pc-mingw
> 32/get_maps.o /c/grass/grass6/lib/gis/OBJ.i686-pc-mingw32/fmode.o -lgrass_gis -l
> grass_datetime -lxdr -liberty -lws2_32 -lz -lxdr -liberty -lws2_32 -l
> z
> OBJ.i686-pc-mingw32/main_cmd.o(.text+0x427): In function `main':
> c:/grass/grass6/general/g.mapsets/main_cmd.c:128: undefined reference to `sleep'
>
> collect2: ld returned 1 exit status
> make: *** [/c/grass/grass6/dist.i686-pc-mingw32/bin/g.mapsets.exe] Error 1
>
> Not at all sure what's the most elegant way of fixing it???

- sprintf (command, "ls -C %s/%s 1>&2", G_gisdbase(), G_location());
+ sprintf (command, "ls -C %s/%s 1>&2 ; sleep 3", G_gisdbase(), G_location());
system (command);
- sleep (3);

?

OK, so it's not exactly "elegant", but assuming the existence of a
"sleep" command doesn't seem to be any more of a Unix-ism than
assuming the existence of an "ls" command.

so #include <unistd.h> is no good in windows?

isn't the system("ls") going to be a problem as well?

Alternatively, use G_sleep() instead:

>>From lib/gis/sleep.c:

  unsigned int G_sleep (unsigned int seconds)
  {
  #ifdef __MINGW32__
      /* TODO: no sleep for now */
      return 0;
  #else /* __MINGW32__ */
      return sleep(seconds);
  #endif /* __MINGW32__ */
  }

On MinGW, it doesn't actually sleep, but at least you don't get a link
error.

AFAIR, Paul's error is exactly why Radim added G_sleep().

and finally, WRT g.mapsets, sleep isn't really needed*, so why not:
[*] not followed by a clear screen command.

- sleep(3);
- exit(1);
+ G_fatal_error("");

or

+ G_fatal_error("Available mapsets:\n%s", mapset_string);

Hamish

H_B · December 6, 2006, 2:14am

Glynn Clements wrote:

> OBJ.i686-pc-mingw32/main_cmd.o(.text+0x427): In function `main':
> c:/grass/grass6/general/g.mapsets/main_cmd.c:128: undefined reference to `sleep'
>
> collect2: ld returned 1 exit status
> make: *** [/c/grass/grass6/dist.i686-pc-mingw32/bin/g.mapsets.exe] Error 1
>
> Not at all sure what's the most elegant way of fixing it???

The sleep() is not needed here so I've removed them in CVS.

Still to be done- get rid of hardcoded `ls`:

the module already has two fns,
get_available_mapsets();
display_available_mapsets(0);

so use them instead of UNIXy `ls`. display_available_mapsets() would need
to be passed a new option for target: display_available_mapsets(0, stderr);

Hamish

Glynn_Clements1 · December 6, 2006, 5:12am

Hamish wrote:

> OK, so it's not exactly "elegant", but assuming the existence of a
> "sleep" command doesn't seem to be any more of a Unix-ism than
> assuming the existence of an "ls" command.

so #include <unistd.h> is no good in windows?

No; the header exists, but it doesn't declare sleep(). The <stdlib.h>
header declares _sleep(), although apparently this is deprecated (it
exists in the MSVCRT DLL, but isn't listed in the documentation index)
in favour of Sleep().

isn't the system("ls") going to be a problem as well?

Only if you don't have "ls"

A lot of the [G_]system and [G_]popen calls in GRASS (I count 105)
will be similarly problematic. Some will be calling GRASS programs,
but a lot use Unix commands.

--
Glynn Clements <glynn@gclements.plus.com>

H_B · December 6, 2006, 6:28am

Glynn Clements wrote:

> so #include <unistd.h> is no good in windows?

No; the header exists, but it doesn't declare sleep(). The <stdlib.h>
header declares _sleep(), although apparently this is deprecated (it
exists in the MSVCRT DLL, but isn't listed in the documentation index)
in favour of Sleep().

sleep() is POSIX.1. Bad Microsoft.

> isn't the system("ls") going to be a problem as well?

Only if you don't have "ls"

A lot of the [G_]system and [G_]popen calls in GRASS (I count 105)
will be similarly problematic. Some will be calling GRASS programs,
but a lot use Unix commands.

105 really isn't that many.

for the ones that are unix commands, the vast majority are just ls, cp,
mv, rm?

step 1: define portable tcl and C replacements (e.g. glob, unlink())
for the common ones. (eg with Paul's new G_convert_dirseps_to_host())

step 2: fix it for some high profile module and let folks know that's
the reference way to do it. Maybe add to SUBMITTING or a wiki page how
to do that well. Putting it in SUBMITTING makes sure that grass devels
should know about doing it, putting it in the wiki might help external
folks doing a web search.

step 3: we fix them as we find them. e.g. if we guess "ls" is the most
common unix-ism, AFAICT in the C code that is just 5 files for
[G_]system:

imagery/i.ortho.photo/libes/ls_cameras.c
imagery/i.ortho.photo/libes/ls_elev.c
lib/imagery/ls_groups.c

these two are really bad:

lib/fonts/for_grass/try.c: system ("cd fonts; ls *.hmp | sed 's/.hmp//' | fmt");
lib/fonts/for_grass/fontmap.c: system ("cd ../fonts; ls *.hmp | sed 's/.hmp//' | fmt");

if steps 1 & 2 above could be solved, I think step 3 could happen very
quickly. At least we could make a start with the libraries...

for popen, lib/gis/error.c and lib/gis/home.c (pwd) look like places to
start.

Hamish

Glynn_Clements1 · December 6, 2006, 6:53am

Hamish wrote:

> A lot of the [G_]system and [G_]popen calls in GRASS (I count 105)
> will be similarly problematic. Some will be calling GRASS programs,
> but a lot use Unix commands.

105 really isn't that many.

for the ones that are unix commands, the vast majority are just ls, cp,
mv, rm?

I'm not saying it's difficult, just a lot of donkey-work.

Although, there are a few non-trivial cases; e.g. replacing "rm -rf"
isn't simple, and getting it wrong can be quite disastrous. But we
already have a replacement in lib/init/clean_temp.c.

rename() is ANSI C, although (on Unix, at least) it only works if the
source and destination paths are on the same filesystem (partition),
otherwise you have to copy the file then delete the original.

Windows has CopyFile(); Unix doesn't have any equivalent (the notion
of copying is ambiguous when the destination exists).

step 1: define portable tcl and C replacements (e.g. glob, unlink())
for the common ones. (eg with Paul's new G_convert_dirseps_to_host())

remove() (ANSI C) is preferable to unlink() (POSIX) for files,
although Windows requires using rmdir() (POSIX) for directories.

MinGW provides an opendir/readdir/closedir implementation, or you can
use FindFirstFile/FindNextFile/FindClose.

these two are really bad:

lib/fonts/for_grass/try.c: system ("cd fonts; ls *.hmp | sed 's/.hmp//' | fmt");
lib/fonts/for_grass/fontmap.c: system ("cd ../fonts; ls *.hmp | sed 's/.hmp//' | fmt");

"try" is just a debugging utility. It doesn't get built as part of the
normal build process. Also, that section of fontmap.c appears to be
for debugging; it only gets called if you specify "ls" as an argument
to splitfont.

--
Glynn Clements <glynn@gclements.plus.com>

H_B · December 6, 2006, 12:27pm

Glynn Clements wrote:

I'm not saying it's difficult, just a lot of donkey-work.

well, this is for Windows after all.

Hamish

Dylan_Beaudette1 · December 6, 2006, 7:34pm

On Wednesday 06 December 2006 04:27, Hamish wrote:

Glynn Clements wrote:
> I'm not saying it's difficult, just a lot of donkey-work.

well, this is for Windows after all.

Hamish

_______________________________________________
grass-dev mailing list
grass-dev@grass.itc.it
http://grass.itc.it/mailman/listinfo/grass-dev

nice-

there should be a collection of these great snippets some where - something
akin to the r 'fortunes' package.

cheers,

--
Dylan Beaudette
Soils and Biogeochemistry Graduate Group
University of California at Davis
530.754.7341

Markus_Neteler1 · May 16, 2007, 8:58pm

[ MinGW compile errors for sleep() ]

Glynn Clements wrote:

...
... use G_sleep() instead:

From lib/gis/sleep.c:

  unsigned int G_sleep (unsigned int seconds)
  {
  #ifdef __MINGW32__
      /* TODO: no sleep for now */
      return 0;
  #else /* __MINGW32__ */
      return sleep(seconds);
  #endif /* __MINGW32__ */
  }

On MinGW, it doesn't actually sleep, but at least you don't get a link
error.

I have added there code for MinGW to use Sleep() and updated
all remaining sleep() to G_sleep().

Markus
--
View this message in context: http://www.nabble.com/g.mapsets-use-of-sleep()-tf2764621.html#a10654453
Sent from the Grass - Dev mailing list archive at Nabble.com.

mlennert · May 18, 2007, 11:58am

On 16/05/07 22:58, Markus Neteler wrote:

[ MinGW compile errors for sleep() ]

Glynn Clements wrote:

...
... use G_sleep() instead:

>From lib/gis/sleep.c:

  unsigned int G_sleep (unsigned int seconds)
  {
  #ifdef __MINGW32__
      /* TODO: no sleep for now */
      return 0; #else /* __MINGW32__ */
      return sleep(seconds);
  #endif /* __MINGW32__ */
  }

On MinGW, it doesn't actually sleep, but at least you don't get a link
error.

I have added there code for MinGW to use Sleep() and updated
all remaining sleep() to G_sleep().

The implementation you used does not work:

#ifdef __MINGW32__
return Sleep((seconds)*1000);
#else

The problem is that Sleep is void and thus does not give a value you can return (http://msdn2.microsoft.com/en-us/library/ms686298.aspx).

We need something like this:

#ifdef __MINGW32__
Sleep((seconds)*1000);
return 0;
#else

Just not sure: is 0 the correct value to return ? IIUC this is the value returned by the unistd.h version of sleep.

Moritz

Markus_Neteler1 · May 18, 2007, 1:54pm

Moritz Lennert wrote on 05/18/2007 01:58 PM:

On 16/05/07 22:58, Markus Neteler wrote:

[ MinGW compile errors for sleep() ]

Glynn Clements wrote:

...
... use G_sleep() instead:

>From lib/gis/sleep.c:

    unsigned int G_sleep (unsigned int seconds)
    {
    #ifdef __MINGW32__
        /* TODO: no sleep for now */
        return 0; #else /* __MINGW32__ */
        return sleep(seconds);
    #endif /* __MINGW32__ */
    }

On MinGW, it doesn't actually sleep, but at least you don't get a link
error.

I have added there code for MinGW to use Sleep() and updated
all remaining sleep() to G_sleep().

The implementation you used does not work:

#ifdef __MINGW32__
   return Sleep((seconds)*1000);
#else

The problem is that Sleep is void and thus does not give a value you
can return (http://msdn2.microsoft.com/en-us/library/ms686298.aspx).

We need something like this:

#ifdef __MINGW32__
   Sleep((seconds)*1000);
   return 0;
#else

I have fixed it like this for now.

Markus

Just not sure: is 0 the correct value to return ? IIUC this is the
value returned by the unistd.h version of sleep.

Moritz

------------------
ITC -> dall'1 marzo 2007 Fondazione Bruno Kessler
ITC -> since 1 March 2007 Fondazione Bruno Kessler
------------------

Glynn_Clements1 · May 18, 2007, 4:24pm

Moritz Lennert wrote:

Just not sure: is 0 the correct value to return ? IIUC this is the value
returned by the unistd.h version of sleep.

RETURN VALUE
Zero if the requested time has elapsed, or the number of seconds left
to sleep.

It will return non-zero if sleep() is interrupted by a signal before
the requested number of seconds has elapsed.

AFAICT, nothing in GRASS actually uses the return value, so you could
just re-define G_sleep() as void.

--
Glynn Clements <glynn@gclements.plus.com>

Glynn_Clements1 · May 18, 2007, 6:01pm

Glynn Clements wrote:

AFAICT, nothing in GRASS actually uses the return value, so you could
just re-define G_sleep() as void.

I've done this.

--
Glynn Clements <glynn@gclements.plus.com>