tweak easybuild.easyblocks package declaration to better support custom easyblocks repo by boegel · Pull Request #617 · easybuilders/easybuild-easyblocks

boegel · 2015-05-28T19:15:53Z

Using extend_path to 'flatten' the easyblocks namespace results in better support for custom easyblocks repos, i.e. easyblocks that override existing easyblocks can be put in place easily.

This does require adding an (empty) __init__.py file in each letter-subdir.

There's one special case: because the R easyblock clashes with the easybuild.easyblocks.r, an icky from ... import * needs to be put in place in the r/__init__.py file...

For custom easyblock repos, including the following in the easybuild/easyblocks/__init__.py file should suffice with this in place:

subdirs = [chr(l) for l in range(ord('a'), ord('z') + 1)] + ['0']
for subdir in subdirs:
    __path__ = extend_path(__path__, '%s.%s' % (__name__, subdir))

 __path__ = extend_path(__path__, __name__)

Although a custom easyblocks repo may not include letter-subdirs, the extend_path loop still needs to be there, because only the first __init__.py there appears in the Python search path is actually executed.

Preliminary testing shows this works as intended (see below). A unit test will be added to confirm this (and make sure we don't break it).

$ find /tmp/easybuild -type f              
/tmp/easybuild/__init__.py
/tmp/easybuild/easyblocks/__init__.py
/tmp/easybuild/easyblocks/icc.py

$ PYTHONPATH=/tmp:$PYTHONPATH python -c "from easybuild.easyblocks import VERSION; print VERSION"
2.2.0dev

$ PYTHONPATH=/tmp:$PYTHONPATH python -c "import easybuild.easyblocks; import easybuild.easyblocks.icc; print easybuild.easyblocks.icc; from easybuild.easyblocks.icc import EB_icc; print EB_icc.__module__; import easybuild.easyblocks.ifort; print easybuild.easyblocks.ifort"
<module 'easybuild.easyblocks.icc' from '/tmp/easybuild/easyblocks/icc.py'>
easybuild.easyblocks.icc
<module 'easybuild.easyblocks.ifort' from '/Users/kehoste/work/easybuild-easyblocks/easybuild/easyblocks/i/ifort.pyc'>

The reason this works is because extend_path basically scans sys.path, and thus considers all easybuild/easyblocks package directories that are included in the Python search path...

@JensTimmerman: please review?

…om easyblocks repo

hpcugentbot · 2015-05-28T19:17:08Z

Refer to this link for build results (access rights to CI server needed):
https://jenkins1.ugent.be/job/easybuild-easyblocks-pr-builder/980/
Easyblocks unit test suite PASSed (see https://jenkins1.ugent.be/job/easybuild-easyblocks-pr-builder/980/console for more details).

This pull request is now ready for review/testing.

Please try and find someone who can tackle this; contact @boegel if you're not sure what to do.

JensTimmerman · 2015-05-29T08:21:45Z

Does this also deal with the case where a custom easyblock repository has a custom r.py package?

boegel · 2015-05-29T08:31:22Z

In that case, there's no problem, since the path to the custom easyblock repo will be first in the Python search path anyway:

$ find /tmp/easybuild -name '*.py'
/tmp/easybuild/__init__.py
/tmp/easybuild/easyblocks/__init__.py
/tmp/easybuild/easyblocks/icc.py
/tmp/easybuild/easyblocks/r.py

$ PYTHONPATH=/tmp:$PYTHONPATH python -c "import easybuild.easyblocks.r; print easybuild.easyblocks.r.__file__"
/tmp/easybuild/easyblocks/r.pyc

JensTimmerman · 2015-05-29T08:42:41Z

ok, yes, this works because you use
from easybuild.easyblocks.r.r import * and not from .r.r import *

JensTimmerman · 2015-05-29T12:08:59Z

please add some kind of integration test for this.

…m easyblocks repo

hpcugentbot · 2015-05-30T12:59:26Z

Refer to this link for build results (access rights to CI server needed):
https://jenkins1.ugent.be/job/easybuild-easyblocks-pr-builder/981/
Easyblocks unit test suite FAILed.

See https://jenkins1.ugent.be/job/easybuild-easyblocks-pr-builder/981/console for more details.

Please fix the reported issues by pushing additional commits to the branch corresponding with this pull request; contact @boegel if you're not sure what to do.

boegel · 2015-05-30T14:39:01Z

Current implementation of the test doesn't work when an easybuild-easyblocks repo that was installed with easy_install, and hence which is included in an easy-install.pth available through $PYTHONPATH, is around (as is the case on Jenkins).

The problem is that that repo gets precedence over whichever repo is listed first through $PYTHONPATH...

… crap

hpcugentbot · 2015-05-30T15:03:27Z

Refer to this link for build results (access rights to CI server needed):
https://jenkins1.ugent.be/job/easybuild-easyblocks-pr-builder/982/
Easyblocks unit test suite PASSed (see https://jenkins1.ugent.be/job/easybuild-easyblocks-pr-builder/982/console for more details).

This pull request is now ready for review/testing.

Please try and find someone who can tackle this; contact @boegel if you're not sure what to do.

boegel · 2015-05-30T18:15:46Z

Worked around possible complications from easy-install.pth by reconstructing only the required parts of the Python search path (i.e. easybuild-framework and vsc-base) into $PYTHONPATH.

Since the easy-install.pth trickery may make it very hard for people not intimately familiar with Python to set up a custom easyblocks repo such that theur easyblocs are preferred over the ones part of the EasyBuild installation, I'll look into adding support for a dedicated configuration option in the EasyBuild framework to specify the location of own easyblocks (and also toolchains, toolchain components and module naming schemes, while I'm at it). Cfr. easybuilders/easybuild-framework#1010.

JensTimmerman · 2015-06-01T10:13:20Z

You've only fixed this in the tests, so, in real world situations this issue will still pop up?

boegel · 2015-06-01T10:33:19Z

In real world situations, we need something like eb --my-easyblocks=/foo/*.py, which just injects a location for a custom easyblocks repo straight into sys.path. We can also take care of the magic that needs to be included in easybuild/easyblocks/__init__.py, in that case.

Setting this up via $PYTHONPATH is clearly not going to work out in practice, there are too many possible complications.

The changes here are orthogonal to that though, and are still required to make things work as expected.

JensTimmerman · 2015-06-01T12:09:41Z

So the first 'untweaked' testcase is also broken in master?

boegel · 2015-06-01T12:27:19Z

@JensTimmerman: not sure what you mean here... If you use $PYTHONPATH now to make eb consider extra easyblocks, it'll work, the trouble with easy-install.pth is only relevant w.r.t. giving own easyblocks priority over existing ones (which doesn't work with current master due to the way we flatten the easyblocks namespace in __init__.py).

JensTimmerman · 2015-06-01T12:34:45Z

My question is: if you take the unittest before you tweaked it to actually work with a workaround, does that also fail on the current master branch of easybuild?
Just making sure you're not breaking anything that used to work.

boegel · 2015-06-01T12:42:34Z

The unit test tests whether custom easyblocks can overrule an existing easyblock, so yes, it would fail on master. Making that possible is exactly the point of this PR.

boegel · 2015-06-01T12:43:41Z

BTW, the workaround for the failing test is in the test itself (I basically avoid running into an easy-install.pth for the easybuild.framework and vsc namespaces by composing $PYTHONPATH myself.

JensTimmerman · 2015-06-01T12:53:17Z

Can you then please add a test that also tests the current behaviour? Where you can just add an easyblock to the path, and see that it's picked up?

boegel · 2015-06-01T12:57:03Z

@JensTimmerman: it's doing that (too) already, see https://github.com/hpcugent/easybuild-easyblocks/pull/617/files#diff-6c1ff9f6cc8975b3ffa9e414bddf0e10R148

JensTimmerman · 2015-06-01T13:23:53Z

I guess it's fine then

boegel · 2015-06-22T18:30:55Z

documentation will be updated along with easybuilders/easybuild-framework#1301

Thanks for the review @JensTimmerman!

…s_repo tweak easybuild.easyblocks package declaration to better support custom easyblocks repo

tweak easybuild.easyblocks package declaration to better support cust…

41eef50

…om easyblocks repo

add test (and new test module) for verifying interaction with a custo…

e114b43

…m easyblocks repo

make custom easyblocks repo unit test robust against easy-install.pth…

ec41bf2

… crap

boegel mentioned this pull request Jun 18, 2015

add support for --include-* (REVIEW) easybuilders/easybuild-framework#1301

Merged

7 tasks

boegel added a commit that referenced this pull request Jun 22, 2015

Merge pull request #617 from boegel/properly_support_custom_easyblock…

737ea25

…s_repo tweak easybuild.easyblocks package declaration to better support custom easyblocks repo

boegel merged commit 737ea25 into easybuilders:develop Jun 22, 2015

boegel deleted the properly_support_custom_easyblocks_repo branch June 22, 2015 18:31

Conversation

boegel commented May 28, 2015

Uh oh!

hpcugentbot commented May 28, 2015

Uh oh!

JensTimmerman commented May 29, 2015

Uh oh!

boegel commented May 29, 2015

Uh oh!

JensTimmerman commented May 29, 2015

Uh oh!

JensTimmerman commented May 29, 2015

Uh oh!

hpcugentbot commented May 30, 2015

Uh oh!

boegel commented May 30, 2015

Uh oh!

hpcugentbot commented May 30, 2015

Uh oh!

boegel commented May 30, 2015

Uh oh!

JensTimmerman commented Jun 1, 2015

Uh oh!

boegel commented Jun 1, 2015

Uh oh!

JensTimmerman commented Jun 1, 2015

Uh oh!

boegel commented Jun 1, 2015

Uh oh!

JensTimmerman commented Jun 1, 2015

Uh oh!

boegel commented Jun 1, 2015

Uh oh!

boegel commented Jun 1, 2015

Uh oh!

JensTimmerman commented Jun 1, 2015

Uh oh!

boegel commented Jun 1, 2015

Uh oh!

JensTimmerman commented Jun 1, 2015

Uh oh!

boegel commented Jun 22, 2015

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants