Skip to content

{devel}[foss/2017a] makedepf90 2.8.8#6504

Merged
migueldiascosta merged 5 commits intoeasybuilders:developfrom
dplacencia:makedepf90
Jul 5, 2018
Merged

{devel}[foss/2017a] makedepf90 2.8.8#6504
migueldiascosta merged 5 commits intoeasybuilders:developfrom
dplacencia:makedepf90

Conversation

@dplacencia
Copy link
Copy Markdown
Contributor

Support for makedepf90

@migueldiascosta
Copy link
Copy Markdown
Member

Test report by @migueldiascosta
SUCCESS
Build succeeded for 1 out of 1 (1 easyconfigs in this PR)
grc-cluster1 - Linux centos 6.9, Intel(R) Xeon(R) CPU E5-2640 0 @ 2.50GHz, Python 2.7.14
See https://gist.github.com/9734afc7e8a6ea749d9e744d3cb49a34 for a full test report.

@migueldiascosta
Copy link
Copy Markdown
Member

@dplacencia is there a specific reason to use such and old toolchain here (and in #6503)?


toolchain = {'name': 'foss', 'version': '2015a'}

sources = ['makedepf90_2.8.8.orig.tar.gz']
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.

please use templates (e.g. '%(name)s_%(version)s.orig.tar.gz')

toolchain = {'name': 'foss', 'version': '2015a'}

sources = ['makedepf90_2.8.8.orig.tar.gz']
source_urls = ['https://launchpad.net/ubuntu/+archive/primary/+sourcefiles/makedepf90/2.8.8-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.

same as above


prebuildopts = 'make depend;'

patches = []
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.

can be removed?


patches = []

dependencies = []
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.

same as above

name = 'makedepf90'
version = '2.8.8'

homepage = 'http://www.iki.fi/erik.edelmann/makedepf90'
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.

url doesn't exist anymore, is there another one?

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.

Hi. That url is the one that appears in official sources, but since it is not available I think it will be better to leave it empty because I did not find another official page.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Or ask Erik where it is now if it still exists.

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.

Following your suggestion I just wrote to Erik right now, thanks for the suggestion akesandgren.

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.

Hi. I need to know if any other change is necessary in this easyblock. Well, I'm waiting for it to be accepted in order to continue working in another (FVCOM)

@dplacencia
Copy link
Copy Markdown
Contributor Author

Hi. I need to know if any other change is necessary in this easyblock. Well, I'm waiting for it to be accepted in order to continue working in another (FVCOM)

@migueldiascosta
Copy link
Copy Markdown
Member

@dplacencia sorry, had missed your earlier comment. There are now two easyconfigs, one for 2015a, unchanged, and another for 2017a, was that intended or did you mean to replace the older one?

@dplacencia
Copy link
Copy Markdown
Contributor Author

The objective was to replace the previous one. Following his subject, I used a more current toolchain and made some changes. If any other changes are necessary, please tell me. Thank you

Copy link
Copy Markdown
Member

@boegel boegel left a comment

Choose a reason for hiding this comment

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

@dplacencia Please remove the other easyconfig file with the old toolchain (git rm easybuild/easyconfigs/m/makedepf90/makedepf90-2.8.8-foss-2015a.eb)

name = 'makedepf90'
version = '2.8.8'

homepage = ''
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.

@dplacencia How about using https://linux.die.net/man/1/makedepf90 instead? Better than leaving it empty imho...

toolchain = {'name': 'foss', 'version': '2017a'}

sources = ['%(name)s_%(version)s.orig.tar.gz']
source_urls = ['https://launchpad.net/ubuntu/+archive/primary/+sourcefiles/%(name)s/%(version)s-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.

@dplacencia Please include a SHA256 checksum, eb makedepf90-2.8.8-foss-2017a.eb --inject-checksums will do this for you


prebuildopts = 'make depend;'


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.

@dplacencia Style nitpicking: please drop duplicate empty line

source_urls = ['https://launchpad.net/ubuntu/+archive/primary/+sourcefiles/%(name)s/%(version)s-1/']
checksums = ['a5118aea198219f59bc04eab0a2099341cecac76a7029c2aef72141645e7596a']

prebuildopts = 'make depend;'
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.

@dplacencia I overlooked this in the previous review sorry...

Please change this to 'make depend && ', i.e. fail early if the make depend fails

@boegel boegel changed the title adding easyconfig: makedepf90/makedepf90-2.8.8-foss-2015a.eb {devel}[foss/2017a] makedepf90 2.8.8 Jul 4, 2018
@boegel
Copy link
Copy Markdown
Member

boegel commented Jul 4, 2018

@migueldiascosta Good to go now?

@boegel
Copy link
Copy Markdown
Member

boegel commented Jul 4, 2018

Test report by @boegel
SUCCESS
Build succeeded for 1 out of 1 (1 easyconfigs in this PR)
node2458.golett.os - Linux centos linux 7.4.1708, Intel(R) Xeon(R) CPU E5-2680 v3 @ 2.50GHz, Python 2.7.5
See https://gist.github.com/5e907a47e8f61a21e9ebbbf862482ca1 for a full test report.

@boegel
Copy link
Copy Markdown
Member

boegel commented Jul 4, 2018

Test report by @boegel
SUCCESS
Build succeeded for 1 out of 1 (1 easyconfigs in this PR)
node2109.delcatty.os - Linux centos linux 7.4.1708, Intel(R) Xeon(R) CPU E5-2670 0 @ 2.60GHz, Python 2.7.5
See https://gist.github.com/8b7003a0f04b940dc88ba93059addb24 for a full test report.

@dplacencia
Copy link
Copy Markdown
Contributor Author

Thanks for the review. Any other change?

@boegel
Copy link
Copy Markdown
Member

boegel commented Jul 4, 2018

@dplacencia OK for me, I'll let @migueldiascosta take another look

@migueldiascosta
Copy link
Copy Markdown
Member

Test report by @migueldiascosta
SUCCESS
Build succeeded for 0 out of 0 (1 easyconfigs in this PR)
grc-cluster1 - Linux centos 6.9, Intel(R) Xeon(R) CPU E5-2640 0 @ 2.50GHz, Python 2.7.14
See https://gist.github.com/91b6eee840870bd5bffc7db1f3b553e7 for a full test report.

Copy link
Copy Markdown
Member

@migueldiascosta migueldiascosta left a comment

Choose a reason for hiding this comment

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

lgtm

@migueldiascosta migueldiascosta added this to the 3.6.2 milestone Jul 5, 2018
@migueldiascosta
Copy link
Copy Markdown
Member

Going in, thanks @dplacencia!

@migueldiascosta migueldiascosta merged commit 6f47a03 into easybuilders:develop Jul 5, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants