Skip to content

METIS easyblock taking into account configopts#494

Merged
boegel merged 9 commits intoeasybuilders:developfrom
hajgato:metis_configopts
Jun 3, 2015
Merged

METIS easyblock taking into account configopts#494
boegel merged 9 commits intoeasybuilders:developfrom
hajgato:metis_configopts

Conversation

@hajgato
Copy link
Copy Markdown
Collaborator

@hajgato hajgato commented Oct 27, 2014

No description provided.

@hpcugentbot
Copy link
Copy Markdown

Automatic reply from Jenkins: Can I test this?

@boegel
Copy link
Copy Markdown
Member

boegel commented Nov 13, 2014

Jenkins: ok to test

@hpcugentbot
Copy link
Copy Markdown

Test PASSed.

@boegel
Copy link
Copy Markdown
Member

boegel commented May 31, 2015

Jenkins: test this please

@hpcugentbot
Copy link
Copy Markdown

Refer to this link for build results (access rights to CI server needed):
https://jenkins1.ugent.be/job/easybuild-easyblocks-pr-builder/984/
Easyblocks unit test suite PASSed (see https://jenkins1.ugent.be/job/easybuild-easyblocks-pr-builder/984/console for more details).

This pull request is now ready for review/testing.

Please try and find someone who can tackle this; contact @boegel if you're not sure what to do.

Comment thread easybuild/easyblocks/m/metis.py Outdated
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.

minor style issue: space after ,

@hpcugentbot
Copy link
Copy Markdown

Refer to this link for build results (access rights to CI server needed):
https://jenkins1.ugent.be/job/easybuild-easyblocks-pr-builder/988/
Easyblocks unit test suite FAILed.

See https://jenkins1.ugent.be/job/easybuild-easyblocks-pr-builder/988/console for more details.

Please fix the reported issues by pushing additional commits to the branch corresponding with this pull request; contact @boegel if you're not sure what to do.

@hpcugentbot
Copy link
Copy Markdown

Refer to this link for build results (access rights to CI server needed):
https://jenkins1.ugent.be/job/easybuild-easyblocks-pr-builder/989/
Easyblocks unit test suite PASSed (see https://jenkins1.ugent.be/job/easybuild-easyblocks-pr-builder/989/console for more details).

This pull request is now ready for review/testing.

Please try and find someone who can tackle this; contact @boegel if you're not sure what to do.

@hajgato
Copy link
Copy Markdown
Collaborator Author

hajgato commented Jun 2, 2015

@boegel metis easyblock corrected

Comment thread easybuild/easyblocks/m/metis.py Outdated
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.

this is a very involved bit of code, why not simply enable building of the .so by always including shared=1?

# in configure_step
self.cfg.update('configopts', 'shared=1')

Alternatively, if you check for shared=1 in self.cfg['configopts'] in configure_step, you don't need to worry about checking whether self.cfg['configopts'] is a list or not (you can assume it'll be a string when in configure_step, in fact, we already do when constructing cmd).

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

METIS makes shared object or archive. Never both. If I use the above mentioned line, I might break old builds, if they would not work with the shared object file.
I had to give a list in my METIS easyconfig to build both archive and shared object.

configopts = [' ', 'shared=1 ']

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, thanks for clarifying that.

But, we should still move the check which libraries should get installed to configure_step, to avoid dealing with the list/string status of configopts.

I'll send a PR your way for that.

Balázs Hajgató and others added 2 commits June 3, 2015 11:20
@hpcugentbot
Copy link
Copy Markdown

Refer to this link for build results (access rights to CI server needed):
https://jenkins1.ugent.be/job/easybuild-easyblocks-pr-builder/999/
Easyblocks unit test suite PASSed (see https://jenkins1.ugent.be/job/easybuild-easyblocks-pr-builder/999/console for more details).

This pull request is now ready for review/testing.

Please try and find someone who can tackle this; contact @boegel if you're not sure what to do.

@hpcugentbot
Copy link
Copy Markdown

Refer to this link for build results (access rights to CI server needed):
https://jenkins1.ugent.be/job/easybuild-easyblocks-pr-builder/1000/
Easyblocks unit test suite PASSed (see https://jenkins1.ugent.be/job/easybuild-easyblocks-pr-builder/1000/console for more details).

This pull request is now ready for review/testing.

Please try and find someone who can tackle this; contact @boegel if you're not sure what to do.

@hpcugentbot
Copy link
Copy Markdown

Refer to this link for build results (access rights to CI server needed):
https://jenkins1.ugent.be/job/easybuild-easyblocks-pr-builder/1001/
Easyblocks unit test suite PASSed (see https://jenkins1.ugent.be/job/easybuild-easyblocks-pr-builder/1001/console for more details).

This pull request is now ready for review/testing.

Please try and find someone who can tackle this; contact @boegel if you're not sure what to do.

@boegel
Copy link
Copy Markdown
Member

boegel commented Jun 3, 2015

Works fine, all existing METIS easyconfigs still work with this in place, see https://gist.github.com/6373a7707218240528a3.

The METIS easyconfig included in easybuilders/easybuild-easyconfigs#1166, for which both libmetis.so and libmetis.a are built works fine too, so going in.

Thanks @hajgato!

boegel added a commit that referenced this pull request Jun 3, 2015
METIS easyblock taking into account configopts
@boegel boegel merged commit bdf5ed2 into easybuilders:develop Jun 3, 2015
@hajgato hajgato deleted the metis_configopts branch October 9, 2015 15:53
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.

3 participants