Opened 7 years ago
Closed 6 years ago
#3462 closed defect (fixed)
Allow setting environment variables in grass startup script
Reported by: | mankoff | Owned by: | martinl |
---|---|---|---|
Priority: | major | Milestone: | 7.6.0 |
Component: | Startup | Version: | 7.2.2 |
Keywords: | init grass.py env D_LIBRARY_PATH bashrc | Cc: | grass-dev@… |
CPU: | Unspecified | Platform: | MacOSX |
Description
In the grass72 startup script, in the function def bash_startup(location, location_name, grass_env_file):
These lines exist:
for line in readfile(env_file).splitlines(): if not line.startswith('export'): f.write(line + '\n')
If I comment out the 2nd line (and fix the indentation for the 3rd line because this is Python), then I can add export DYLD_LIBRARY_PATH=...
in either ~/.grass.bashrc
or ~/.grass7/bashrc
and this fixes various GRASS issues on OS X relating to the temporal framework.
Why does the startup script ignores export
statements from the GRASS config file? Is there a problem if I change this behavior?
In more detail, this is not an issue with Fink installations, but is with MacPorts. Fink installs an OS X Application: GRASS.app. Deep in the app I found a Makefile that seems to tell GRASS.app to respect GRASS_LD_LIBRARY_PATH
. The Macports version does not install an app, and does not respect the GRASS_LD_LIBRARY_PATH
variable. For some reason, shell-level LD_LIBRARY_PATH
and DYLD_LIBRARY_PATH
's are also not respected (perhaps due to SIP?).
Attachments (2)
Change History (29)
comment:1 by , 7 years ago
comment:2 by , 7 years ago
Priority: | normal → major |
---|---|
Type: | defect → enhancement |
I'm submitting a patch and changing this to a feature request. I don't know how to ping/CC MartinL, the original author of this change.
by , 7 years ago
Attachment: | allow_export.diff added |
---|
Patch to allow setting environment variables in grass startup files
comment:3 by , 7 years ago
Environmental variables are loaded by load_env() source:grass/trunk/lib/init/grass.py?rev=65585#L1710 from default config file source:grass/trunk/lib/init/grass.py?rev=65585#L1220. In my case on Linux (using Bash), it's ~/.grass7/bashrc
. Eg.
$ cat ~/.grass7/bashrc export GRASS_MESSAGE_FORMAT=plain $ GRASS_DEBUG=ON ./bin.x86_64-pc-linux-gnu/grass75 -text ... DEBUG: Environmental variable set GRASS_MESSAGE_FORMAT=plain > env | grep MESSAGE GRASS_MESSAGE_FORMAT=plain
r65585 has been introduced to read only alias and other commands since environmental variable should be already read.
comment:6 by , 7 years ago
My shell is /bin/bash
.
I have the following in ~/.grass.bashrc
:
echo ".grass.bashrc" export FOO=42 export DYLD_LIBRARY_PATH=/opt/local/share/grass-7.2.2/lib
And this in ~/.grass7/bashrc
:
echo ".grass7/bashrc" export BAR=42 export DYLD_LIBRARY_PATH=/opt/local/share/grass-7.2.2/lib
And when I launch grass with: grass72 -c ./Gtmp
, I see the following:
.grass.bashrc .grass7/bashrc
And:
GRASS 7.2.2 (Gtmp):~ > echo $FOO GRASS 7.2.2 (Gtmp):~ > echo $BAR 42 GRASS 7.2.2 (Gtmp):~ > echo $DYLD_LIBRARY_PATH GRASS 7.2.2 (Gtmp):~ >
If I comment out the line that blocks export
statements in my startup file, both FOO
and DYLD_LIBRARY_PATH
are correctly set. Is the DYLD
issue related to the following? What is going on on line 564: https://trac.osgeo.org/grass/browser/grass/trunk/lib/init/grass.py?rev=65585#L564
# Set LD_LIBRARY_PATH (etc) to find GRASS shared libraries # this works for subprocesses but won't affect the current process path_prepend(gpath("lib"), ld_library_path_var)
follow-up: 9 comment:8 by , 7 years ago
I'm not clear how the comment in grass.py
pointing to this ticket helps. Certain critical environment variables that are required for GRASS to work correctly cannot be set in either ~/.grass.bashrc
or ~/.grass7/bashrc
, as long as the line ignoring export
statements remains. Why can't I export
variables from these scripts?
comment:9 by , 7 years ago
Replying to mankoff:
I'm not clear how the comment in
grass.py
pointing to this ticket helps. Certain critical environment variables that are required for GRASS to work correctly cannot be set in either~/.grass.bashrc
or~/.grass7/bashrc
, as long as the line ignoringexport
statements remains. Why can't Iexport
variables from these scripts?
As I mentioned earlier, the environment variables should be loaded by load_env()
. I tested your config file and it's working on my Linux machine. So probably something related to MAC? Try to put more debug message around load_env()
.
cat ~/.grass7/bashrc export BAR=42 export DYLD_LIBRARY_PATH=/opt/local/share/grass-7.2.2/lib GRASS_DEBUG=ON ./bin.x86_64-pc-linux-gnu/grass75 -text ... DEBUG: Environmental variable set BAR=42 DEBUG: Environmental variable set DYLD_LIBRARY_PATH=/opt/local/share/grass-7.2.2/lib ... GRASS> env | grep BAR BAR=42 GRASS> env | grep DY DYLD_LIBRARY_PATH=/opt/local/share/grass-7.2.2/lib
follow-up: 11 comment:10 by , 7 years ago
Yes this is something MAC related. The Fink installation of GRASS gets around this by modifying the program to look for a GRASS_LIBRARY_PATH
which lets me set the DYLD_LIBRARY_PATH
. I'm using MacPorts and this modification has not been made.
I get that as mentioned earlier, environment variables should be loaded by load_env()
. But nobody has offered an explanation for why export
statements are ignored here. GRASS is, generally, a power user tool and lets the users do what they want. Why is it acting in such a protective manner here?
Results from running GRASS with GRASS_DEBUG=ON
are:
$ GRASS_DEBUG=ON grass72 -c EPSG:3413 ./Gtmp DEBUG: GRASS_DEBUG environmental variable is set. It is meant to be an internal variable for debugging only this script. Use 'g.gisenv set="DEBUG=[0-5]"' to turn GRASS GIS debug mode on if you wish to do so. DEBUG: Environmental variable set BAR=42 DEBUG: Environmental variable set DYLD_LIBRARY_PATH=/opt/local/share/grass-7.2.2/lib DEBUG: Tmp directory '/var/folders/7y/lbz1bs057398tql00m7fz0hr0000gn/T/grass7-kdm-10348' created for user 'kdm' Cleaning up temporary files... access: No such file or directory ERROR: LOCATION </Users/kdm/Gtmp> not available Creating new GRASS GIS location/mapset... DEBUG: Mapset </Users/kdm/Gtmp/PERMANENT> locked using '/Users/kdm/Gtmp/PERMANENT/.gislock' __________ ___ __________ _______________ / ____/ __ \/ | / ___/ ___/ / ____/ _/ ___/ / / __/ /_/ / /| | \__ \\_ \ / / __ / / \__ \ / /_/ / _, _/ ___ |___/ /__/ / / /_/ // / ___/ / \____/_/ |_/_/ |_/____/____/ \____/___//____/ Welcome to GRASS GIS 7.2.2 GRASS GIS homepage: http://grass.osgeo.org This version running through: Bash Shell (/bin/bash) Help is available with the command: g.manual -i See the licence terms with: g.version -c See citation options with: g.version -x Start the GUI with: g.gui wxpython When ready to quit enter: exit DEBUG: GRASS GUI should be <text> .grass.bashrc .grass7/bashrc GRASS 7.2.2 (Gtmp):~ > env | grep BAR BAR=42 GRASS 7.2.2 (Gtmp):~ > env | grep DY GRASS 7.2.2 (Gtmp):~ >
comment:11 by , 7 years ago
Replying to mankoff:
I get that as mentioned earlier, environment variables should be loaded by
load_env()
. But nobody has offered an explanation for whyexport
statements are ignored here. GRASS is,
Well, then it's up to you to help us discovering where the problem is. Nobody else was able till now to reproduce this bug. Than it's hard to guess.
generally, a power user tool and lets the users do what they want. Why is it acting in such a protective manner here?
Instead of hacking around it would be nice to find out why load_env()
is not working in your case.
> $ GRASS_DEBUG=ON grass72 -c EPSG:3413 ./Gtmp > DEBUG: GRASS_DEBUG environmental variable is set. It is meant to be an internal variable for debugging only this script. > Use 'g.gisenv set="DEBUG=[0-5]"' to turn GRASS GIS debug mode on if you wish to do so. > DEBUG: Environmental variable set BAR=42 > DEBUG: Environmental variable set DYLD_LIBRARY_PATH=/opt/local/share/grass-7.2.2/lib
Looks OK.
> .grass.bashrc > .grass7/bashrc
Such lines are not printed in my case.
> GRASS 7.2.2 (Gtmp):~ > env | grep BAR > BAR=42
> GRASS 7.2.2 (Gtmp):~ > env | grep DY > GRASS 7.2.2 (Gtmp):~ >
Please attach your .grass7/bashrc
file.
comment:12 by , 7 years ago
Component: | Default → Startup |
---|---|
Milestone: | → 7.2.3 |
Type: | enhancement → defect |
by , 7 years ago
~/.grass7/bashrc showing that setting DYLD_LIBRARY_PATH
does not work.
comment:13 by , 7 years ago
OK. This bug is related to OS X "SIP" and is discussed, for example, in these two threads:
1) http://osgeo-org.1560.x6.nabble.com/Mac-El-Capitan-and-GRASS-td5230885.html and
2) https://www.mail-archive.com/grass-user@lists.osgeo.org/msg31571.html
For some reason, all of this "security" can easily be bypassed if the GRASS startup script does not explicitly block export
statements as in the attached patch.
comment:14 by , 7 years ago
Fink solves this by using GRASS_LD_LIBRARY_PATH
. See 3 uses in their patch to get GRASS working on OS X: http://fink.cvs.sourceforge.net/viewvc/fink/dists/10.9-libcxx/stable/main/finkinfo/sci/grass72.patch?revision=1.1&view=markup
follow-up: 16 comment:15 by , 7 years ago
Hello again,
To try to clarify/summarize the above comments: OS X has a security feature that makes load_env()
not work properly, and therefore the temporal framework is not available to OS X users. There is a simple work-around: Remove one line in the startup script. Patch is attached.
There may be other ways to fix this bug, but I have not yet discovered them, and the proposed fix is simple and has no downsides that I can see or that anyone has mentioned in the comments.
I have asked but not received an explanation for why this line exists in the startup script. The offending line blocks users from export
'ing environment variables in ~/.grass7/bashrc
. This decision confuses me - GRASS is a power-user tool, and anyone setting environment variables there presumably knows what they are doing.
comment:16 by , 7 years ago
Replying to mankoff:
I have asked but not received an explanation for why this line exists in the startup script. The
sorry, but you got an explanation! Please read carefully the comments. There is a special procedure for loading environmental variables (load_env()
). That's reason why this line exists in the code.
Anyway I will prepare patch for security issue reported in MacOS. It will be still workaround anyway.
comment:19 by , 7 years ago
Cc: | added |
---|---|
Owner: | changed from | to
Status: | new → assigned |
follow-up: 22 comment:21 by , 7 years ago
Thank you for making the change! I'm sorry I did not thank you and test/confirm the change previously. However - I cannot test it. I compile GRASS dev versions on a Linux machine, but because OS X is only partially supported, I have never gotten a full build/test environment working on OS X. I need to wait until one of the package managers provides a version with your updates. I recently upgraded to 7.4.0 but had to downgrade to 7.2.2 because 7.4.0 does not work with QGIS. I *think* you can close this ticket (again, based on visual inspection of the change).
comment:22 by , 7 years ago
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
Replying to mankoff:
Thank you for making the change! I'm sorry I did not thank you and test/confirm the change previously. However - I cannot test it. I compile GRASS dev versions on a Linux machine, but because OS X is only partially supported, I have never gotten a full build/test environment working on OS X.
Did you try this? https://grasswiki.osgeo.org/wiki/Compiling_on_MacOSX_using_homebrew
I need to wait until one of the package managers provides a version with your updates. I recently upgraded to 7.4.0 but had to downgrade to 7.2.2 because 7.4.0 does not work with QGIS.
It meanwhile does:
https://github.com/qgis/QGIS/pull/6394
- present in QGIS 3.0.0: https://github.com/qgis/QGIS/blob/release-3_0/cmake/FindGRASS.cmake
- present in QGIS 2.18.17: https://github.com/qgis/QGIS/blob/release-2_18/cmake/FindGRASS.cmake
I *think* you can close this ticket (again, based on visual inspection of the change).
Backport?
comment:23 by , 7 years ago
Resolution: | fixed |
---|---|
Status: | closed → reopened |
Reopen for backport (to 7.4 at least?)
comment:24 by , 7 years ago
Did you try this? https://grasswiki.osgeo.org/wiki/Compiling_on_MacOSX_using_homebrew
I wrote the instructions there for 7.2. At the time the "How to install GRASS GIS 7.1 SVN Head" section didn't work on 7.2 head. And I'm no longer using Homebrew but MacPorts - so no I cannot easily test non-packaged versions on OS X :(.
comment:26 by , 7 years ago
Milestone: | → 7.2.4 |
---|
comment:27 by , 6 years ago
Keywords: | init grass.py env D_LIBRARY_PATH bashrc added |
---|---|
Milestone: | 7.2.4 → 7.6.0 |
Resolution: | → fixed |
Status: | reopened → closed |
This was already fixed and reopened for backport in comment:23. This may not be a change which is appropriate to backport. At this point, it will just be in 7.6. Changing milestone. Closing as fixed.
For the record, GRASS GIS itself doesn't use GRASS_LD_LIBRARY_PATH
at this point.
Checking with "svn blame" in track I see these related changes:
Not sure why to ignore export statements from the GRASS config file. Maybe MartinL remembers?