Opened 10 years ago
Last modified 6 years ago
#2681 new task
Remove legacy meaning of LOCATION variable
Reported by: | wenzeslaus | Owned by: | |
---|---|---|---|
Priority: | blocker | Milestone: | 8.0.0 |
Component: | Startup | Version: | svn-trunk |
Keywords: | init, grass.py, interface, CLI, environment variables | Cc: | |
CPU: | Unspecified | Platform: | All |
Description
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.
Change History (15)
follow-up: 2 comment:1 by , 10 years ago
follow-up: 3 comment:2 by , 10 years ago
Replying to glynn:
Replying to wenzeslaus:
Especially in older and stable code like
grass.py
we still keep the legacyLOCATION
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 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, 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 supportedLOCATION
means Location name (analogy toMAPSET
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).
comment:3 by , 10 years ago
Replying to wenzeslaus:
There is some export somewhere even now. I can do:
GRASS > echo $LOCATION .../grassdata/nc_spm_08_grass7/user1
It seems that 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 forLOCATION_NAME
. This would require changes tog.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).
follow-up: 10 comment:5 by , 6 years ago
As for using the name LOCATION
to mean "full path to a mapset", remaining places are the following:
The main in the variable defined for the shell in lib/init/grass.py
. That seems to be only interface-related usage. I think this should be replaced by something else like MAPSET_PATH
or FULL_MAPSET
.
There is something in lib/init/README
which would likely use general update.
There might be some bad examples in existing code, these two may qualify: raster/r.mapcalc/testsuite/const_map_test.sh
and testsuite/raster/raster_md5test.sh
.
There are places which need deeper look, namely lib/gis/make_mapset.c
and lib/gis/location.c
.
comment:10 by , 6 years ago
Here is the status of the files identified in the comment:5:
Resolved:
- source:grass/trunk/lib/init/README - Done, text removed (r73355)
- source:grass/trunk/raster/r.mapcalc/testsuite/const_map_test.sh - Done, replaced by MAPSET_PATH, test is running (r73357)
- source:grass/trunk/testsuite/raster/raster_md5test.sh - Done, replaced by MAPSET_PATH, test is running (r73357)
Resolved, needs change for 8.0:
- source:grass/trunk/lib/init/grass.py - Done, replaced by MAPSET_PATH (LOCATION left for 7.8), LOCATION_NAME in doc replaced by LOCATION (r73352, r73353, r73354, r73356)
Unresolved:
- source:grass/trunk/lib/gis/location.c - LOCATION used in error message in G_location_path(), probably "location path" or "Location path" would be more appropriate here
- source:grass/trunk/lib/gis/make_mapset.c - I would expect LOCATION_NAME here, but it says LOCATION. I'm not sure what it does or what it is supposed to do. Comes all the way from r21496.
Replying to wenzeslaus:
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.