Fix environment variable overrides

[tokejepsen]

Problem

Override environment variables like AVALON_CORE are appended to PYTHONPATH resulting in the built-in submodules always getting loaded first.

Solution

Prepend override environment variables.

[mottosso]

I'm not sure this is right.

If you don't set AVALON_CORE, you should end up with the one provided in this repo. If you do provide it, then it would get caught here.

E.g.

$ set AVALON_CORE=c:\my\avalon-core
$ avalon

Can you confirm the above isn't working for you?

[tokejepsen]

C:\Users\admin\avalon-docker\volume>set AVALON_CORE=C:\Users\admin\core

C:\Users\admin\avalon-docker\volume>avalon
PYTHONPATH:
C:\Users\admin\avalon-docker\volume\bin\pythonpath;C:\Users\admin\avalon-docker\volume\git\cgwire-gazu;C:\Users\admin\avalon-docker\volume\git\pyblish-qml;C:\Users\admin\avalon-docker\volume\git\pyblish-base;C:\Users\admin\avalon-docker\volume\git\mindbender-config;C:\Users\admin\avalon-docker\volume\git\avalon-launcher;C:\Users\admin\avalon-docker\volume\git\avalon-core;C:\Users\admin\avalon-docker\volume\bin\pythonpath;C:\Users\admin\avalon-docker\volume\git\pyblish-base;C:\Users\admin\avalon-docker\volume\git\pyblish-qml;C:\Users\admin\avalon-docker\volume\git\avalon-launcher;C:\Users\admin\core;C:\Users\admin\avalon-docker\volume\git\mindbender-config
Using Python @ 'C:\Users\admin\avalon-docker\volume\bin\windows\python36\python.exe'
Using PyQt5 @ 'C:\Users\admin\avalon-docker\volume\bin\windows\python36\lib\site-packages\PyQt5'
Using core @ 'C:\Users\admin\core'
Using launcher @ 'C:\Users\admin\avalon-docker\volume\git\avalon-launcher'
Using root @ 'C:\Users\admin\avalon-docker\volume\git\avalon-examples\projects'
Using config: 'polly'
Starting avalon-launcher
Registering default actions..
Registering config actions..
Current configuration `polly` has no 'register_launcher_actions'
initialising..
ready
Success

Notice C:\Users\admin\core is after C:\Users\admin\avalon-docker\volume\git\avalon-core in PYTHONPATH.

[mottosso]

Ok, I'm not sure why you would have core on your PYTHONPATH prior to launching Avalon; but I suppose if you did then this PR would "fix" that.

It's a little confusing however, as now there are four permutations involved in which core you're actually ending up with.

1. No PYTHONPATH, no AVALON_CORE = default
2. No PYTHONPATH, AVALON_CORE = AVALON_CORE
3. PYTHONPATH, no AVALON_CORE = PYTHONPATH
4. PYTHONPATH, AVALON_CORE = AVALON_CORE

I've noticed that terminal.bat actually sets the PYTHONPATH. It shouldn't do that. I think we should assume (or make sure) that PYTHONPATH is either empty or doesn't contain anything Avalon prior to launch, otherwise this is a little confusing.

Should we make the .bat files clear the PYTHONPATH automatically, so as to avoid any of this? We could have a AVALON_PYTHONPATH for additional "extra" Python libraries the user may want to have on his path prior to launching, that gets added by the CLI on launch.

[tokejepsen]

I've noticed that terminal.bat actually sets the PYTHONPATH. It shouldn't do that. I think we should assume (or make sure) that PYTHONPATH is either empty or doesn't contain anything Avalon prior to launch, otherwise this is a little confusing.

This might be the actual issue then.

Should we make the .bat files clear the PYTHONPATH automatically, so as to avoid any of this?

I think clearing PYTHONPATH would certainly help with debugging in the future if anything.

We could have a AVALON_PYTHONPATH for additional "extra" Python libraries the user may want to have on his path prior to launching, that gets added by the CLI on launch.

I have been thinking about all these environment variables and whether we could collect them all under one? As we continue to add libraries to avalon-docker, we would need to keep an environment variable for each. For example cgwire gazu currently does not have an override variable.

The implementation would look something like;

1. Developer maybe adds a custom python library to AVALON_PYTHONPATH
2. avalon_cli copies AVALON_PYTHONPATH to PYTHONPATH
3. avalon_cli test imports each required python library and adds it to PYTHONPATH is the import fails.

This would allow developers to override any library now and in the future without any code change.

[tokejepsen]

One thing to keep in mind is that AVALON_CORE and AVALON_LAUNCHER seems to be required currently.

We could just find the modules and add the environment variables for a transition.

[tokejepsen]

Have adjusted this PR to the discussion.

For this to function, we need get https://github.com/getavalon/launcher/pull/37 merged and update the launcher.

[tokejepsen]

The reason why Travis is failing is because of how we are running the CLI tests.

Before terminal.sh would set PYTHONPATH, so once tests\test_cli.sh runs, the environment is correct. The trouble now is that the environment setup is in a python subprocess so the environment variables won't get set in the terminal.sh process.

[tokejepsen]

I think the terminal scripts are working, but its not a very good solution atm.

A better solution would be to have a method in python where we define and return an environment, similar to _install, but we execute this in the terminal scripts and set the environment. My knowledge of batch and bash are not good enough for that yet, but that is the best solution I can think of now.

[tokejepsen]

I think this is a much more elegant and simple solution.

The terminal entry points only have PATH hardcoded to add the built-in python and working directory for avalon. The rest of the environment comes from avalon --environment. Technically we output the contents of avalon --environment to a batch/shell script and then execute that.

This is ready to be merged IMO :)

[tokejepsen]

This is currently waiting on getting https://github.com/getavalon/launcher/pull/37 merged.