Skip to content

ensure that $EBEXTSLIST* is also included in generated module under --module-only#2112

Merged
boegel merged 5 commits intoeasybuilders:developfrom
geimer:fix_exts_module_only
Feb 10, 2017
Merged

ensure that $EBEXTSLIST* is also included in generated module under --module-only#2112
boegel merged 5 commits intoeasybuilders:developfrom
geimer:fix_exts_module_only

Conversation

@geimer
Copy link
Copy Markdown
Contributor

@geimer geimer commented Feb 10, 2017

Rather than using the filtered list of extensions built up during installation, use the full list from the easyconfig. Using the filtered list breaks when adding extensions and with --module-only.

Rather than using the filtered list of extensions built up during
installation, use the full list from the easyconfig.  Using the
filtered list breaks when adding extensions and with `--module-only`.
@boegel boegel added this to the 3.2.0 milestone Feb 10, 2017
Comment thread easybuild/framework/easyblock.py Outdated
footer.append(self.make_module_extra_extensions())
extensions = self.cfg['exts_list']
if extensions:
footer.append(self.make_module_extra_extensions(extensions))
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.

hmm, there's little point in passing down self.cfg['exts_list'] here, since self.make_module_extra_extensions also has access to it?

Comment thread easybuild/framework/easyblock.py Outdated
if self.cfg['exts_list']:
footer.append(self.make_module_extra_extensions())
extensions = self.cfg['exts_list']
if extensions:
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.

it should be fine to just drop this condition imho... with self.exts_all we had to be more careful since the default value was None, but self.cfg['exts_list'] is an empty list by default so it's fine to always iterate over it

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Nope. We will generate an empty env var in this case...

Comment thread easybuild/framework/easyblock.py Outdated
exts_list = ','.join(['%s-%s' % (ext['name'], ext.get('version', '')) for ext in self.exts_all])
env_var_name = convert_name(self.name, upper=True)
lines.append(self.module_generator.set_environment('EBEXTSLIST%s' % env_var_name, exts_list))
exts_list = ','.join(['%s-%s' % (ext[0], ext[1]) for ext in extensions])
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.

just use self.cfg['exts_list'] here rather than passing it down extensions

@@ -1054,10 +1054,9 @@ def make_module_extra_extensions(self):
lines = [self.module_extra_extensions]
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Is this still needed now that we use self.cfg['exts_list']?

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.

Yes, this is extra stuff that can go into the module file that is specific to a particular extension, which is returned by the run method.
This is still broken now, since every call to run is being skipped under --module-only...

It's possible to fix this, but that's orthogonal to this fix imho. I'll mention this in #1877, which is sort of a tracking issue for problems with --module-only

@boegel boegel changed the title Fix generation of $EBEXTSLIST ensure that $EBEXTSLIST* is also included in generated module under --module-only Feb 10, 2017
@boegel boegel merged commit a2d6911 into easybuilders:develop Feb 10, 2017
@geimer geimer deleted the fix_exts_module_only branch February 11, 2017 13:30
@boegel boegel modified the milestones: 3.2.0, 3.1.1 Feb 28, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants