add easyblock for CFDEMcoupling #1439
Conversation
| orig_dir = os.path.join(self.builddir, pkg_topdirs[0]) | ||
| move_file(orig_dir, target_dir) | ||
| else: | ||
| raise EasyBuildError("Failed to find subdirectory for %s in %s %s", pkgname, self.builddir, top_dirs) |
There was a problem hiding this comment.
Maybe suggest here that the sources for that package might be missing? If LIGGGTHS is missing easybuild will proceed until this point and then say that the directory hasn't been found, but giving no clue about the reason.
| else: | ||
| raise EasyBuildError("Failed to find subdirectory for %s in %s %s", pkgname, self.builddir, top_dirs) | ||
|
|
||
| # always use env.setvar instead of os.putenv or os.environ for defining environment variables |
There was a problem hiding this comment.
This seems more like a guideline for easyblock developers than a relevant comment for this particular easyblock. I'd remove it.
| env.setvar('CFDEM_PROJECT_DIR', self.cfdem_project_dir) | ||
|
|
||
| # define $CFDEM_PROJECT_USER_DIR to an empty existing directory | ||
| env.setvar('CFDEM_PROJECT_USER_DIR', os.path.join(self.builddir, 'CFDEM_PROJECT_USER_DIR')) |
There was a problem hiding this comment.
CFDEM_PROJECT_USER_DIR seems to me like a weird name for a directory, and it is confusing that the directory name is the same as the environment variable. I suggest to rename it to user_dir or something similar.
|
|
||
| # define $CFDEM_PROJECT_USER_DIR to an empty existing directory | ||
| env.setvar('CFDEM_PROJECT_USER_DIR', os.path.join(self.builddir, 'CFDEM_PROJECT_USER_DIR')) | ||
| mkdir(os.getenv('CFDEM_PROJECT_USER_DIR'), parents=True) |
There was a problem hiding this comment.
Seems odd to set a variable and read it right after. Not that it matters a lot, but it is certainly not efficient. Doesn't it make more sense to define a python variable and use it in both places?
| vtk_root = get_software_root('VTK') | ||
| if vtk_root: | ||
| vtk_ver_maj_min = '.'.join(get_software_version('VTK').split('.')[:2]) | ||
| env.setvar('VTK_INC_USR', '-I%s' % os.path.join(vtk_root, 'include', 'vtk-%s' % vtk_ver_maj_min)) |
There was a problem hiding this comment.
I think it makes sense to check if these directories exists and are not empty.
| else: | ||
| raise EasyBuildError("VTK not included as dependency") | ||
|
|
||
| # can't seem to use defined 'cfdemSysTest' alias... |
There was a problem hiding this comment.
I am not sure I understand the meaning of this comment.......
|
@boegel ping |
|
@boegel ping since this blocks EC PR 6465 which is in the sprint... |
|
Going in, thanks @boegel! |
No description provided.