[GRASS-dev] Windows-style Pathnames

[Paul Kelly]

While experimenting with native Windows GRASS I was having loads of 
problems getting anything to work at all because the GISDBASE path read 
from the GISRC file was being corrupted in some strange way. Eventually 
tracked it down to lots of MinGW-specific hacks in lib/gis/env.c. When I 
removed them all everything (that I've tried so far: text-based startup 
and quite a few different modules) worked fine. Really! I've included the 
patch that removes them below at the bottom of this e-mail.

It appears to contain lots of stuff about prepending and stripping the 
hardcoded path to an Msys installation - what is that all about? Surely:
1) GRASS may be installed other places than under the Msys root
2) The correct solution is to pass the correct Windows-style path to 
whichever functions need it, rather than hacking about with it in the GIS 
library functions.
OK maybe I've missed something but it really doesn't make sense to me and 
seems like (from what I can see) to be fixing the problem in the wrong 
place?

There's lots of other MinGW hacks and ifdefs all over the place that look 
very dubious to me. An example is this one in lib/gis/parser.c:

         error = 0 ;
         need_first_opt = 1 ;
         i = strlen(pgm_name = argv[0]) ;
         while (--i >= 0)
         {
#ifdef __MINGW32__
                 if (pgm_name[i] == '/' || pgm_name[i] == '\\')
#else
                 if (pgm_name[i] == '/')
#endif
                 {
                         pgm_name += i+1;
                         break;
                 }
         }

It looks to me like pgm_name[i] is the first character in an array, so how 
does it make sense to compare it to a two-character string? And if it is 
correct, why the #ifdef anyway? Why not do the logical or comparison on 
all platforms? I think the aim should be to try and find a way of doing 
things that works on all platforms, rather than putting in 
platform-specific bits like this. It is getting really annoying trying to 
debug things when you're never quite sure what seems to be happening is 
really happening, because there's a MinGW-specific hack in somewhere. 
Especially when (OK I'm generalising here but only because it's getting 
rather annoying!) that so many of the hacks seem to be rather bogus.

Any opinions/justifications/flames?

Paul

Index: env.c
===================================================================
RCS file: /home/grass/grassrepository/grass6/lib/gis/env.c,v
retrieving revision 1.15
diff -u -r1.15 env.c
--- env.c       2 Sep 2006 05:43:39 -0000       1.15
+++ env.c       1 Nov 2006 19:40:05 -0000
@@ -97,9 +97,6 @@
  read_env ( int loc )
  {
      char buf[200];
-#ifdef __MINGW32__
-    char tmp[200];
-#endif
      char *name;
      char *value;

@@ -125,31 +122,6 @@
             *value++ = 0;
             G_strip (name);
             G_strip (value);
-#ifdef __MINGW32__
-           /* We need to prepend the MSYS base path (C:/msys/1.0) to any
-            * internal path variables since these variables cannot be
-            * auto-transformed to win32 path by the MSYS shell. */
-           if (strcmp(name, "GISDBASE") == 0 && value[1] != ':')
-           {
-               char *p;
-
-               sprintf(tmp, "%s", getenv("WD"));
-               /* the default $WD = C:\msys\1.0\\bin\ */
-               for(p=tmp+strlen(tmp); --p>=tmp && *p=='\\';);
-               /* C:\msys\1.0\\bin */
-               for(; --p>=tmp && *p!='\\';);
-               /* C:\msys\1.0\\ */
-               for(; --p>=tmp && *p=='\\';);
-               /* C:\msys\1.0 */
-               *(p+1) = 0;
-               for(p=tmp; *p; p++)
-                   if(*p == '\\')
-                       *p = '/';
-               /* now tmp is "C:/msys/1.0" and value is "/posix/path" */
-               strcat(tmp, value);
-               value = tmp;
-           }
-#endif
             if (*name && *value)
                 set_env (name, value, loc);
         }
@@ -271,29 +243,7 @@
         for (n = 0; n < count; n++)
             if (env[n].name && env[n].value && env[n].loc == loc
             && (sscanf (env[n].value,"%1s", dummy) == 1))
-#ifdef __MINGW32__
-           /* Strip out the MSYS base path (C:/msys/1.0). */
-           {
-               char *value = env[n].value;
-               char tmp[200];
-
-               if (strcmp(env[n].name, "GISDBASE") == 0 && value[1] == 
':')
-               {
-                   char *p;
-
-                   sprintf(tmp, "%s", getenv("WD"));
-                   for(p=tmp+strlen(tmp); --p>=tmp && *p=='\\';);
-                   for(; --p>=tmp && *p!='\\';);
-                   for(; --p>=tmp && *p=='\\';);
-                   *(p+1) = 0;
-                   /* skip C:/msys/1.0 */
-                   value += p+1-tmp;
-               }
-               fprintf(fd,"%s: %s\n", env[n].name, value);
-           }
-#else
                 fprintf(fd,"%s: %s\n", env[n].name, env[n].value);
-#endif
         fclose (fd);
      }

@@ -324,11 +274,7 @@
         /* TODO: better place ? */
         read_env ( G_VAR_GISRC );

-#ifdef __MINGW32__
-       sprintf ( buf, "%s\\%s\\VAR", G_location_path(), G_mapset() );
-#else
         sprintf ( buf, "%s/%s/VAR", G_location_path(), G_mapset() );
-#endif
      }

      return fopen (buf, mode);