Tkinter easyblock by wpoely86 · Pull Request #1184 · easybuilders/easybuild-easyblocks

wpoely86 · 2017-05-21T18:35:33Z

Follow up from #1118

* origin/develop: (114 commits) enable building of shared FFTW libraries enhance sanity check for NCL Revert "update NCL easyblock to support building in parallel" (re-)bump to v3.3.0.dev0 bump version to v3.2.1, clarify updates to NAMD easyblock in release notes prepare release notes for eb321 minor style fixes in GROMACS w/ PLUMED support only use -std=c++11 for NAMD 2.12 and up fix specifying FFT(W) dirs + minor style fixes Move plumed patching step. Add openmp support to namd easyblock. Also add TCL support. Fix some more comments from Kenneth. bump to v3.3.0.dev0 bump version to 3.2.0 + minor enhancements to release notes prepare release notes for eb320 integrate support for open-source version of TBB into existing TBB easyblock update NCL easyblock to support building in parallel PoC for the TBB open source version add support to install Python extensions without unpacking Improvement, per reviewer suggestion ...

boegel · 2017-05-22T09:08:40Z

+@author: Dries Verdegem (Ghent University)
+@author: Kenneth Hoste (Ghent University)
+@author: Pieter De Baets (Ghent University)
+@author: Jens Timmerman (Ghent University)


drop these above, only you and Adam are actual authors of this?

boegel · 2017-05-22T09:10:32Z

+
+        custom_paths = {
+            'files': [],
+            'dirs': ['lib']


Make this more specific, just include 'files': [tkinter_so]?

Should we also check whether nothing else is in the install dir?

Well, we delete everything in there and copy only the tk parts back?

tkinter_so is a glob.

boegel · 2017-05-22T09:11:05Z

+        """Set PYTHONPATH"""
+        txt = super(EB_Tkinter, self).make_module_extra()
+        pyver = '.'.join(self.version.split('.')[:2])
+        txt += self.module_generator.prepend_paths('PYTHONPATH', os.path.join("lib", "python%s" % pyver))


use self.all_pylibdirs?

boegel · 2017-05-22T09:11:46Z

+        shlib_ext = get_shared_lib_ext()
+
+        # check whether _tkinter*.so is found, exact filename doesn't matter
+        tkinter_so = os.path.join(self.installdir, 'lib', pyver, '_tkinter*.' + shlib_ext)


use all_pylibdirs rather than hardcoding lib?

boegel · 2017-05-22T09:13:18Z

+        mkdir(tmpdir)
+
+        pyver = '.'.join(self.version.split('.')[:2])
+        libdir = os.path.join(self.installdir, "lib", "python%s" % pyver)


same here, use all_pylibdirs?

boegel · 2017-05-22T09:13:44Z

+        super(EB_Tkinter, self).install_step()
+
+        tmpname = "eb-tmp"
+        tmpdir = os.path.join(self.installdir, tmpname)


I'd make this a subdir of self.builddir, which is cleaned up automagically?

also, use tempfile.mkdtemp, you can tell it in which location to create a directory, and specify a prefix for the name (eb-tmp- for example)

that possible on a different filesystem. The reason why I do it like this is to have a real 'move' on the same fs.

Is that really going to matter for the amount of files involved?

Are you worried about speed, or something else?

Not sure, some people do weird things out there. But I don't feel strongly about this. I can change if that makes you happy.

Well, it's a lot cleaner to use the build dir for this, since it's cleaned up automagically...

boegel · 2017-05-22T09:13:59Z

+
+        copy([os.path.join(libdir, x) for x in tkdirs], tmpdir)
+
+        delete_dirs = [x for x in os.listdir(self.installdir) if not x == tmpname]


x != tmpname

and if you use self.builddir in tmpdir, you don't even need to do this, you can just wipe self.installdir entirely?

boegel · 2017-05-22T09:15:28Z

+        shutil.move(os.path.join(tmpdir, tkdirs[0]), libdir)
+        shutil.move(os.path.join(tmpdir, os.path.basename(tkdirs[1])), libdir)
+
+        rmtree2(tmpdir)


no need for this if tmpdir uses self.builddir

boegel · 2017-05-22T09:16:41Z

+
+        pyver = '.'.join(self.version.split('.')[:2])
+        libdir = os.path.join(self.installdir, "lib", "python%s" % pyver)
+        tkdirs = ["lib-tk", "lib-dynload/_tkinter.so"]


not all of these are dirs, but they are all files (somewhat) ;)

boegel · 2017-05-22T09:18:23Z

+    """
+
+    def configure_step(self):
+        """Check for Tk"""


Check for Tk before configuring

boegel · 2017-05-23T12:45:31Z

+from distutils.version import LooseVersion
+
+from easybuild.easyblocks.generic.pythonpackage import det_pylibdir
+from easybuild.easyblocks.p.python import EB_Python


don't use the .p., import directly from easybuild.easyblocks.python

boegel · 2017-05-23T12:47:11Z

+
+        mkdir(pylibdir, parents=True)
+        shutil.move(os.path.join(tmpdir, tkparts[0]), pylibdir)
+        shutil.move(os.path.join(tmpdir, os.path.basename(tkparts[1])), pylibdir)


wrap this in try/except OSError as err:?

boegel · 2017-05-23T12:47:58Z

+            'files': ['%s/_tkinter.%s' % (pylibdir, shlib_ext)],
+            'dirs': ['lib']
+        }
+


drop this empty line?

boegel · 2017-05-23T17:02:13Z

+        try:
+            shutil.move(os.path.join(tmpdir, tkparts[0]), pylibdir)
+            shutil.move(os.path.join(tmpdir, os.path.basename(tkparts[1])), pylibdir)
+        except:


include the exception in the error message, so it's clear what exactly went wrong...

except OSError as err: raise EasyBuildError("Failed to move Tkinter back to the install directory: %s", err)

boegel · 2017-05-23T20:11:01Z

lgtm, and tested with easybuilders/easybuild-easyconfigs#4620, thanks @wpoely86 and @verdurin for levelling the ground for this!

verdurin and others added 3 commits February 28, 2017 10:56

Initial version of tkinter easyblock

36d2a50

Clean up Tkinter

0ce124d

wpoely86 mentioned this pull request May 21, 2017

{lang} [intel/2017a] Tkinter 2.7.13 easybuilders/easybuild-easyconfigs#4620

Merged

wpoely86 added 2 commits May 22, 2017 09:34

Fix bugs in tkinter

c0b7cca

Rename to tkinter

15c3b7e

boegel requested changes May 22, 2017

View reviewed changes

boegel added this to the 3.3.0 milestone May 22, 2017

Fix remarks

9240e23

boegel requested changes May 23, 2017

View reviewed changes

Fix remarks

582556b

boegel requested changes May 23, 2017

View reviewed changes

Fix remarks

e74c590

boegel approved these changes May 23, 2017

View reviewed changes

boegel merged commit 5a25efb into easybuilders:develop May 23, 2017

wpoely86 deleted the tkinter branch May 23, 2017 20:18


		copy([os.path.join(libdir, x) for x in tkdirs], tmpdir)

		delete_dirs = [x for x in os.listdir(self.installdir) if not x == tmpname]

Conversation

wpoely86 commented May 21, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

boegel commented May 23, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants