[GRASS-dev] [GRASS GIS] #2681: Remove legacy meaning of LOCATION variable

GDAL · May 19, 2015, 10:15pm

#2681: Remove legacy meaning of LOCATION variable
-------------------------------------------------+-------------------------
Reporter: wenzeslaus | Owner: grass-dev@…
Type: task | Status: new
Priority: blocker | Milestone: 8.0.0
Component: Startup | Version: svn-trunk
Keywords: init, grass.py, interface, CLI, | CPU: Unspecified
environment variables |
Platform: All |
-------------------------------------------------+-------------------------
Especially in older and stable code like `grass.py` we still keep the
legacy `LOCATION` variable with meaning "full path to a Mapset".

The bug #2679 suggests that nobody is actually using it. So, there is no
need for keeping this for backward compatibility anymore.

I suggest to gradually change the meaning to "Location name" and use
different variable name for "full path to a Mapset". This can be a gradual
change with keeping some compatibility or providing warnings. For example,
the "gisrc" file can support both `LOCATION` and `LOCATION_NAME` as
"Location name".

Dropping of LOCATION as "full path to a Mapset" can be without
consequences but replacing `LOCATION_NAME` by `LOCATION` might be possible
only for GRASS GIS 8 because of usages outside GRASS by users or in other
projects (this might be valid only for the "gisrc" file):

  *
https://github.com/qgis/QGIS/blob/0a1382a0be36d408aebd227fb0066f68c513e41e/python/plugins/processing/algs/grass7/Grass7Utils.py
  *
https://github.com/moovida/jgrasstools/blob/530c87f26d220f3eeff9d2fb9d21abd8821c00c3/grass/src/main/java/org/jgrasstools/grass/utils/GrassUtils.java
  *
http://sextante.googlecode.com/svn/trunk/soft/sextante_lib/sextante_gui/src/es/unex/sextante/gui/grass/GrassUtils.java
  *
https://github.com/geopython/PyWPS/blob/425f0eb160f60714a6705a24ba926e03690ab371/pywps/Grass.py
  *
http://grasswiki.osgeo.org/wiki/Working_with_GRASS_without_starting_it_explicitly#Bash_examples_.28GNU.2FLinux.29

In source code, `LOCATION` and `location` variables with meaning "full
Mapset path" can be replaced by `mapset_path`. (I've also tried
`full_mapset` but it doesn't seem nice.)

In the "gisrc" file `LOCATION_NAME` would be replaced by `LOCATION`.

In environment variables interface, `LOCATION_NAME` would be replaced by
`LOCATION` and original `LOCATION` by `MAPSET_PATH`.

It is questionable if we want to support input environment variable with
meaning "full Mapset path" when we already support setting Location and
Mapset separately. The "gisrc" file does not support it anyway.

See also #2679 for possible removal or restoration of environment
variables for setting Location and Mapset.

Setting as blocker for 8.0.0 because this can be fully resolved only with
new version but there are changes which could be done now such as (only)
dropping the `LOCATION` environmental variable as input.

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/2681>
GRASS GIS <http://grass.osgeo.org>

GDAL · May 20, 2015, 12:45pm

#2681: Remove legacy meaning of LOCATION variable
-------------------------+-------------------------------------------------
  Reporter: wenzeslaus | Owner: grass-dev@…
      Type: task | Status: new
  Priority: blocker | Milestone: 8.0.0
Component: Startup | Version: svn-trunk
Resolution: | Keywords: init, grass.py, interface, CLI,
       CPU: | environment variables
  Unspecified | Platform: All
-------------------------+-------------------------------------------------

Comment (by glynn):

Replying to [ticket:2681 wenzeslaus]:

> Especially in older and stable code like `grass.py` we still keep the
legacy `LOCATION` variable with meaning "full path to a Mapset".
>
> The bug #2679 suggests that nobody is actually using it. So, there is no
need for keeping this for backward compatibility anymore.

Originally, GISDBASE, LOCATION_NAME and MAPSET were actual environment
variables. Even after G_getenv() etc were added, the startup script
continued to set the environment variables for the benefit of scripts.
When g.mapset was added, we had to change scripts to explicitly query the
environment using g.gisenv, because g.mapset cannot change the environment
variables of its parent process (e.g. the shell). To ensure that this was
done, the startup script stopped setting the environment variables
(r10655), so that unfixed scripts failed rather than using a possibly-
incorrect environment. The export for LOCATION was re-added temporarily in
r10790 then removed again in r10793.

None of these variables should have been exported to the environment in
over a decade.

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/2681#comment:1>
GRASS GIS <http://grass.osgeo.org>

GDAL · May 20, 2015, 1:42pm

#2681: Remove legacy meaning of LOCATION variable
-------------------------+-------------------------------------------------
  Reporter: wenzeslaus | Owner: grass-dev@…
      Type: task | Status: new
  Priority: blocker | Milestone: 8.0.0
Component: Startup | Version: svn-trunk
Resolution: | Keywords: init, grass.py, interface, CLI,
       CPU: | environment variables
  Unspecified | Platform: All
-------------------------+-------------------------------------------------

Comment (by wenzeslaus):

Replying to [comment:1 glynn]:
> Replying to [ticket:2681 wenzeslaus]:
>
> > Especially in older and stable code like `grass.py` we still keep the
legacy `LOCATION` variable with meaning "full path to a Mapset".
> >
> > The bug #2679 suggests that nobody is actually using it. So, there is
no need for keeping this for backward compatibility anymore.
>
> Originally, GISDBASE, LOCATION_NAME and MAPSET were actual environment
variables. [...]

This makes sense. Thanks for the clarification.

> ...the startup script stopped setting the environment variables
(r10655)... The export for LOCATION was re-added temporarily in r10790
then removed again in r10793.

There is some export somewhere even now. I can do:

{{{
GRASS > echo $LOCATION
.../grassdata/nc_spm_08_grass7/user1
}}}

It seems that [source:grass/trunk/lib/init/grass.py#L1421 bash_startup()]
is to blame. It seems that this was added in r60216. I don't see it in
`prompt.py` which was removed in that commit.

> None of these variables should have been exported to the environment in
over a decade.

And they are not. However, as described in #2679,
[http://grass.osgeo.org/grass70/manuals/grass7.html#other-examples the
documentation] says that you can set these environment variables including
`LOCATION` //before// starting GRASS and that GRASS will respect them
(i.e. store them into "gisrc" file).

So then I would say that if #2679 functionality will be implemented
(rather then dropped) it should be done in the way that

  * `LOCATION_NAME` is not supported
  * `LOCATION` means Location name (analogy to `MAPSET` meaning Mapset
name)
  * `MAPSET_PATH` means full path to mapset (if implemented)
  * variables are removed from the environment by `grass.py` because no
GRASS processes should rely on them
  * we consider prefixing all with `GRASS_`

The documentation of `grass.py` (`grass7 --help`) can be changed without
any harm. (As well as any source code such as `grass.py`.)

This would leave the "gisrc" file the only occurrence of `LOCATION_NAME`.
As I mentioned before, we cannot remove it because we've already released
GRASS GIS 7.0 and the "gisrc" file is used as an interface when "not
starting GRASS explicitly." We can do this for GRASS GIS 8.

What we could do for 7 series is to introduce forward compatibility and
provide `LOCATION` as an alternative for `LOCATION_NAME`. This would
require changes to `g.gisenv` and related library functions but it seems
possible (although I haven't checked source code yet).

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/2681#comment:2>
GRASS GIS <http://grass.osgeo.org>

GDAL · May 21, 2015, 8:22am

#2681: Remove legacy meaning of LOCATION variable
-------------------------+-------------------------------------------------
  Reporter: wenzeslaus | Owner: grass-dev@…
      Type: task | Status: new
  Priority: blocker | Milestone: 8.0.0
Component: Startup | Version: svn-trunk
Resolution: | Keywords: init, grass.py, interface, CLI,
       CPU: | environment variables
  Unspecified | Platform: All
-------------------------+-------------------------------------------------

Comment (by glynn):

Replying to [comment:2 wenzeslaus]:

> There is some export somewhere even now. I can do:
>
{{{
GRASS > echo $LOCATION
.../grassdata/nc_spm_08_grass7/user1
}}}
>
> It seems that [source:grass/trunk/lib/init/grass.py#L1421
bash_startup()] is to blame.

There is no export. LOCATION is a shell variable, not an environment
variable.

> So then I would say that if #2679 functionality will be implemented
(rather then dropped)

I see no reason to implement it. And if it is implemented, the startup
script should explicitly remove those variables from the environment.

> This would leave the "gisrc" file the only occurrence of LOCATION_NAME

That's already the case.

> What we could do for 7 series is to introduce forward compatibility and
provide `LOCATION` as an alternative for `LOCATION_NAME`. This would
require changes to `g.gisenv` and related library functions but it seems
possible (although I haven't checked source code yet).

Unless you're going to make g.gisenv automatically map LOCATION to
LOCATION_NAME, it would require changes to anything which uses it (e.g.
lib/python/script and a number of scripts).

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/2681#comment:3>
GRASS GIS <http://grass.osgeo.org>