Skip to content

Conversation

@JackPerdue
Copy link
Contributor

(created using eb --new-pr)

@JackPerdue
Copy link
Contributor Author

OK... working on checksums.... --check-style really should catch this before I upload.

@jose-d
Copy link
Contributor

jose-d commented Mar 18, 2019

OK... working on checksums.... --check-style really should catch this before I upload.

hi, --check-contrib is the right switch for preflight check.

@JackPerdue
Copy link
Contributor Author

re: --check-contrib??? are there any more arcane --check commands I should know about? why is check-contrib more rigorous than check-style and why are there two options to do basically the same thing? How do they differ?

@boegel
Copy link
Member

boegel commented Mar 19, 2019

@JackPerdue --check-contrib is basically --check-style + checking whether SHA256 checksums are in place, see also https://easybuild.readthedocs.io/en/latest/Contributing.html#code-style-review

We felt that checking for SHA256 checksums didn't belong under --check-style, since it's not a code style aspect, but more of a technical/security requirement...

@easybuilders easybuilders deleted a comment from boegelbot Mar 19, 2019
buildininstalldir = True

preinstallopts = """
echo -e "install:\n\t./setup_intel_python.sh" > Makefile
Copy link
Member

Choose a reason for hiding this comment

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

@JackPerdue It looks like this could be handled via the Binary easyblock as well, by specifying install_cmd?

Accelerating Python* performance on modern architectures from Intel.
"""

# l_pythoni2_p_2019.2.066.tar.gz from the Intel site
Copy link
Member

Choose a reason for hiding this comment

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

@JackPerdue We can actually include the source_urls imho, since Intel allows you to simply curl them without authorization (and I was told they're not planning to change this).

We already do this for recent icc, ifort, impi, imkl versions too (and so do other projects).

@boegel boegel added the new label Mar 19, 2019
@boegel boegel added this to the 3.x milestone Mar 19, 2019
@JackPerdue
Copy link
Contributor Author

re: --check-style vs. --check-contrib.... to me just adds confusion... the only time I really care about EB's style is when I want to contribute upstream to EB.... why not just make them one command and just put out a warning if the checksum is missing?

And while on the subject of code checks for PRs.... does EB yet have a tool that will allow users to check the order of their easyconfigs. e.g. easyblock, name, version, versionsuffix, toolchain, toolchainopts, source_urls[1], sources, builddependencies[2], dependencies, ......, moduleclass).
[1] no point in knowing the name of the source if you don't know where to get it
[2] these are only need at build and don't have anything to do with runtime so list first

There was talk of such a feature at one point. It would be nice if it could be merged into --check-contrib (while just dropping or aliasing --check-style)

@JackPerdue
Copy link
Contributor Author

s/versionsuffix, toolchain/versionsuffix, homepage, description, toolchain/

@JackPerdue
Copy link
Contributor Author

checkeborder.sh.txt

Accelerating Python* performance on modern architectures from Intel.
"""

source_urls = ['http://registrationcenter-download.intel.com/akdlm/irc_nas/tec/15293/']
Copy link
Member

Choose a reason for hiding this comment

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

@JackPerdue This is the download URL for 2019 update 3, should be .../15115/?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay... just made a guess based on our Intel account... if you say 15115 then I'll do that

@boegel
Copy link
Member

boegel commented Mar 19, 2019

@JackPerdue W.r.t. also checking order of easyconfig parameter, I started working on it a while ago, but didn't manage to finish it (yet), see easybuilders/easybuild-framework#2165

@boegel
Copy link
Member

boegel commented Mar 21, 2019

Test report by @boegel
SUCCESS
Build succeeded for 2 out of 2 (2 easyconfigs in this PR)
node3100.skitty.os - Linux centos linux 7.6.1810, Intel(R) Xeon(R) Gold 6140 CPU @ 2.30GHz, Python 2.7.5
See https://gist.github.com/78726806cabbbe576859f02a8a45ebc1 for a full test report.

@boegel
Copy link
Member

boegel commented Mar 21, 2019

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

@boegel
Copy link
Member

boegel commented Mar 21, 2019

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

@boegel
Copy link
Member

boegel commented Mar 21, 2019

Going in, thanks @JackPerdue!

@boegel boegel merged commit 16546aa into easybuilders:develop Mar 21, 2019
@migueldiascosta migueldiascosta modified the milestones: 3.x, 3.9.0 Apr 8, 2019
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