Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added work around getsitepackages Issue #715 #1031

Merged
merged 1 commit into from
Jul 3, 2018

Conversation

akshit-sharma
Copy link
Contributor

Support for building openage under a virtualenv. Issue #715

@@ -29,7 +29,7 @@ if(NOT CMAKE_PY_INSTALL_PREFIX)
if(MSVC)
set(CMAKE_PY_INSTALL_PREFIX "python")
else()
py_exec("import site, os; print(os.path.normpath(site.getsitepackages()[0]))" PREFIX)
py_exec("import os; print(os.path.dirname(os.__file__) + '/site-packages')" PREFIX)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This breaks the installation for distribution packaging because there the destination path must be the sitepackages directory. What is the intention of this fix, or rather: Why doesn't it work with a virtualenv currently? To solve this correctly, we'll need a more complete solution.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would like to quote samkellet from issue #715.
virtualenv has its own site.py which does not have getsitepackages defined.

I was not able to compile on my machine till I changed it.
This also build (kevin-ci) without errors so I thought it was good.

there is an issue for virtualenv saying site.getsitepackages() is missing.

Copy link
Member

@TheJJ TheJJ Jul 3, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please excuse me, you are right, i did not realize the os.__file__'s effect.

Maybe we can improve it further to not depend on this "os.py is one folder above site packages":
https://stackoverflow.com/a/122340/1348535
so we would have:

from distutils.sysconfig import get_python_lib; print(get_python_lib())

There is another answer saying that this is false on Ubuntu, but I think it's the path we need.

@TheJJ TheJJ added improvement Enhancement of an existing component area: buildsystem Related to our cmake/python buildsystem labels Jul 1, 2018
@akshit-sharma
Copy link
Contributor Author

from distutils.sysconfig import get_python_lib; print(get_python_lib()) also works for me.

I have updated my pull request.

@TheJJ TheJJ merged commit a554ab4 into SFTtech:master Jul 3, 2018
@TheJJ
Copy link
Member

TheJJ commented Jul 3, 2018

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: buildsystem Related to our cmake/python buildsystem improvement Enhancement of an existing component
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants