[GRASS-dev] [bug #4368] (grass) g.parser: pass through --o

parser.c now sets the OVERWRITE environment variable, but doesn't export it.
Scripts run by g.parser now have it set so that modules they run will be run
as if --o was included. It can be used here to skip the overwrite check that's
implemented in r.blend to check for .r .g and .b extensions to the map name.

-------------------------------------------- Managed by Request Tracker

Cedric Shock via RT wrote:

parser.c now sets the OVERWRITE environment variable, but doesn't export it.

It doesn't make sense to talk about "exporting" an environment
variable.

More accurately, parser.c now sets the OVERWRITE environment variable
if either the OVERWRITE GRASS variable is set or if the --o switch is
used. It doesn't modify the OVERWRITE GRASS variable.

Scripts run by g.parser now have it set so that modules they run will be run
as if --o was included.

Only if the scripts themselves check $OVERWRITE and pass --o to any
commands which they run. G_parser() itself doesn't pay any attention
to the OVERWRITE environment variable.

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

Glynn,

Cedric Shock via RT wrote:
> parser.c now sets the OVERWRITE environment variable, but doesn't export
> it.

In bug 4331 you wrote:

I would have thought that it would be sufficient for G_parser() to set
OVERWRITE itself if --o was used. That would then be inherited by the
script and by any modules which it calls.

Would this be G__setenv("OVERWRITE", "1") ? Or will something like:

r.patch --o ...
r.in.gdal output=map_that_already_exists ...

overwrite the map_that_already_exists because the "env" changed?

More accurately, parser.c now sets the OVERWRITE environment variable
if either the OVERWRITE GRASS variable is set or if the --o switch is
used. It doesn't modify the OVERWRITE GRASS variable.

Is the above how to modify this like you suggested?

--Cedric

Cedric Shock wrote:

> > parser.c now sets the OVERWRITE environment variable, but doesn't export
> > it.

In bug 4331 you wrote:

> I would have thought that it would be sufficient for G_parser() to set
> OVERWRITE itself if --o was used. That would then be inherited by the
> script and by any modules which it calls.

Sorry. When I wrote that, I was under the impression that OVERWRITE
was an environment variable.

Would this be G__setenv("OVERWRITE", "1") ?

That would work, but it's the wrong approach, as the change would be
persistent ...

Or will something like:

r.patch --o ...
r.in.gdal output=map_that_already_exists ...

overwrite the map_that_already_exists because the "env" changed?

Exactly.

> More accurately, parser.c now sets the OVERWRITE environment variable
> if either the OVERWRITE GRASS variable is set or if the --o switch is
> used. It doesn't modify the OVERWRITE GRASS variable.

Is the above how to modify this like you suggested?

What I would suggest is to change G_parser() to:

1. Set the "overwrite" C variable if any of the following are
true:

a) The --o switch is used.
b) The OVERWRITE GRASS variable is set.
c) The OVERWRITE environment variable is set

2. Set the OVERWRITE environment variable from the "overwrite" C
variable.

1a) provides a means for the user to enable overwriting for a single
command. 1b) provides a means for the user to enable overwriting in a
persistent manner. 1c) provides a mechanism to make the --o switch to
do the right thing for scripts (and C programs which invoke other
modules via system(); there are a few of those).

More generally, environment variables should be used for parameters
where you might want a different setting for individual commands. Each
process has its own set of environment variables, but there is only
one set of GRASS variables.

OTOH, GRASS variables should be used where you want to be able to
enable or disable them for the session as a whole from a command. You
can't do this with environment variables, as only the shell can change
its own environment.

In terms of confusion between the two types of variables, it doesn't
help that the libgis functions which manipulate GRASS variables have
"env" in their names and are defined in a file called env.c (and the
documentation used to refer to them as "environment variables"; maybe
some still does?).

One of the biggest issues with integrating GRASS modules into a
higher-level architecture (e.g. the GUI) is the extensive use of state
stored in files (and the rather ad-hoc way in which this is done).

