[GRASS-dev] MASK_OVERRIDE

[Glynn Clements]

Ivan Shmakov wrote:

>  >> Since it's somewhat cumbersome to work with multiple `MASK's
>  >> currently (`r.reclass', `g.copy'?),
> 
>  > Once you have a set of maps suitable for use as masks, switching
>  > between them just requires using g.rename or "g.copy --o".
> 
> 	Yes.  Though, in my opinion, it's hardly as convenient as,
> 	e. g.:
> 
> $ MASK_OVERRIDE=my_mask r.stats -c my_raster 
> 
> $ MASK_OVERRIDE=my_mask bash process-it-all-for-me.sh 

An environment variable is probably easier if you're only using the
mask for a single command, provided that the user has a working
knowledge of their shell (you'd be surprised how many don't; or maybe
you wouldn't).

If you're using a mask for multiple commands, g.copy is probably
easier. It doesn't require knowledge of shell variables versus
environment variables, and it can be wrapped inside a script (whereas
you can't make a script "export" to the shell from which it's called).

>  >> +    {
>  >> +	const char *override = getenv ("MASK_OVERRIDE");
>  >> +	if (override != 0) strncpy (name, override, sizeof (name));
> 
>  > Use "if (override)" rather than an explicit comparison against 0.
>  > Although C allows comparisons between pointers and literal 0, you may
>  > get a warning, and we have enough of those cluttering up the build
>  > output already. Also, NULL/non-NULL pointers are conventionally as
>  > much of a logic value as zero/non-zero integers.
> 
> 	... The thing I cannot get used in C...

What thing? That C doesn't have a distinct boolean type?

>  > For this feature to be of real use, you need to allow MASK_OVERRIDE
>  > to be used to disable reading a mask altogether. E.g. if
>  > MASK_OVERRIDE is defined but empty, no mask should be used rather
>  > than falling back to MASK. This would allow the GUI to operate
>  > without a mask even when MASK has been created via the command line.
> 
> 	The code doesn't ever fall back to `MASK' if `MASK_OVERRIDE' is
> 	defined.  If `MASK_OVERRIDE' doesn't point to a valid raster
> 	(e. g., if it's an empty string), the mask is ignored.

Okay; I misread the part where it was saving the mapset.

AFAICT, it shouldn't be necessary to handle the mapset explicitly. The
lower level functions should all accept a qualified name, i.e.:

	G__open_cell_old("mask at mapset", "")

So you should be able to simply test for existence with
G_find_cell2(), and store the (possibly qualified) name.

Ideally, I'd like to remove the mapset argument from the core libgis
functions in 7.x. It will involve modifying almost every single
module, but it will make writing new modules and maintaining existing
modules simpler.

The only code which really needs to split a qualified name is in low
level functions which actually open files, plus anything which creates
a derived name (i.e. foo at bar -> foo.tmp at bar not foo at bar.tmp).

> 	However, I doubt if it's a reasonable behaviour.  It seems to be
> 	usual in the Unix world to ignore the environment variable's
> 	value if it's an empty string.  Using a non-existing raster
> 	(say, `.') is probably a better idea.

Good point. It's a nuisance for a shell script to distinguish between
a variable being unset and being set to the empty string. We should
really choose a specific "none" value rather than just silently
ignoring a reference to a non-existent map. If $MASK_OVERRIDE is set
but the map doesn't exist, it should at least produce a warning.

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