Skip to content

{lang}[GCCcore/6.4.0] Cython v0.26#4966

Merged
verdurin merged 1 commit intoeasybuilders:developfrom
JackPerdue:20170801065302_new_pr_Cython026
Aug 3, 2017
Merged

{lang}[GCCcore/6.4.0] Cython v0.26#4966
verdurin merged 1 commit intoeasybuilders:developfrom
JackPerdue:20170801065302_new_pr_Cython026

Conversation

@JackPerdue
Copy link
Copy Markdown
Contributor

(created using eb --new-pr)

@JackPerdue
Copy link
Copy Markdown
Contributor Author

Was PR #4923

@verdurin verdurin added this to the 3.4.0 milestone Aug 3, 2017
@verdurin
Copy link
Copy Markdown
Member

verdurin commented Aug 3, 2017

Test report by @verdurin
SUCCESS
Build succeeded for 0 out of 0 (1 easyconfigs in this PR)
ca005.camp.thecrick.org - Linux centos linux 7.3.1611, Intel(R) Xeon(R) CPU E5-2640 v3 @ 2.60GHz, Python 2.7.5
See https://gist.github.com/187fabea23577425647b4f840d29e1c3 for a full test report.

@verdurin
Copy link
Copy Markdown
Member

verdurin commented Aug 3, 2017

Test report by @verdurin
SUCCESS
Build succeeded for 1 out of 1 (1 easyconfigs in this PR)
ca005.camp.thecrick.org - Linux centos linux 7.3.1611, Intel(R) Xeon(R) CPU E5-2640 v3 @ 2.60GHz, Python 2.7.5
See https://gist.github.com/ff02e81408b78bb8588864a3c3626854 for a full test report.

Copy link
Copy Markdown
Member

@verdurin verdurin left a comment

Choose a reason for hiding this comment

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

Looks good

@verdurin
Copy link
Copy Markdown
Member

verdurin commented Aug 3, 2017

Merging, thanks @JackPerdue

@verdurin verdurin merged commit 9367e72 into easybuilders:develop Aug 3, 2017
pyver = '2.7.13'
pyshortver = '2.7'
pysubver = '-bare'
versionsuffix = '-Python-%s%s' % (pyver, pysubver)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

including -Python-<version> in the versionsuffix is meant to indicate that Python is pulled in as a runtime dependency

That's not the case here (it's only a build dep), but it should be since Cython can't be used without having (the right) Python around?

@JackPerdue Can you clarify your intentions here?

I suspect the %(pyver)s and %(pyshortver)s templates didn't work when Python is included as a build dep?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Correct... the templates don't work when it is a builddep.

The point here is avoid building Cython for every toolchain option. e.g. for Python-2.7.13I don't want/need three different versions (intel/iomkl/foss) when one will do. Same for 3.6.2.

I need different ones for 2.7 and 3.6 so that when PythonPackage bite-compiles the .py (to create .pyc) files it uses the right Python and sets PYTHONPATH appropriately.

Python has to be a build dep at this stage (Cython is only a library and serves no real purpose on its one), since it will be later used by something that has the full-blown Python (with MPI/MKL). That is, unless you have some way to make the GCCcore-bare version not conflict with the full-blown version (which you don't).

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

What you are basically doing is dancing around the fact that you're not including Python as a runtime dependency, while it actually is a runtime dependency? To me (currently), that's a sign that this may be the wrong approach.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Not dancing around it at all. '-bare' indicates that it doesn't include the Python needed later. That it, it indicates that this isn't a standalone module but must be paired with a full Python. If you prefer '-build' to '-bare' we can do that. But the Python version has to be specified (I can't use the 2.7 Cython with 3.6)..

@JackPerdue
Copy link
Copy Markdown
Contributor Author

See PR #4962 which is probably where we need to discuss this (versus splitting among PRs).

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