While this is a good idea from the perspective of a command-line
session (you wouldn't want to have to type database=... location=...
mapset=... region=... for every command, along with monitor=... for
d.* commands), it's a nuisance if you want to be able to run multiple
commands concurrently (e.g. running commands from gis.m when the user
might also be running commands from the shell) or if you want multiple
contexts (e.g. one for each gis.m display).

Actually, the "fixes" have also been rather ad-hoc. It might be better
to do away with WIND and store the name of a saved region in $GISRC
instead, remove MONITOR_OVERRIDE and WIND_OVERRIDE, and just create an
alternate $GISRC file whenever you want to change any of those
parameters.

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

Glynn,

[Too much to quote]

That's some crystal clear thinking there. parser.c now does the following:

Decides to overwrite if:
a.) --o switch is present (to force for one command)
b.) If the grass variable OVERWRITE is set to 1 (to force for all commands)
c.) If the GRASS_OVERWRITE environment variable is set to 1 (so that commands
run from scripts, system, etc. pick up on the original --o override)

When parser.c decides to overwrite it sets the GRASS_OVERWRITE environment
variable to 1. This can be used in scripts like this one.

The OVERWRITE environment variable is no more (it never existed before my
missed attempt).

GRASS_OVERWRITE is documented in variables.htm*

--Cedric

On 5/10/06, Glynn Clements <glynn@gclements.plus.com> wrote:

Actually, the "fixes" have also been rather ad-hoc. It might be better
to do away with WIND and store the name of a saved region in $GISRC
instead, remove MONITOR_OVERRIDE and WIND_OVERRIDE, and just create an
alternate $GISRC file whenever you want to change any of those
parameters.

Note that it is also possible to set region with GRASS_REGION
environment variable.

Radim

Radim Blazek wrote:

> Actually, the "fixes" have also been rather ad-hoc. It might be better
> to do away with WIND and store the name of a saved region in $GISRC
> instead, remove MONITOR_OVERRIDE and WIND_OVERRIDE, and just create an
> alternate $GISRC file whenever you want to change any of those
> parameters.

Note that it is also possible to set region with GRASS_REGION
environment variable.

I wasn't aware of that feature. That would be simpler for running
commands with a specific region from gis.m.

However, I don't think that the way it is currently implementated is a
good idea, as it overrides all saved regions as well as the current
region. I would suggest moving it to G_get_window(), so that programs
which use G__get_window() to read a specific named region aren't
affected.

I've recently fixed a small number of modules which gratuitously used
G__get_window(..., "WIND") to get the current region.

[Most of these did so in order to handle the case where the current
region was undefined, and use the default region instead. If that's
considered a good idea, it should be done in G_get_window() so that it
works for every module, not by a handful of specific modules.]

Modifying G_get_window() should now suffice for everything which
actually wants "the current region", except for g.region (which has a
legitimate need to specifically handle the case where the current
region is invalid). Everything else that still uses G__get_window()
does so because it wants a named region, or the default region, or the
current region for some other mapset.

For future development, I'm wondering if it would be a good idea to
replace the WIND file with a GRASS variable (e.g. REGION) using the
same syntax as GRASS_REGION.

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

> Actually, the "fixes" have also been rather ad-hoc. It might be
> better to do away with WIND and store the name of a saved region in
> $GISRC instead, remove MONITOR_OVERRIDE and WIND_OVERRIDE, and just
> create an alternate $GISRC file whenever you want to change any of
> those parameters.

It is nice to have the WIND file and its contents out in the open and
easy to look at from the file sytstem vs. being buried somewhere in a
hidden file. It's the one file (besides the dir) you need to make a new
(+ clean) mapset. Alternate $GISRC files would get confusing rather
quickly, bring along unwanted settings from the previous version, etc;
especially if they are persistent on the disk/not cleaned up.

Hamish

On 5/11/06, Glynn Clements <glynn@gclements.plus.com> wrote:

> Note that it is also possible to set region with GRASS_REGION
> environment variable.

I wasn't aware of that feature. That would be simpler for running
commands with a specific region from gis.m.

