Skip to content

Conversation

@JackPerdue
Copy link
Contributor

(created using eb --new-pr)
depends on PR #4969

@boegel boegel changed the title {vis}[GCCcore/6.4.0] X11 v20170805 {vis}[GCCcore/6.4.0] X11 v20170805 (w/ Python 2.7.13) Aug 6, 2017
pyver = '2.7.13'
pyshortver = '2.7'
pysubver = '-bare'
versionsuffix = '-Python-%s%s' % (pyver, pysubver)
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure if we should add -Python-<version> to the versionsuffix just because the installation includes a Python package for xcbgen.

Mentioning Python in the versionsuffix is meant to imply that Python is loaded automatically as a dependency; that way, it's easier to avoid mixing two versions of Python in a large stack of modules that load Python as dependencies.

Since Python is included only as a build dep here, I would drop the versionsuffix.

If anything needs the xcbgen Python package, we should load (the same version of) Python ourselves there as a (build?) dep...

Copy link
Member

Choose a reason for hiding this comment

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

OK, I guess the problem then is that we can't discriminate between an X11 installation that provides xcbgen Python package for Python2 vs Python3.

Rather than installing the whole X11 bundle on top of both Python2 (here) & Python3 (cfr. #4980), we should just installing whatever bits come with Python packages on top of a common X11?

Copy link
Member

Choose a reason for hiding this comment

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

So, the xcbgen Python package is installed via xcb-proto, but as it is currently it's not accessible because the location it gets installed to (e.g. $EBROOTPYTHON/lib/python2.7/site-packages) is not being added to $PYTHONPATH...

I think we should consider blatently removing lib/python* from the X11 bundle installation, or at least try to skip the installation of the Python bindings to xcb-proto.

If the xcbgen Python module is then indeed required somewhere, we can install xcb-proto that does update $PYTHONPATH as a stand-alone on top of X11 and whatever (full) Python is needed.

Thoughts @JackPerdue?

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 think a Python-less X11 would be great. But we need to be sure we can provide the Python libraries needed later.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note: I'm not sure how to do that yet.

Copy link
Contributor Author

@JackPerdue JackPerdue Aug 30, 2017

Choose a reason for hiding this comment

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

Just don't bring this shit back:

# Python is required for xcb-proto
allow_system_deps = [('Python', SYS_PYTHON_VERSION)]
pyshortver = '.'.join(SYS_PYTHON_VERSION.split('.')[0:2])

That reeks.

Copy link
Member

Choose a reason for hiding this comment

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

Providing the Python libraries later (if needed) is trivial: provide an easyconfig for xcb-proto that sits on top of Python and X11.

We don't need the allow_system_deps stuff at all, that was just a way to specify that the system Python could be used.

I suspect the xcb Python package isn't of much use; it clearly hasn't been needed at all up until now since it wasn't available because $PYTHONPATH wasn't extended to include its install location...

@migueldiascosta
Copy link
Member

@JackPerdue, this PR is being closed for the following reason(s): no activity for > 1 year.
Please don't hesitate to reopen this PR or add a comment if you feel this contribution is still relevant.
For more information on our policy w.r.t. closing PRs, see https://easybuild.readthedocs.io/en/latest/Contributing.html#why-a-pull-request-may-be-closed-by-a-maintainer

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants