Skip to content

Conversation

@paulmelis
Copy link
Contributor

(created using eb --new-pr)

@migueldiascosta migueldiascosta changed the title Add freetype as dependency, as otherwise the build can pick up the system-wide library Add freetype as dependency to OpenImageIO, as otherwise the build can pick up the system-wide library Oct 17, 2019
@migueldiascosta migueldiascosta added this to the next release (4.1.0?) milestone Oct 17, 2019
@migueldiascosta
Copy link
Member

@paulmelis thanks for your contribution, hopefully the first of many :)

although you are only adding a dependency, the tests now require using https for files touched by PRs, please update it (you can do so with --update-pr)

@paulmelis
Copy link
Contributor Author

although you are only adding a dependency, the tests now require using https for files touched by PRs, please update it (you can do so with --update-pr)

I'll fix it, but find it a bit surprising:

  • I based this easyconfig on the version in the official repos, none of which use https for the versions of OpenImageIO in the repo, which where updated last 11 months ago. Was the https-check added after that?
  • Why not also do this check and others as part of the --new-pr command? Might save quite a few unnecessary work cycles for some people, including submitters.

@migueldiascosta
Copy link
Member

migueldiascosta commented Oct 17, 2019

@paulmelis yes, the https check was added after that

the CI tests are much more extensive than what can be done at --new-pr. You are right that it could do a bit more, but it would also require more dependencies to be installed (see, for instance, easybuilders/easybuild-framework#2987)

@migueldiascosta
Copy link
Member

Test report by @migueldiascosta
SUCCESS
Build succeeded for 1 out of 1 (1 easyconfigs in this PR)
generoso - Linux centos linux 7.6.1810, Intel(R) Xeon(R) CPU E5-2660 v4 @ 2.00GHz, Python 3.6.8
See https://gist.github.com/fb1f10351941a6f0194b46c3c3055ef7 for a full test report.

@migueldiascosta
Copy link
Member

@paulmelis this was indeed a problem and your PR fixes it, thanks again

would you consider also fixing the other OpenImageIO easyconfigs?

Copy link
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
Copy link
Member

Going in, thanks @paulmelis!

@migueldiascosta migueldiascosta merged commit 07ae87b into easybuilders:develop Oct 18, 2019
@easybuilders easybuilders deleted a comment from boegelbot Oct 21, 2019
@boegel
Copy link
Member

boegel commented Oct 21, 2019

This should definitely also be fixed in OpenImageIO-1.8.16-intel-2018b.eb to keep things consistent...

@boegel
Copy link
Member

boegel commented Oct 21, 2019

Ah, that was done in #9152, my apologies. :)

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.

3 participants