However, I don't think that the way it is currently implementated is a
good idea, as it overrides all saved regions as well as the current
region. I would suggest moving it to G_get_window(), so that programs
which use G__get_window() to read a specific named region aren't
affected.

Of course, I moved it to G_get_window(). But then g.region -p
prints WIND file, not GRASS_REGION variable, I am not sure
what it should. Probably it should print GRASS_REGION?

I've recently fixed a small number of modules which gratuitously used
G__get_window(..., "WIND") to get the current region.

[Most of these did so in order to handle the case where the current
region was undefined, and use the default region instead. If that's
considered a good idea, it should be done in G_get_window() so that it
works for every module, not by a handful of specific modules.]

I think that undefined region is error and it should result in
G_fatal_error().

For future development, I'm wondering if it would be a good idea to
replace the WIND file with a GRASS variable (e.g. REGION) using the
same syntax as GRASS_REGION.

Do you mean GRASS mapset variable (in VAR)?

I am using WIND as test for mapset in QGIS, exists another way
to test if a directory in location is a mapset?

Radim

Radim Blazek wrote:

> > Note that it is also possible to set region with GRASS_REGION
> > environment variable.
>
> I wasn't aware of that feature. That would be simpler for running
> commands with a specific region from gis.m.
>
> However, I don't think that the way it is currently implementated is a
> good idea, as it overrides all saved regions as well as the current
> region. I would suggest moving it to G_get_window(), so that programs
> which use G__get_window() to read a specific named region aren't
> affected.

Of course, I moved it to G_get_window(). But then g.region -p
prints WIND file, not GRASS_REGION variable, I am not sure
what it should. Probably it should print GRASS_REGION?

I would say so.

g.region doesn't use G_get_window() to get the current region, but
uses:

  if (G__get_window (&window, "", "WIND", G_mapset()) != NULL)
  {
    G_get_default_window (&window);
    G_put_window (&window);
  }

The logic here is that if the WIND file is invalid, the only way to
set it is to run g.region, so so g.region can't just just call
G_get_window() (which will generate a fatal error if the WIND file is
invalid).

Note that it does this before calling G_parser(), so that it can use
window.proj in the option definitions.

After G_parser() returns, it replaces the region with the default
region if the -d flag is used, or with the named region if region= is
used. Otherwise, the previously-obtained region is retained and
modified according to the other options.

There are a few possible solutions.

One is to obtain the initial region using G_get_default_window().
After G_parser() returns, if the -d flag isn't given, it would use
G_get_window() to replace this with the current region. That would
allow the use of "g.region -d" to fix a missing/invalid WIND file,
while causing GRASS_REGION and WIND_OVERRIDE to work normally.

The other is to explicitly test for GRASS_REGION and/or WIND_OVERRIDE
in g.region.

> I've recently fixed a small number of modules which gratuitously used
> G__get_window(..., "WIND") to get the current region.
>
> [Most of these did so in order to handle the case where the current
> region was undefined, and use the default region instead. If that's
> considered a good idea, it should be done in G_get_window() so that it
> works for every module, not by a handful of specific modules.]

I think that undefined region is error and it should result in
G_fatal_error().

Agreed.

But even if the fault-tolerant behaviour was desirable, it should be
implemented in the libraries, rather than a handful of modules doing
it themselves.

> For future development, I'm wondering if it would be a good idea to
> replace the WIND file with a GRASS variable (e.g. REGION) using the
> same syntax as GRASS_REGION.

Do you mean GRASS mapset variable (in VAR)?

Well, I meant that G_get_window() would become use G_getenv("REGION")
rather than G__get_window(..., "WIND"). It wouldn't really matter if
it was a mapset or GISRC variable; the main point was not to store
state in a form which can only have a single instance. Actually, the
same issue applies to the VAR file; what uses that? Just QGIS?

If all "session state" goes into $GISRC, you can just create a new
$GISRC file for each operation, without worrying about conflicts
(providing that the operations only read the data; supporting
concurrent modification is a long way off).

I am using WIND as test for mapset in QGIS, exists another way
to test if a directory in location is a mapset?

You could test whether ../PERMANENT/DEFAULT_WIND exists. Or even just
whether ../PERMANENT exists.

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

Dear developers,
usually, I do not say anything, when you are tallking about ways and
strategies conserning general GRASS development. Now I would like to add
my two cents:

In the "Art of Unix programing" Eric Raymond describes common ways, how flags and
parameters are organized in Unix.

In general, there are two main groups: "X-way" and the "GNU-way". The
"X" programs are using one "-" for both, flags and parameters. The
"flags" can have and usually do have more than one character. Example:

    xclock -analog -fg black
    
Beyond GNU-programs usually have one character flags with one "-" and
character strings, quoted by two "--". Exapmle:

    fetchmal -q --logfile mylog

GRASS could not be subsumed in any of these groups, there was the
clear "GRASS-way", how things were organized, which was consistent: Flags
"-o", parameters "map=raster".

When I compiled after longer time GRASS again, I saw, that it was not
consistent any more. There is new flag, which does something, what was
solved IMHO better by the environment variable. To me looks new
"--o" flag strange. It does not fill GRASS's concept. IF so, there
should be single "-o" flag for *all* (suggested) modules - it would be more work,
but it would be IMHO more clean, than this workaround. What more, it
does not fill in any of the two groups mentioned by Eric Raymond.

I do not understand, why it was not possible simple to export "somehow"
the environment variable -> I believe you, there was no way, how to make
suggested scripts to work with this variable. But this solution comes as
"the worst from all good solutions" to me.

My apologize, if I hurt anybody on the list, I did not mean
to.

Jachym

On Tue, May 09, 2006 at 09:36:01PM +0200, Cedric Shock via RT wrote:

parser.c now sets the OVERWRITE environment variable, but doesn't export it.
Scripts run by g.parser now have it set so that modules they run will be run
as if --o was included. It can be used here to skip the overwrite check that's
implemented in r.blend to check for .r .g and .b extensions to the map name.

-------------------------------------------- Managed by Request Tracker

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

--
Jachym Cepicky
e-mail: jachym.cepicky@centrum.cz
URL: http://les-ejk.cz
GPG: http://les-ejk.cz/gnupg_public_key/jachym_cepicky-gpg_public_key.asc
-----------------------------------------
OFFICE:
GDF-Hannover
Mengendamm 16d
30177 Hannover
Germany
e-mail: cepicky@gdf-hannover.de
URL: http://gdf-hannover.de
Tel.: +49 511-39088507

Jachym,

The --o flag has been in grass for a long time. Things that start -- in grass
are options to the parser, things that start - are flags that are parsed by
the parser for the module. --o tells the parser not to check that to make
sure elements in "new" gisprompts don't exist.

The following are also parser options, all of these options must be the first
argument:
--help
show usage and exit. Also -help and help work for ease.

--interface-description
Print out xml description of command line interface and exit

--html-description
Print out html usage help for command to be integrated in manual pages.

--tcltk
Print out tcl/tk source code to be evaluated after sourcing gui.tcl to make a
user interface. This is for user interfaces like d.m and gis.m.

--ui
Force the gui user interface to run. This can be used by itself or in
combination with options and flags to preload those options and flags into
the ui. This could be changed so that it doesn't need to be the first
argument, since it doesn't preempt parsing.

--o and --overwrite
These don't need to be in the first argument. They override existing map check
for this command only.

Note that overwrite can also be set by g.gisenv so that it happens
automatically for all commands. Some users (at least me) prefer to not have
maps automatically overwritten, but for it to be easy to do for a single
command.

The only recent change was adding --o to the help. It was added to module help
because it is specifically useful for modules that employ a new gisprompt.

--Cedric

ok, thank you for explanation.

all the best

jachym

On Sat, May 13, 2006 at 10:31:09AM -0700, Cedric Shock wrote:

Jachym,

The --o flag has been in grass for a long time. Things that start -- in grass
are options to the parser, things that start - are flags that are parsed by
the parser for the module. --o tells the parser not to check that to make
sure elements in "new" gisprompts don't exist.

The following are also parser options, all of these options must be the first
argument:
--help
show usage and exit. Also -help and help work for ease.

--interface-description
Print out xml description of command line interface and exit

--html-description
Print out html usage help for command to be integrated in manual pages.

--tcltk
Print out tcl/tk source code to be evaluated after sourcing gui.tcl to make a
user interface. This is for user interfaces like d.m and gis.m.

--ui
Force the gui user interface to run. This can be used by itself or in
combination with options and flags to preload those options and flags into
the ui. This could be changed so that it doesn't need to be the first
argument, since it doesn't preempt parsing.

--o and --overwrite
These don't need to be in the first argument. They override existing map check
for this command only.

Note that overwrite can also be set by g.gisenv so that it happens
automatically for all commands. Some users (at least me) prefer to not have
maps automatically overwritten, but for it to be easy to do for a single
command.

The only recent change was adding --o to the help. It was added to module help
because it is specifically useful for modules that employ a new gisprompt.

--Cedric

--
Jachym Cepicky
e-mail: jachym.cepicky@centrum.cz
URL: http://les-ejk.cz
GPG: http://les-ejk.cz/gnupg_public_key/jachym_cepicky-gpg_public_key.asc
-----------------------------------------
OFFICE:
GDF-Hannover
Mengendamm 16d
30177 Hannover
Germany
e-mail: cepicky@gdf-hannover.de
URL: http://gdf-hannover.de
Tel.: +49 511-39088507

When I compiled after longer time GRASS again, I saw, that it was not
consistent any more. There is new flag, which does something, what was
solved IMHO better by the environment variable. To me looks new
"--o" flag strange. It does not fill GRASS's concept.

it is a shortcut for "--overwrite"

IF so, there should be single "-o" flag for *all* (suggested) modules
- it would be more work, but it would be IMHO more clean, than this
workaround.

There is already "-o" in modules that we cannot change (e.g. r.in.gdal).

For us, "-f" is always a module specific flag, "--word" is always a
global module flag.

Flags like -o, -h, -C should be avoided in future, if possible, to avoid
confusion.

Cedric:

The only recent change was adding --o to the help. It was added to
module help because it is specifically useful for modules that employ
a new gisprompt.

is it added to GUIs as well?

It should be listed in the help pages as --overwrite. "--o" is just a
shortcut that works, like,
  v.clean in=foo out=foo2*
instead of full input= and output=. Or omitting the first option name.

[*] using the same map name for both in= and out= with --o is bad, yes?
vectors modules do random access?

The lib/gis/parser.c test uses strncmp():

  /* Overwrite option */
  if ( strncmp(ptr,"--o", 3) == 0 || strncmp(ptr,"--overwrite",11) == 0 )
  {
     overwrite = 1;
  }

The first half of that if(||) always catches the second half (as well as
"--over", etc.); demote the second half to a comment?

Hamish

Hamish wrote:

It should be listed in the help pages as --overwrite. "--o" is just a
shortcut that works, like,
  v.clean in=foo out=foo2*
instead of full input= and output=. Or omitting the first option name.

[*] using the same map name for both in= and out= with --o is bad, yes?

It depends.

The actual data file (cell/fcell) for a raster map is created in the
temporary directory, then renamed when the map is closed (with
G_close_cell()), so that case will work.

Files which are read into or written from memory in one go (e.g.
colour tables, range, history, etc) aren't a problem.

Anything which has separate open, read/write, and close operations
(and which doesn't explicitly allow for this situation) will be
problematic.

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

Hamish wrote:

[*] using the same map name for both in= and out= with --o is bad, yes?

Maybe
  Vect_check_input_output_name ( inopt->answer, outopt->answer, GV_FATAL_EXIT );

should catch this (maybe it does?)? See v.extract for example.

Markus

On 5/12/06, Glynn Clements <glynn@gclements.plus.com> wrote:

> Of course, I moved it to G_get_window(). But then g.region -p
> prints WIND file, not GRASS_REGION variable, I am not sure
> what it should. Probably it should print GRASS_REGION?

I would say so.

g.region doesn't use G_get_window() to get the current region, but
uses:

        if (G__get_window (&window, "", "WIND", G_mapset()) != NULL)
        {
                G_get_default_window (&window);
                G_put_window (&window);
        }

The logic here is that if the WIND file is invalid, the only way to
set it is to run g.region, so so g.region can't just just call
G_get_window() (which will generate a fatal error if the WIND file is
invalid).

Note that it does this before calling G_parser(), so that it can use
window.proj in the option definitions.

After G_parser() returns, it replaces the region with the default
region if the -d flag is used, or with the named region if region= is
used. Otherwise, the previously-obtained region is retained and
modified according to the other options.

There are a few possible solutions.

One is to obtain the initial region using G_get_default_window().
After G_parser() returns, if the -d flag isn't given, it would use
G_get_window() to replace this with the current region. That would
allow the use of "g.region -d" to fix a missing/invalid WIND file,
while causing GRASS_REGION and WIND_OVERRIDE to work normally.

The other is to explicitly test for GRASS_REGION and/or WIND_OVERRIDE
in g.region.

In theory the module should know nothing about GRASS_REGION.
I would prefer the first possibility.
In any case I am not going to work on g.region.

> > For future development, I'm wondering if it would be a good idea to
> > replace the WIND file with a GRASS variable (e.g. REGION) using the
> > same syntax as GRASS_REGION.
>
> Do you mean GRASS mapset variable (in VAR)?

Well, I meant that G_get_window() would become use G_getenv("REGION")
rather than G__get_window(..., "WIND"). It wouldn't really matter if
it was a mapset or GISRC variable; the main point was not to store
state in a form which can only have a single instance. Actually, the
same issue applies to the VAR file; what uses that? Just QGIS?

Standard GRASS variables (GISRC) are stored in ~/.grassrc6
and reloaded when a new mapset is opened. It does not make sense
to store a region of one mapset and reload it in another one (which can be
even in a different projection).

In VAR file are stored variables specific for the mapset. For example
connection to database. It is not used by QGIS at all.

If all "session state" goes into $GISRC, you can just create a new
$GISRC file for each operation, without worrying about conflicts
(providing that the operations only read the data; supporting
concurrent modification is a long way off).

Yes, I like the idea, but we have to store the state of the last
session for mapset somewhere. And the session state which includes
WIND is mapset specific.

> I am using WIND as test for mapset in QGIS, exists another way
> to test if a directory in location is a mapset?

You could test whether ../PERMANENT/DEFAULT_WIND exists. Or even just
whether ../PERMANENT exists.

I use that as location test, but location can contain directories
which are not mapsets.

Radim

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

Radim Blazek wrote:

> > Of course, I moved it to G_get_window(). But then g.region -p
> > prints WIND file, not GRASS_REGION variable, I am not sure
> > what it should. Probably it should print GRASS_REGION?
>
> I would say so.
>
> g.region doesn't use G_get_window() to get the current region, but
> uses:
>
> if (G__get_window (&window, "", "WIND", G_mapset()) != NULL)
> {
> G_get_default_window (&window);
> G_put_window (&window);
> }
>
> The logic here is that if the WIND file is invalid, the only way to
> set it is to run g.region, so so g.region can't just just call
> G_get_window() (which will generate a fatal error if the WIND file is
> invalid).
>
> Note that it does this before calling G_parser(), so that it can use
> window.proj in the option definitions.
>
> After G_parser() returns, it replaces the region with the default
> region if the -d flag is used, or with the named region if region= is
> used. Otherwise, the previously-obtained region is retained and
> modified according to the other options.
>
> There are a few possible solutions.
>
> One is to obtain the initial region using G_get_default_window().
> After G_parser() returns, if the -d flag isn't given, it would use
> G_get_window() to replace this with the current region. That would
> allow the use of "g.region -d" to fix a missing/invalid WIND file,
> while causing GRASS_REGION and WIND_OVERRIDE to work normally.
>
> The other is to explicitly test for GRASS_REGION and/or WIND_OVERRIDE
> in g.region.

In theory the module should know nothing about GRASS_REGION.
I would prefer the first possibility.
In any case I am not going to work on g.region.

I'll implement the first solution in g.region.

> > > For future development, I'm wondering if it would be a good idea to
> > > replace the WIND file with a GRASS variable (e.g. REGION) using the
> > > same syntax as GRASS_REGION.
> >
> > Do you mean GRASS mapset variable (in VAR)?
>
> Well, I meant that G_get_window() would become use G_getenv("REGION")
> rather than G__get_window(..., "WIND"). It wouldn't really matter if
> it was a mapset or GISRC variable; the main point was not to store
> state in a form which can only have a single instance. Actually, the
> same issue applies to the VAR file; what uses that? Just QGIS?

Standard GRASS variables (GISRC) are stored in ~/.grassrc6
and reloaded when a new mapset is opened. It does not make sense
to store a region of one mapset and reload it in another one (which can be
even in a different projection).

In VAR file are stored variables specific for the mapset. For example
connection to database. It is not used by QGIS at all.

I see.

The point remains that using fixed files within the mapset directory
(WIND, VAR) means that you can only have one instance of that state
per mapset. That's fine for the historical "command-line session"
usage, but problematic for other cases.

The simplest fix is to repeat the hack used for WIND (provide an
environment variable to override it), but ultimately we need to
consider better mechanisms for saving state.

> If all "session state" goes into $GISRC, you can just create a new
> $GISRC file for each operation, without worrying about conflicts
> (providing that the operations only read the data; supporting
> concurrent modification is a long way off).

Yes, I like the idea, but we have to store the state of the last
session for mapset somewhere. And the session state which includes
WIND is mapset specific.

Actually, location-specific.

Ideally, all saved state should go into the user's home directory.
Using the database directories creates obstacles for sharing the data
between multiple users. Even then, a single user should be able to
create multiple multiple instances of that state.

One option would be to give each location a unique ID at the point
that it is created (e.g. hash of hostname, pathname, current time) and
use that ID as a key in the user's saved state files (e.g. G_getenv()
would look for <key>:<variable> rather than just <variable>).

