Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix issue with templates defined by deps being required while still parsing deps #1812

Merged
merged 8 commits into from
Jun 20, 2016

Conversation

boegel
Copy link
Member

@boegel boegel commented Jun 16, 2016

This should fix the problems being reported in #1402 and easybuilders/easybuild-easyconfigs#3210.

cc @verdurin, @ocaisa

@boegel boegel added this to the v2.9.0 milestone Jun 16, 2016
@boegel
Copy link
Member Author

boegel commented Jun 16, 2016

@verdurin, @ocaisa: I'll look into an test that covers this, but it would be great if you could verify this fix (and review the changes).

@verdurin
Copy link
Member

verdurin commented Jun 17, 2016

@boegel having manually applied the new version of easyconfig.py then reloaded the module, I tried this:

eb cnvkit-0.7.3-foss-2015b-Python-2.7.10.eb -dr --force --minimal-toolchains --robot ./

and it worked, without generating the errors I reported in easybuilders/easybuild-easyconfigs#3210

self.log.debug("Obtained minimal toolchain: %s", tc)
else:
self.log.debug("Inheriting parent toolchain %s for dependency %s", tc, dependency)
self.log.debug("Inheriting parent toolchain %s for dependency %s (for now)", tc, dependency)
Copy link
Member

Choose a reason for hiding this comment

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

@boegel - can you clarify what you mean by "(for now)" here? This is an interim fix, or just "for now" as part of the dependency resolution process?

Copy link
Member

Choose a reason for hiding this comment

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

With minimal-toolchains you might be replacing this toolchain with one further down the hierarchy when the dependencies are finalised. Probably could do with replacing (for now) by (until dependencies are finalised)

@ocaisa
Copy link
Member

ocaisa commented Jun 20, 2016

@boegel Tested with Boost-1.60.0-intel-2016a-Python-2.7.11.eb from the main easyconfig repo and parses everything correctly.

@ocaisa
Copy link
Member

ocaisa commented Jun 20, 2016

lgtm

@wpoely86
Copy link
Member

👍

@boegel
Copy link
Member Author

boegel commented Jun 20, 2016

Thanks for the confirmation of the fix @verdurin and @ocaisa, and for the review of the changes!

Going in!

@boegel boegel merged commit f72a76e into easybuilders:develop Jun 20, 2016
@boegel boegel deleted the fix_dep_templates_bug branch June 20, 2016 16:44
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.

4 participants