Make FDTD_Solutions easyblock do the install with copytree instead of rpm rebuild #1307

akesandgren · 2017-12-08T13:12:07Z

rpm installation is not kosher on Debian based systems.

…rpm rebuild. rpm installation is not kosher on Debian based systems.

mboisson · 2018-01-11T20:31:36Z

FWIW, I have used this easyblock successfully to install FDTD Solutions on Compute Canada servers.

…m-rebuild

houndci-bot · 2018-09-28T10:48:50Z

easybuild/easyblocks/f/fdtd_solutions.py

-        return guesses
+            raise EasyBuildError("Failed to copy %s to %s: %s",
+	os.path.join(self.cfg['start_dir'], 'opt', 'lumerical', 'fdtd'),
+	self.installdir, err)


indentation contains tabs
continuation line missing indentation or outdented

houndci-bot · 2018-09-28T10:48:51Z

easybuild/easyblocks/f/fdtd_solutions.py

-        })
-        return guesses
+            raise EasyBuildError("Failed to copy %s to %s: %s",
+	os.path.join(self.cfg['start_dir'], 'opt', 'lumerical', 'fdtd'),


indentation contains tabs
indentation contains mixed spaces and tabs
continuation line under-indented for visual indent

houndci-bot · 2018-09-28T10:48:51Z

easybuild/easyblocks/f/fdtd_solutions.py

+            # shutil.copytree doesn't allow the target directory to exist already
+            rmtree2(self.installdir)
+            shutil.copytree(os.path.join(self.cfg['start_dir'], 'opt', 'lumerical', 'fdtd'),
+                                    self.installdir, symlinks=self.cfg['keepsymlinks'])


continuation line over-indented for visual indent

boegel · 2018-10-30T13:13:42Z

easybuild/easyblocks/f/fdtd_solutions.py

-            shutil.copy2(rebuilt_rpms[0], rpms[0])
+            # shutil.copytree doesn't allow the target directory to exist already
+            rmtree2(self.installdir)
+            shutil.copytree(os.path.join(self.cfg['start_dir'], 'opt', 'lumerical', 'fdtd'),


@akesandgren This should be updated to use copy_dir from filetools (no need for the try/except anymore then)

Also, please define a variable with the path you're copying, since you're using it in two places (here + the error message)

ftdt_dir = os.path.join(self.cfg['start_dir'], 'opt', 'lumerical', 'fdtd') copy_dir(ftdt_dir, self.installdir, symlinks=self.cfg['keepsymlinks'])

boegel · 2018-10-30T13:15:56Z

easybuild/easyblocks/f/fdtd_solutions.py


    def sanity_check_step(self):
        """Custom sanity check for FDTD Solutions."""
        custom_paths = {
            'files': [],
-            'dirs': ['opt/lumerical/fdtd/bin', 'opt/lumerical/fdtd/lib'],
+            'dirs': ['bin', 'lib'],


@akesandgren It's probably worth checking for some specific files at least?

…m-rebuild

Add some actual files to sanity_check and use a dir not checked by files.

houndci-bot · 2018-10-30T14:12:57Z

easybuild/easyblocks/f/fdtd_solutions.py

-        return guesses
+        """Install FDTD Solutions using copy tree."""
+        fdtd_dir = os.path.join(self.cfg['start_dir'], 'opt', 'lumerical', 'fdtd')
+        copy_dir(ftdt_dir, self.installdir, symlinks=self.cfg['keepsymlinks'])


undefined name 'copy_dir'
undefined name 'ftdt_dir'

easybuild/easyblocks/f/fdtd_solutions.py

houndci-bot · 2018-10-30T14:12:57Z

easybuild/easyblocks/f/fdtd_solutions.py

@@ -33,64 +33,39 @@
 from easybuild.easyblocks.generic.packedbinary import PackedBinary
 from easybuild.easyblocks.generic.rpm import rebuild_rpm
 from easybuild.tools.build_log import EasyBuildError
-from easybuild.tools.run import run_cmd_qa
+from easybuild.tools.filetools import rmtree2
+from easybuild.tools.run import run_cmd, run_cmd_qa


'easybuild.tools.run.run_cmd_qa' imported but unused

houndci-bot · 2018-10-30T14:12:57Z

easybuild/easyblocks/f/fdtd_solutions.py

@@ -33,64 +33,39 @@
 from easybuild.easyblocks.generic.packedbinary import PackedBinary
 from easybuild.easyblocks.generic.rpm import rebuild_rpm
 from easybuild.tools.build_log import EasyBuildError
-from easybuild.tools.run import run_cmd_qa
+from easybuild.tools.filetools import rmtree2


'easybuild.tools.filetools.rmtree2' imported but unused

And fix typo.

boegel · 2018-10-31T10:01:18Z

easybuild/easyblocks/f/fdtd_solutions.py

-        return guesses
+        """Install FDTD Solutions using copy tree."""
+        fdtd_dir = os.path.join(self.cfg['start_dir'], 'opt', 'lumerical', 'fdtd')
+        copy_dir(fdtd_dir, self.installdir, symlinks=self.cfg['keepsymlinks'])


@akesandgren Installation fails now because the install directory is pre-created, and copy_dir doesn't like that.

You'll need to either call remove_dir(self.installdir)s first, or override the make_installdir method to make it a no-op (just log a message like "Not pre-creating installation directory %s" % self.installdir instead)

Something like that then?

I overlooked that make_installdir is also responsible for removing an existing installation directory in case of a forced reinstall, so you should (also) set self.cfg['dontcreateinstalldir'] = True before calling the parent:

def make_installdir(self): """Override installdir creation""" self.log.warning("Not pre-creating installation directory %s" % self.installdir) self.cfg['dontcreateinstalldir'] = True super(EB_EB_FDTD_underscore_Solutions, self).make_installdir()

…_installdir.

…stalldir=True Otherwise a forced reinstall won't remove the existing installdir.

houndci-bot · 2018-10-31T16:49:44Z

easybuild/easyblocks/f/fdtd_solutions.py

+        """Override installdir creation"""
+        self.log.warning("Not pre-creating installation directory %s" % self.installdir)
+        self.cfg['dontcreateinstalldir'] = True
+        super(EB_EB_FDTD_underscore_Solutions, self).make_installdir()


undefined name 'EB_EB_FDTD_underscore_Solutions'

boegel · 2018-11-01T08:31:17Z

easybuild/easyblocks/f/fdtd_solutions.py

-            'files': [],
-            'dirs': ['opt/lumerical/fdtd/bin', 'opt/lumerical/fdtd/lib'],
+            'files': ['bin/fdtd-solutions', 'lib/fdtd-engine-mpichp4_libFNP.so'],
+            'dirs': ['api'],


@akesandgren There no api installed for the existing FDTD_Solutions easyconfigs (8.6.2 and 8.11.337).

And for 8.6.2, there's no lib/fdtd-engine-mpichp4_libFNP.so either (not even close).

Gah.... I only have 8.16.982 to look at...
Any suggestions?

How about just checking for bin/fdtd-solutions and a non-empty lib dir?

That's still a little bit stricter than what was there (which was basically just checking for bin & lib)...

checking for a non-empty lib? what do you mean? I can check for an existing lib dir like it was done before, but how can i check for a non-empty one without checking for a specific file?

Or did you just mean an existing lib dir?

Correct me if I'm wrong, but 'dirs': ['lib'] already fails if the lib directory is empty.

It does? Didn't really know that, but if that's the case it's good enough for me :-)

In easyblock.py :

2140 path_keys_and_check = { 2141 # files must exist and not be a directory 2142 'files': ('file', lambda fp: os.path.exists(fp) and not os.path.isdir(fp)), 2143 # directories must exist and be non-empty 2144 'dirs': ("(non-empty) directory", lambda dp: os.path.isdir(dp) and os.listdir(dp)), 2145 }

Exactly, the dirs entries need to exist & be non-empty. Checking for an empty dir is kind of useless. ;)

…th all versions.

boegel

lgtm, now still works with the existing easyconfigs FDTD_Solutions-8.11.337.eb and FDTD_Solutions-8.6.2.eb

Make fdtd_solutions.py do the install with copytree instead of using …

8194ae8

…rpm rebuild. rpm installation is not kosher on Debian based systems.

Merge branch 'develop' into make-fdtd_solutions-do-install-without-rp…

1eb434a

…m-rebuild

houndci-bot reviewed Sep 28, 2018

View reviewed changes

Fix style issues

66d5d2e

boegel added this to the next release milestone Oct 10, 2018

migueldiascosta mentioned this pull request Oct 30, 2018

{cae}[dummy/dummy] FDTD_Solutions v8.20.1731 easybuilders/easybuild-easyconfigs#7088

Merged

boegel requested changes Oct 30, 2018

View reviewed changes

boegel added the bug fix label Oct 30, 2018

akesandgren added 2 commits October 30, 2018 15:01

Merge branch 'develop' into make-fdtd_solutions-do-install-without-rp…

f22225d

…m-rebuild

Change to using copy_dir.

4c6ac06

Add some actual files to sanity_check and use a dir not checked by files.

houndci-bot reviewed Oct 30, 2018

View reviewed changes

akesandgren added 3 commits October 30, 2018 15:14

fdtd_solutions.py: forgot to import copy_dir

b29e67e

And fix typo.

fdtd_solutions.py: remove unused run_cmd_qa import

ef4a4df

fdtd_solutions.py: Remove unused import rmtree2

9db1b06

boegel requested changes Oct 31, 2018

View reviewed changes

akesandgren added 2 commits October 31, 2018 12:18

fdtd_solutions.py: copy_dir fails if target dir exists, override make…

351a3af

…_installdir.

fdtd_solutions.py: Need to call make_installdir but with dontcreatein…

5a2ff78

…stalldir=True Otherwise a forced reinstall won't remove the existing installdir.

houndci-bot reviewed Oct 31, 2018

View reviewed changes

fdtd_solutions.py: Fix typo.

e8d41fb

boegel requested changes Nov 1, 2018

View reviewed changes

fdtd_solutions.py: Change sanity check to something that will work wi…

bdadd97

…th all versions.

boegel changed the title ~~Make fdtd_solutions.py do the install with copytree instead of rpm rebuild~~ Make FDTD_Solutions easyblock do the install with copytree instead of rpm rebuild Nov 1, 2018

boegel approved these changes Nov 1, 2018

View reviewed changes

boegel merged commit cd961d4 into easybuilders:develop Nov 1, 2018

akesandgren deleted the make-fdtd_solutions-do-install-without-rpm-rebuild branch November 1, 2018 18:45

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make FDTD_Solutions easyblock do the install with copytree instead of rpm rebuild #1307

Make FDTD_Solutions easyblock do the install with copytree instead of rpm rebuild #1307

akesandgren commented Dec 8, 2017

mboisson commented Jan 11, 2018

houndci-bot Sep 28, 2018

houndci-bot Sep 28, 2018

houndci-bot Sep 28, 2018

boegel Oct 30, 2018

boegel Oct 30, 2018

houndci-bot Oct 30, 2018

houndci-bot Oct 30, 2018

houndci-bot Oct 30, 2018

boegel Oct 31, 2018

akesandgren Oct 31, 2018

boegel Oct 31, 2018

houndci-bot Oct 31, 2018

boegel Nov 1, 2018

akesandgren Nov 1, 2018

boegel Nov 1, 2018

akesandgren Nov 1, 2018

mboisson Nov 1, 2018

akesandgren Nov 1, 2018

mboisson Nov 1, 2018

boegel Nov 1, 2018

boegel left a comment

Make FDTD_Solutions easyblock do the install with copytree instead of rpm rebuild #1307

Make FDTD_Solutions easyblock do the install with copytree instead of rpm rebuild #1307

Conversation

akesandgren commented Dec 8, 2017

mboisson commented Jan 11, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

boegel left a comment

Choose a reason for hiding this comment