Opened 6 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)

allow_export.diff (564 bytes ) - added by mankoff 6 years ago.
Patch to allow setting environment variables in grass startup files
bashrc (210 bytes ) - added by mankoff 6 years ago.
~/.grass7/bashrc showing that setting DYLD_LIBRARY_PATH does not work.

Download all attachments as: .zip

Change History (29)

comment:1 by neteler, 6 years ago

Checking with "svn blame" in track I see these related changes:

r65585: partly re-introduce r65307 (see r65584)

Not sure why to ignore export statements from the GRASS config file. Maybe MartinL remembers?

comment:2 by mankoff, 6 years ago

Priority: normalmajor
Type: defectenhancement

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 mankoff, 6 years ago

Attachment: allow_export.diff added

Patch to allow setting environment variables in grass startup files

comment:3 by martinl, 6 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:4 by martinl, 6 years ago

In 71942:

explain setting environment variables in grass startup script, see #3462

comment:5 by martinl, 6 years ago

So what is your shell?

env | grep SHELL

comment:6 by mankoff, 6 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)

comment:7 by neteler, 6 years ago

In 71945:

grass.py: explain setting environment variables in grass startup script, see #3462

comment:8 by mankoff, 6 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?

in reply to:  8 comment:9 by martinl, 6 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 ignoring export statements remains. Why can't I export 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

comment:10 by mankoff, 6 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):~ >

in reply to:  10 comment:11 by martinl, 6 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 why export 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 martinl, 6 years ago

Component: DefaultStartup
Milestone: 7.2.3
Type: enhancementdefect

by mankoff, 6 years ago

Attachment: bashrc added

~/.grass7/bashrc showing that setting DYLD_LIBRARY_PATH does not work.

comment:13 by mankoff, 6 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 mankoff, 6 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

comment:15 by mankoff, 6 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.

in reply to:  15 comment:16 by martinl, 6 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.

Last edited 6 years ago by martinl (previous) (diff)

comment:17 by martinl, 6 years ago

In 72095:

grass.py: workaround for bug related to OS X, see #3462

comment:18 by martinl, 6 years ago

Done in r72095, please try out fresh trunk.

comment:19 by martinl, 6 years ago

Cc: grass-dev@… added
Owner: changed from grass-dev@… to martinl
Status: newassigned

comment:20 by martinl, 6 years ago

Does the fix work for you?

comment:21 by mankoff, 6 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).

in reply to:  21 comment:22 by neteler, 6 years ago

Resolution: fixed
Status: assignedclosed

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

I *think* you can close this ticket (again, based on visual inspection of the change).

Backport?

comment:23 by neteler, 6 years ago

Resolution: fixed
Status: closedreopened

Reopen for backport (to 7.4 at least?)

comment:24 by mankoff, 6 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:25 by martinl, 6 years ago

Milestone: 7.2.3

Ticket retargeted after milestone closed

comment:26 by martinl, 6 years ago

Milestone: 7.2.4

comment:27 by wenzeslaus, 6 years ago

Keywords: init grass.py env D_LIBRARY_PATH bashrc added
Milestone: 7.2.47.6.0
Resolution: fixed
Status: reopenedclosed

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.

Note: See TracTickets for help on using tickets.