Skip to content

verify installation of a dummy Python package before actually installing a Python package #475

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.

Sign up for GitHub

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

Closed
boegel wants to merge 1 commit into from
Closed

verify installation of a dummy Python package before actually installing a Python package #475

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
178 changes: 152 additions & 26 deletions easybuild/easyblocks/generic/pythonpackage.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@
@author: Jens Timmerman (Ghent University)
"""
import os
import re
import shutil
import tempfile
from os.path import expanduser
from vsc import fancylogger
Expand All @@ -40,10 +42,37 @@
from easybuild.easyblocks.python import EXTS_FILTER_PYTHON_PACKAGES
from easybuild.framework.easyconfig import CUSTOM
from easybuild.framework.extensioneasyblock import ExtensionEasyBlock
from easybuild.tools.filetools import mkdir, rmtree2, run_cmd
from easybuild.tools.filetools import mkdir, rmtree2, run_cmd, write_file
from easybuild.tools.modules import get_software_version


# test setup.py script for PythonPackage.python_safe_install
TEST_SETUP_PY = """#!/usr/bin/env python
try:
Copy link
Contributor

@stdweird stdweird Sep 13, 2014

Choose a reason for hiding this comment

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

why would you do this? now you don't even know what you are verifying.
and we are dealing with EB installing python packages with an EB installed python and setuptools, right?

Copy link
Member Author

@boegel boegel Mar 11, 2015

Choose a reason for hiding this comment

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

not necessarily, e.g. when installing EasyBuild with EasyBuild, we're typically using the system Python

and even if we think we're using a EB-installed Python with an EB-installed setuptools, another setuptools may 'win' (e.g. one installed with --user)

defining $PYTHONNOUSERSITE would already help there, though...

from setuptools import setup
except ImportError:
from distutils.core import setup

setup(
name='%(pkg)s',
version='1.0',
scripts=['%(pkg)s.py'],
packages=['%(pkg)s'],
data_files=['%(pkg)s.txt'],
provides=['%(pkg)s.py', '%(pkg)s'],
zip_safe=False,
)
"""
TEST_SCRIPT_PY = """#!/usr/bin/env python
import os, sys
sys.stdout.write(os.path.dirname(os.path.abspath(__file__)))
"""
TEST_INIT_PY = """import os, sys
def where():
sys.stdout.write(os.path.dirname(os.path.abspath(__file__)))
"""


def det_pylibdir():
"""Determine Python library directory."""
log = fancylogger.getLogger('det_pylibdir', fname=False)
Expand Down Expand Up @@ -153,28 +182,142 @@ def build_step(self):
cmd = "python setup.py build %s" % self.cfg['buildopts']
run_cmd(cmd, log_all=True, simple=True)

def python_install(self, prefix=None, preinstallopts=None, installopts=None):
"""Install using 'python setup.py install --prefix'."""
if prefix is None:
prefix = self.installdir
if preinstallopts is None:
preinstallopts = self.cfg['preinstallopts']
if installopts is None:
installopts = self.cfg['installopts']

if not self.pylibdir:
self.pylibdir = det_pylibdir()

# create expected directories
abs_pylibdir = os.path.join(prefix, self.pylibdir)
mkdir(abs_pylibdir, parents=True)

# set PYTHONPATH as expected
pythonpath = os.getenv('PYTHONPATH')
env.setvar('PYTHONPATH', ":".join([x for x in [abs_pylibdir, pythonpath] if x is not None]))

# install using setup.py
install_cmd_template = "%(preinstallopts)s python setup.py install --prefix=%(prefix)s %(installopts)s"
cmd = install_cmd_template % {
'preinstallopts': preinstallopts,
'prefix': prefix,
'installopts': installopts,
}
run_cmd(cmd, log_all=True, simple=True)

# setuptools stubbornly replaces the shebang line in scripts with
Copy link
Contributor

@stdweird stdweird Sep 13, 2014

Choose a reason for hiding this comment

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

patch setuptools? (the one used is one EB installed right?)

Copy link
Contributor

@stdweird stdweird Sep 13, 2014

Choose a reason for hiding this comment

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

also, move in separate method

# the full path to the Python interpreter used to install;
# we change it (back) to '#!/usr/bin/env python' here
Copy link
Contributor

@stdweird stdweird Sep 13, 2014

Choose a reason for hiding this comment

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

you need to handle the case where the shebang is followed with options, because this search and repalce trickery will break the script otehrwise

shebang_re = re.compile("^#!/.*python")
Copy link
Contributor

@stdweird stdweird Sep 13, 2014

Choose a reason for hiding this comment

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

define in the if block

bindir = os.path.join(prefix, 'bin')
if os.path.exists(bindir):
for script in os.listdir(bindir):
Copy link
Contributor

@stdweird stdweird Sep 13, 2014

Choose a reason for hiding this comment

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

might be better to use fileinput for this?

Copy link
Contributor

@stdweird stdweird Sep 13, 2014

Choose a reason for hiding this comment

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

and loop over for script in filter(os.path.isfile,map(lambda x: os.path.join(bindir, x), os.listdir(bindir)))
(or make that one a function?)

script = os.path.join(bindir, script)
if os.path.isfile(script):
Copy link
Contributor

@stdweird stdweird Sep 13, 2014

Choose a reason for hiding this comment

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

maybe a if not os.path.isfile(script): continue here to avodi indentation?

try:
txt = open(script, 'r').read()
Copy link
Contributor

@stdweird stdweird Sep 13, 2014

Choose a reason for hiding this comment

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

use filehandle, so you'll open it only once (and close it explicitly)
second, don't read in the whole file, use readline or eg first 4k bytes.
after the read and match if found, don't forget to fh.seek(0) before reading the whole script.
although i'd prefer fileinput thoe all this....

if shebang_re.search(txt):
new_shebang = "#!/usr/bin/env python"
Copy link
Contributor

@stdweird stdweird Sep 13, 2014

Choose a reason for hiding this comment

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

define outside loop?

self.log.debug("Patching shebang header line in %s to '%s'" % (script, new_shebang))
txt = shebang_re.sub(new_shebang, txt)
open(script, 'w').write(txt)
except IOError, err:
self.log.error("Failed to patch shebang header line in %s: %s" % (script, err))

# restore PYTHONPATH if it was set
if pythonpath is not None:
env.setvar('PYTHONPATH', pythonpath)

def python_safe_install(self, **kwargs):
Copy link
Contributor

@stdweird stdweird Sep 13, 2014

Choose a reason for hiding this comment

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

this actually sound like a good test to run after EB installs setupstools

Copy link
Member Author

@boegel boegel Mar 11, 2015

Choose a reason for hiding this comment

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

+1

"""Install using 'python setup.py install --prefix', after verifying it does the right thing."""
cwd = os.getcwd()

# create dummy Python package to verify whether 'python setup.py install --prefix' does the right thing
Copy link
Contributor

@stdweird stdweird Sep 13, 2014

Choose a reason for hiding this comment

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

why don't you ship these files as part of the easyblocks release. is there something defined at runtime that i miss?

tmpdir = tempfile.mkdtemp()
pkg = 'easybuild_pyinstalltest'
mkdir(os.path.join(tmpdir, pkg))
write_file(os.path.join(tmpdir, 'setup.py'), TEST_SETUP_PY % {'pkg': pkg})
test_py_script = '%s.py' % pkg
write_file(os.path.join(tmpdir, test_py_script), TEST_SCRIPT_PY)
test_data_file = '%s.txt' % pkg
write_file(os.path.join(tmpdir, test_data_file), 'data')
write_file(os.path.join(tmpdir, pkg, '__init__.py'), TEST_INIT_PY)

# install dummy Python package
try:
os.chdir(tmpdir)
testinstalldir = tempfile.mkdtemp()
self.python_install(prefix=testinstalldir)
os.chdir(cwd)
except OSError, err:
self.log.error("Failed to move to %s: %s" % (tmpdir, err))
Copy link
Contributor

@stdweird stdweird Sep 13, 2014

Choose a reason for hiding this comment

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

the OSError only comes from the initial chdir?

Copy link
Member Author

@boegel boegel Mar 11, 2015

Choose a reason for hiding this comment

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

or the latter


# verify installation of dummy Python package
verified = True
full_pylibdir = os.path.join(testinstalldir, self.pylibdir)
cmds = [
("python -c 'from %s import where; where()'" % pkg, full_pylibdir),
(test_py_script, testinstalldir),
]
for cmd, out_prefix in cmds:
precmd = "PYTHONPATH=%s:$PYTHONPATH PATH=%s:$PATH" % (full_pylibdir, os.path.join(testinstalldir, 'bin'))
Copy link
Contributor

@stdweird stdweird Sep 13, 2014

Choose a reason for hiding this comment

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

define outside for loop

fullcmd = ' '.join([precmd, cmd])
(out, ec) = run_cmd(fullcmd, simple=False)
tup = (out_prefix, fullcmd)
if out.startswith(out_prefix):
self.log.debug("Found %s in output of '%s' during verification of dummy Python installation" % tup)
else:
tup = (tup[0], tup[1], ec, out)
self.log.warning("%s not found in output of '%s' (exit code: %s, output: %s)" % tup)
verified = False
pyver = get_software_version('Python')
if not pyver:
self.log.error("Python module not loaded.")
pyver = '.'.join(pyver.split('.')[:2])
datainstalldir = os.path.join(full_pylibdir, '%s-1.0-py%s.egg' % (pkg, pyver))
tup = (test_data_file, datainstalldir)
if os.path.exists(os.path.join(datainstalldir, test_data_file)):
self.log.debug("Found file %s in %s during verification of dummy Python installation" % tup)
else:
self.log.warning("Failed to find file %s in %s during verification of dummy Python installation" % tup)
verified = False

if verified:
self.log.debug("Verification of dummy Python installation OK.")
else:
self.log.error("Verification of dummy Python installation failed, setuptools not honoring --prefix?")
Copy link
Contributor

@stdweird stdweird Sep 13, 2014

Choose a reason for hiding this comment

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

so, actually you don't know if it'ssetuptools...

Copy link
Member Author

@boegel boegel Mar 11, 2015

Choose a reason for hiding this comment

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

no, setuptools may (apparently) blatently ignore --prefix

this seems to happen when setuptools itself was installed using --user


# cleanup
shutil.rmtree(testinstalldir)
Copy link
Contributor

@stdweird stdweird Sep 13, 2014

Choose a reason for hiding this comment

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

cleanup the case where it was not installed in the prefix?

shutil.rmtree(tmpdir)

# actually run install command
Copy link
Contributor

@stdweird stdweird Sep 13, 2014

Choose a reason for hiding this comment

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

please don't. keep the tes function separate

Copy link
Contributor

@stdweird stdweird Sep 13, 2014

Choose a reason for hiding this comment

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

i meant it's ok to have a safe install, but the test code should be a separate method

Copy link
Contributor

@stdweird stdweird Sep 13, 2014

Choose a reason for hiding this comment

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

actually, this should be the python_install, and the old should be renamed to _python_unsafe_install and never ever used directly

self.python_install(**kwargs)

def test_step(self):
"""Test the built Python package."""

if isinstance(self.cfg['runtest'], basestring):
self.testcmd = self.cfg['runtest']

if self.cfg['runtest'] and not self.testcmd is None:
extrapath = ""
extrapath = ''
testinstalldir = None

if self.testinstall:
# install in test directory and export PYTHONPATH

# install in test directory and export PYTHONPATH for running tests
try:
testinstalldir = tempfile.mkdtemp()
mkdir(os.path.join(testinstalldir, self.pylibdir), parents=True)
except OSError, err:
self.log.error("Failed to create test install dir: %s" % err)

tup = (self.cfg['preinstallopts'], testinstalldir, self.cfg['installopts'])
cmd = "%s python setup.py install --prefix=%s %s" % tup
run_cmd(cmd, log_all=True, simple=True)
self.python_safe_install(prefix=testinstalldir)

run_cmd("python -c 'import sys; print(sys.path)'") # print Python search path (debug)
extrapath = "export PYTHONPATH=%s:$PYTHONPATH && " % os.path.join(testinstalldir, self.pylibdir)
Expand All @@ -191,23 +334,7 @@ def test_step(self):

def install_step(self):
"""Install Python package to a custom path using setup.py"""

# create expected directories
abs_pylibdir = os.path.join(self.installdir, self.pylibdir)
mkdir(abs_pylibdir, parents=True)

# set PYTHONPATH as expected
pythonpath = os.getenv('PYTHONPATH')
env.setvar('PYTHONPATH', ":".join([x for x in [abs_pylibdir, pythonpath] if x is not None]))

# actually install Python package
tup = (self.cfg['preinstallopts'], self.installdir, self.cfg['installopts'])
cmd = "%s python setup.py install --prefix=%s %s" % tup
run_cmd(cmd, log_all=True, simple=True)

# restore PYTHONPATH if it was set
if pythonpath is not None:
env.setvar('PYTHONPATH', pythonpath)
self.python_safe_install()

def run(self):
"""Perform the actual Python package build/installation procedure"""
Expand All @@ -233,6 +360,5 @@ def sanity_check_step(self, *args, **kwargs):

def make_module_extra(self):
"""Add install path to PYTHONPATH"""

txt = self.moduleGenerator.prepend_paths("PYTHONPATH", [self.pylibdir])
return super(PythonPackage, self).make_module_extra(txt)
33 changes: 2 additions & 31 deletions easybuild/easyblocks/generic/versionindependentpythonpackage.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,35 +54,6 @@ def prepare_step(self):
def install_step(self):
"""Custom install procedure to skip selection of python package versions."""
full_pylibdir = os.path.join(self.installdir, self.pylibdir)

env.setvar('PYTHONPATH', '%s:%s' % (full_pylibdir, os.getenv('PYTHONPATH')))

try:
os.mkdir(full_pylibdir)
except OSError, err:
# this will raise an error and not return
self.log.error("Failed to install: %s" % err)

args = "--prefix=%s --install-lib=%s " % (self.installdir, full_pylibdir)
args = "--install-lib=%s " % full_pylibdir
args += "--single-version-externally-managed --record %s --no-compile" % os.path.join(self.builddir, 'record')
cmd = "python setup.py install %s" % args
run_cmd(cmd, log_all=True, simple=True, log_output=True)

# setuptools stubbornly replaces the shebang line in scripts with
# the full path to the Python interpreter used to install;
# we change it (back) to '#!/usr/bin/env python' here
shebang_re = re.compile("^#!/.*python")
bindir = os.path.join(self.installdir, 'bin')
if os.path.exists(bindir):
for script in os.listdir(bindir):
script = os.path.join(bindir, script)
if os.path.isfile(script):
try:
txt = open(script, 'r').read()
if shebang_re.search(txt):
new_shebang = "#!/usr/bin/env python"
self.log.debug("Patching shebang header line in %s to '%s'" % (script, new_shebang))
txt = shebang_re.sub(new_shebang, txt)
open(script, 'w').write(txt)
except IOError, err:
self.log.error("Failed to patch shebang header line in %s: %s" % (script, err))
self.python_safe_install(installopts=args)
7 changes: 3 additions & 4 deletions easybuild/easyblocks/n/neuron.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,13 +31,13 @@
import re

from easybuild.easyblocks.generic.configuremake import ConfigureMake
from easybuild.easyblocks.generic.pythonpackage import det_pylibdir
from easybuild.easyblocks.generic.pythonpackage import det_pylibdir, PythonPackage
from easybuild.framework.easyconfig import CUSTOM
from easybuild.tools.filetools import run_cmd, adjust_permissions
from easybuild.tools.modules import get_software_root


class EB_NEURON(ConfigureMake):
class EB_NEURON(ConfigureMake, PythonPackage):
"""Support for building/installing NEURON."""

def __init__(self, *args, **kwargs):
Expand Down Expand Up @@ -103,8 +103,7 @@ def install_step(self):
except OSError, err:
self.log.error("Failed to change to %s: %s" % (pypath, err))

cmd = "python setup.py install --prefix=%s" % self.installdir
run_cmd(cmd, simple=True, log_all=True, log_ok=True)
PythonPackage.python_safe_install(self, preinstallopts='', installopts='')

try:
os.chdir(pwd)
Expand Down
6 changes: 2 additions & 4 deletions easybuild/easyblocks/n/numpy.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -174,14 +174,12 @@ def test_step(self):

# temporarily install numpy, it doesn't alow to be used straight from the source dir
tmpdir = tempfile.mkdtemp()
cmd = "python setup.py install --prefix=%s %s" % (tmpdir, self.installopts)
run_cmd(cmd, log_all=True, simple=True)

self.python_safe_install(prefix=tmpdir, installopts=self.installopts)
try:
pwd = os.getcwd()
os.chdir(tmpdir)
except OSError, err:
self.log.error("Faild to change to %s: %s" % (tmpdir, err))
self.log.error("Failed to change to %s: %s" % (tmpdir, err))

# evaluate performance of numpy.dot (3 runs, 3 loops each)
size = 1000
Expand Down
29 changes: 9 additions & 20 deletions easybuild/easyblocks/v/vsc_tools.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,20 +40,13 @@ class EB_VSC_minus_tools(PythonPackage):

def build_step(self):
"""No build procedure for VSC-tools."""

pass

def install_step(self):
"""Custom install procedure for VSC-tools."""

args = "install --prefix=%(path)s --install-lib=%(path)s/lib" % {'path': self.installdir}

pylibdir = os.path.join(self.installdir, 'lib')
env.setvar('PYTHONPATH', '%s:%s' % (pylibdir, os.getenv('PYTHONPATH')))

self.pylibdir = 'lib'
try:
os.mkdir(pylibdir)

pwd = os.getcwd()

pkg_list = ['-'.join(src['name'].split('-')[0:-1]) for src in self.src if src['name'].startswith('vsc')]
Expand All @@ -64,8 +57,7 @@ def install_step(self):
self.log.error("Found none or more than one %s dir in %s: %s" % (pkg, self.builddir, sel_dirs))

os.chdir(os.path.join(self.builddir, sel_dirs[0]))
cmd = "python setup.py %s" % args
run_cmd(cmd, log_all=True, simple=True, log_output=True)
self.python_safe_install(installopts='--install-lib=%s/lib' % self.installdir)

os.chdir(pwd)

Expand All @@ -74,21 +66,18 @@ def install_step(self):

def sanity_check_step(self):
"""Custom sanity check for VSC-tools."""

custom_paths = {
'files': ['bin/%s' % x for x in ['ihmpirun', 'impirun', 'logdaemon', 'm2hmpirun',
'm2mpirun', 'mhmpirun', 'mmmpirun', 'mmpirun',
'mympirun', 'mympisanity', 'myscoop', 'ompirun',
'pbsssh', 'qmpirun', 'sshsleep', 'startlogdaemon',
'fake/mpirun']],
'dirs': ['lib'],
}

'files': ['bin/%s' % x for x in ['ihmpirun', 'impirun', 'logdaemon', 'm2hmpirun',
'm2mpirun', 'mhmpirun', 'mmmpirun', 'mmpirun',
'mympirun', 'mympisanity', 'myscoop', 'ompirun',
'pbsssh', 'qmpirun', 'sshsleep', 'startlogdaemon',
'fake/mpirun']],
'dirs': ['lib'],
}
super(EB_VSC_minus_tools, self).sanity_check_step(custom_paths=custom_paths)

def make_module_extra(self):
"""Add install path to PYTHONPATH"""

txt = super(EB_VSC_minus_tools, self).make_module_extra()

txt += self.moduleGenerator.prepend_paths('PATH', ["bin/fake"])
Expand Down