sethmlarson / virtualbox-python

Complete implementation of VirtualBox's COM API with a Pythonic interface.
https://pypi.org/project/virtualbox
Apache License 2.0
354 stars 75 forks source link

Check sys.executable/../lib and sys.executable/../Library for packages #67

Closed sethmlarson closed 7 years ago

sethmlarson commented 7 years ago

On systems where Python isn't installed in the standard location we can only rely on sys.executable for hints about where to import vboxapi and other packages from. We should add this additional set of locations to import_vboxapi().

mjdorma commented 7 years ago

Agree. This import is a hack, I may have some code laying about to PR against this issue.

The current importer is doing extra unnecessary evil to path to try to resolve importing pyvbox in a python virtualenv. At the time I didn't realise the proper fix would have been interrogate the site module to see if virtualenv was setup, if so, raise an import error which spells out virtualenv maybe causing issues if it was not prepared using the --system-site-packages flag. I.e:

$ mkvirtualenv --system-site-packages env1

I propose the only additional path to be searched should be that of the vboxapi.

mjdorma commented 7 years ago

If there is a good reason to keep the path fix up hack I'd suggest this as a possible hack#2 improved:

Forget searching site-packages in known places, simply interrogate the file path of a standard lib module that virtualenv never has under its site.

This should point straight to the Libs path for all platforms and site-packages should be found under there.

Alternatives?

sethmlarson commented 7 years ago

Good idea, we can query the sys module as that'll not have anything in front of it in sys.modules (Unless the user intentionally wants to break everything).

I actually don't think the sys.path mangling is too bad of a hack. I say we keep it because a lot of users probably install the VirtualBox SDK via their systems package manager (like I had done in the past) so there's still value in keeping it.

mjdorma commented 7 years ago

Awesome. Sounds like keeping the path extension stuff has value.