Skip to content

Allow use of start_dir in extensions#2353

Merged
boegel merged 6 commits intoeasybuilders:developfrom
wpoely86:start_dir
Nov 29, 2017
Merged

Allow use of start_dir in extensions#2353
boegel merged 6 commits intoeasybuilders:developfrom
wpoely86:start_dir

Conversation

@wpoely86
Copy link
Copy Markdown
Member

It's really annoying that symmetric between an EasyBlock and ExtensionEasyBlock is so broken. Why does an extension use a different extractor method than the general extractor_step? It should use the same function so we can use the same parameters (like start_dir).

This PR is a dirty hack to fix it.

# name and version properties of EasyBlock are used, so make sure name and version are correct
self.cfg['name'] = self.ext.get('name', None)
self.cfg['version'] = self.ext.get('version', None)
self.cfg['start_dir'] = self.ext.get('options', {}).get('start_dir', None)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Do you have an example easyconfig where you'd like to use this?

Since #2194, I don't think this is actually needed?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Why would it work? The prepare_step is never called if you install an extension under the extensions_step?

Current use case: arrow. It has the main C++ library in a subdirectory that needs to be build and installed first. Afterwards you go to the python subdirectory to build the python extensions.
See: easybuilders/easybuild-easyconfigs#5416

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

OK, now I understand better what the issue is here, you need to prevent that the start_dir value is inherited from the parent, where it's usually defined...

Can you clarify that with a comment above this line?

targetdir = os.path.join(self.master.builddir, remove_unwanted_chars(self.name))
self.ext_dir = extract_file("%s" % self.src, targetdir, extra_options=self.unpack_options)

if self.cfg['start_dir'] and os.path.isdir(self.cfg['start_dir']):
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

use self.start_dir rather than self.cfg['start_dir'] when just reading the value, cfr. #2339 (we should do a cleanup sweep in framework & easyblocks for this)

@boegel boegel added this to the 3.5.0 milestone Nov 27, 2017
self.cfg['name'] = self.ext.get('name', None)
self.cfg['version'] = self.ext.get('version', None)
# We cannot inherit the start_dir from the easyconfig. It should be specified in the extension
# itself or be empty.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Do you mind rephrasing this to:

# We can't inherit the 'start_dir' value from the parent (which will be set, and will most likely be wrong).
# It should be specified for the extension specifically, or be empty (so it is auto-derived).

@boegel boegel merged commit db9350d into easybuilders:develop Nov 29, 2017
@wpoely86 wpoely86 deleted the start_dir branch November 29, 2017 16:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants