[GRASS-dev] Windows-style Pathnames

[Glynn Clements]

Paul Kelly wrote:

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

Quite possibly.

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

'\\' isn't a string, it's a character literal equal to 92 - the ASCII
code for the backslash character (character literals support sequences
such as '\n', so the backslash itself has to be preceded by a
backslash).

> And if it is 
> correct, why the #ifdef anyway? Why not do the logical or comparison on 
> all platforms?

A backslash isn't a directory separator on Unix, it's a legal (albeit
unlikely) character in filenames.

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

For the above case, there should probably be a
G_is_directory_separator() function which determines whether or not
its argument is a directory separator on the host platform.

There also needs to be a macro (e.g. DIRSEP) which expands to the
preferred separator. While Windows accepts both / and \ in a lot of
situations, there are some (notably command lines) where only \ works.

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

Windows isn't Unix. There will inevitably need to be some handling of
platform-specific behaviour.

However, it's far better to create a set of functions to handle such
issues than to deal with individual files separately.

> Index: env.c

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

This is bogus, and should go.

>              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

Ditto.

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

This is arguably correct, although possbily better formulated as e.g.:

	sprintf ( buf, "%s" DIRSEP "%s" DIRSEP "VAR", G_location_path(), G_mapset() );
or:
	sprintf ( buf, "%s%s%s%sVAR", G_location_path(), DIRSEP, G_mapset(), DIRSEP );

rather than using a platform test.

However, using / should work as an argument to fopen(). The issue is
more significant if paths are constructed and stored for later use
(e.g. in $GISRC). In that situation, we need to decide whether to:

1. create and store the pathname in the host's syntax, or
2. store it in Unix syntax and convert it as required.

Either option will involve a lot of changes. #1 is easier to
implement, but will probably need to be done in more places (it needs
to be done for every pathname which might need to be in host syntax at
some point). #2 is harder, but only needs to be performed in places
where host syntax is known to be necessary.

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