Fix environment cache regression

[bet4it]

Currently, if VIRTUAL_ENV is empty, which will occur when we don't use a virtual environment, the get_cached_default_environment will always clear the cache.
I think it's a regression caused by 862f611.

[davidhalter]

I agree. This attempt at fixing this does use the cache even more. I think we just need to check if $VIRTUALENV is set and then act.

[bet4it]

The code may seem to be weird, but I have tested it and the cache is really used.

[davidhalter]

Yes, but only if $VIRTUALENV is not set.

[bet4it]

Why? get_default_environment will call get_default_environment and get_default_environment will still call os.environ.get('VIRTUAL_ENV').
And I have test it in virtual environment and it works.

[davidhalter]

My logic was a bit off, but what I said still applies. If VIRTUALENV is set, you will always create a new environment.

[bet4it]

I have dug into the code and found what's wrong with me. Apologize for my reckless.
We can surely change the code like what you said, but I think we can don't create subprocess until _get_subprocess is called in Environment class like what SameEnvironment class has done?
If there are still something I have ignored, let me know.

[bet4it]

Sorry, I find that I'm wrong again.
Totally rewrite the commit as what you said.

[bet4it]

Should we consider the situation that people switch virtual environment to real environment?

[heftig]

This was my attempt:

def get_cached_default_environment():
    return _get_cached_default_environment(os.environ.get('VIRTUAL_ENV'))


@time_cache(seconds=10 * 60)  # 10 Minutes
def _get_cached_default_environment(_venv):
    return get_default_environment()

Much simpler, IMO.

[ChanningBJ]

This fix sloved my problem

[blueyed]

@bet4it
Should we go with the patch from @heftig (which is applied to python-jedi in Arch Linux already)?

[davidhalter]

The solution of @heftig has the issue that it doesn't clean the cache if it encounters something different. I don't really like that (because that would mean keeping around processes longer than necessary).

The only remaining issue with this PR is that it does compare with executable instead of _start_executable (they don't need to be the same).

[Yevgnen]

Any news on this ?

[davidhalter]

@bet4it and everyone else: Thanks a lot for your patience. I was very demotivated lately to visit the issue tracker. Therefore this was lingering around here way longer than it should have. Sorry! I guess I will have a bit more motivation once I have the typeshed stuff merged.

There's this one small issue that I'm going to fix, but you don't need to care anymore.

[davidhalter]

I actually just realized that my criticism was wrong anyway. This could have been merged way sooner...

[davidhalter]

I only just realized how bad this issue was. Big sorry for that. I therefore did a release immediately. (0.13.2)

[CarlQLange]

No need for sorry, @davidhalter! Jedi is a great tool and everybody who uses it appreciates all the hard work you put in!