Simply using the pathname to the location directory as a key is
problematic if the directory is shared over a network and mounted at
different points on different hosts, as you wouldn't be able to re-use
the state files.

> > I am using WIND as test for mapset in QGIS, exists another way
> > to test if a directory in location is a mapset?
>
> You could test whether ../PERMANENT/DEFAULT_WIND exists. Or even just
> whether ../PERMANENT exists.

I use that as location test, but location can contain directories
which are not mapsets.

Presumably there's nothing stopping those directories containing a
file named WIND. Ultimately, you have to rely upon the user not to try
to use a non-mapset directory as a mapset. Also, we can just assert
that the manually adding arbitrary files or directories within a GRASS
database directory will result in undefined behaviour (i.e. we're
under no obligation to handle such cases).

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

> > There are a few possible solutions.
> >
> > One is to obtain the initial region using G_get_default_window().
> > After G_parser() returns, if the -d flag isn't given, it would use
> > G_get_window() to replace this with the current region. That would
> > allow the use of "g.region -d" to fix a missing/invalid WIND file,
> > while causing GRASS_REGION and WIND_OVERRIDE to work normally.
> >
> > The other is to explicitly test for GRASS_REGION and/or
> > WIND_OVERRIDE in g.region.
>
> In theory the module should know nothing about GRASS_REGION.
> I would prefer the first possibility.
> In any case I am not going to work on g.region.

I'll implement the first solution in g.region.

If the WIND file is bad/missing, g.region should fix it without
requiring a special have-to-look-for-it flag (as it does now), probably
with a warning about what it just did. Many modules give an error
("Invalid region, run g.region"). "Allow sloppy input, only give precise
output"

The point remains that using fixed files within the mapset directory
(WIND, VAR) means that you can only have one instance of that state
per mapset. That's fine for the historical "command-line session"
usage, but problematic for other cases.

Do "other cases" ensure data integrity? Or does that require something
ugly like lock files for each map? (e.g. made even worse: existing gis.m
problem of r.colors etc changes not triggering an updated PPM in the
display layer cache). I don't mind the restriction of one [read-write]
session per mapset. If I want to access a map I can always start in a
new mapset and call map@first_mapset [read-only].

The simplest fix is to repeat the hack used for WIND (provide an
environment variable to override it), but ultimately we need to
consider better mechanisms for saving state.

FWIW, I am a fan of retaining a WIND file per mapset.

> > > I am using WIND as test for mapset in QGIS, exists another way
> > > to test if a directory in location is a mapset?
> >
> > You could test whether ../PERMANENT/DEFAULT_WIND exists. Or even
> > just whether ../PERMANENT exists.
>
> I use that as location test, but location can contain directories
> which are not mapsets.

Presumably there's nothing stopping those directories containing a
file named WIND.

No, but it's unlikely. Anything under the location dir is officially
part of the GRASS database, so people add stuff in there at their peril.
Tough luck.

Ultimately, you have to rely upon the user not to try to use a
non-mapset directory as a mapset.

The GUI startup screen will stop you if the mapset doesn't have a WIND
file (I'd prefer them greyed out on the mapset list). The text version
doesn't (it could; although text mode should probably be more permissive
& assume the user knows what they are doing).

Also, we can just assert that the manually adding arbitrary files or
directories within a GRASS database directory will result in undefined
behaviour (i.e. we're under no obligation to handle such cases).

This is already true. (unasserted, but mucking with files in $GISDBASE
is at own risk)

Hamish

Hamish wrote:

> The point remains that using fixed files within the mapset directory
> (WIND, VAR) means that you can only have one instance of that state
> per mapset. That's fine for the historical "command-line session"
> usage, but problematic for other cases.

Do "other cases" ensure data integrity? Or does that require something
ugly like lock files for each map? (e.g. made even worse: existing gis.m
problem of r.colors etc changes not triggering an updated PPM in the
display layer cache). I don't mind the restriction of one [read-write]
session per mapset. If I want to access a map I can always start in a
new mapset and call map@first_mapset [read-only].

For the foreseeable future, each mapset will be limited to either one
read-write session or multiple read-only sessions, but not both.

However, the notion of "write" should be restricted to modifying
actual /maps/. The multiple-readers case should allow each reader to
have a different region, different monitor, different settings for any
variables, etc. Which precludes using any fixed files such as WIND or
VAR.

FWIW, most of the concurrency issues still apply even if you read maps
from a different mapset. If a writer is modifying the mapset which you
are reading, you could end up reading a map (or even a file) which is
in an inconsistent state.

In terms of reliably supporting both a writer /and/ one or more
readers, there are two levels: individual files and entire maps.

Ensuring that a reader doesn't get to see an inconsistent file is
simple enough. The raster cell/fcell files are already handled
correctly, but the code which writes out support files (cellhd, range,
colr etc) needs to be modified to behave similarly (i.e. write to a
temporary file then rename() it over the original file once it has
been completely written).

Supporting atomic updates of entire maps would be significantly
harder. It might be possible to avoid having to modify all of the
individual modules by writing all output files to a temporary
directory then having an atexit() handler rename them to their correct
locations.

Changing the layout so that all of a map's files reside within a
single directory would simplify this, as you just need to replace one
directory with another, rather than replacing multiple files.

Even so, you can't atomically replace a directory. You can get quite
close by renaming the old directory out of the way, renaming the new
directory into place, then removing the old directory, but that still
leaves a small window where the directory doesn't exist. True atomic
updates would require that maps are accessed via symlinks; those can
be replaced atomically.

> The simplest fix is to repeat the hack used for WIND (provide an
> environment variable to override it), but ultimately we need to
> consider better mechanisms for saving state.

FWIW, I am a fan of retaining a WIND file per mapset.

There is no problem in retaining it for legacy usage, but allowing
multiple readers requires that they use a different mechanism. Which
basically means that everything needs to use the new mechanism,
otherwise you need some mechanism to decide who gets to use the WIND
file.

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