Skip to content

{bio}[system/system] Godon v20210913 (x86_64)#16574

Merged
boegel merged 3 commits intoeasybuilders:developfrom
sib-swiss:20221108094459_new_pr_Godon20210913
Nov 9, 2022
Merged

{bio}[system/system] Godon v20210913 (x86_64)#16574
boegel merged 3 commits intoeasybuilders:developfrom
sib-swiss:20221108094459_new_pr_Godon20210913

Conversation

@SIB-software
Copy link
Copy Markdown
Contributor

(created using eb --new-pr)

@smoretti smoretti added new BioHack2022 Related to EU BioHackathon 2022 labels Nov 8, 2022
toolchain = SYSTEM

source_urls = ['https://bitbucket.org/Davydov/godon/downloads']
sources = ['godon-master-linux-gnu-x86_64']
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.

No versioned download? :-/

At the very least, let EasyBuild rename the file on download, something like:

sources = [{
    'download_filename': 'godon-master-linux-gnu-x86_64',
    'filename': 'godon-%(version)s-linux-gnu-x86_64',
}]

It's much better to use a download URL or filename that includes a version though, if that's available, since otherwise the download will be broken due to getting a different checksum if there's an update...

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.

the source code is available too (it's written in Go), so we could look at a from-source build too. Looks like it relies on NLOpt and a BLAS implementation

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I tried to compile it several times, but it always failed in my hands, as most Go packages do.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

There is no version for the binary download, that's why I used the last change date

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.

Building from source would indeed be better, but the versionsuffix makes it very clear we're using a pre-built binary here, so no blocker to merge this


extract_sources = False

postinstallcmds = ["mkdir -p %(installdir)s/bin/ && cd %(installdir)s/bin/ && "
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.

Use install_cmd instead?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Right

sources = ['godon-master-linux-gnu-x86_64']
checksums = ['159058f7577093548f3ced1540d1ff9fd5f7915cdfcb8f8cd7fb40f5c202fcca']


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 remove duplicate empty line

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Right


sanity_check_paths = {
'files': ["bin/godon"],
'dirs': ['bin'],
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.

There's no point in checking for a non-empty bin directory if we're already checking for a specific file in there, so:

sanity_check_paths = {
    'files': ["bin/godon"],
    'dirs': [],
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Right

source_urls = ['https://bitbucket.org/Davydov/godon/downloads']
sources = [{
'download_filename': 'godon-master-linux-gnu-x86_64',
'filename': 'godon',
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.

@smoretti You should definitely use %(version)s here, and probably also keep the x86_64 part, and then change install_cmd accordingly below:

sources = [{
    'download_filename': 'godon-master-linux-gnu-x86_64',
    'filename': 'godon-%(version)s-linux-gnu-x86_64',
}]
...
install_cmd = "mkdir -p %(installdir)s/bin/ && cp -a godon-%(version)s-linux-gnu-x86_64 %(installdir)s/bin/ && "
install_cmd += "cd %(installdir)s/bin && ln -s godon-%(version)s-linux-gnu-x86_64 godon"

Also use cp -a rather than mv, so you don't remove the downloaded file while installing

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@boegel You mean also to change the version value "20210913" to "master"?

Copy link
Copy Markdown
Member

@jfgrimm jfgrimm Nov 8, 2022

Choose a reason for hiding this comment

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

@smoretti

sources = [{
    'download_filename': 'godon-master-linux-gnu-x86_64',
    'filename': 'godon-%(version)s-linux-gnu-x86_64',
}]

will download the godon-master-linux-gnu-x86_64 file and save it as godon-%(version)s-linux-gnu-x86_64

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks

@boegel
Copy link
Copy Markdown
Member

boegel commented Nov 9, 2022

@boegelbot please test @ generoso

@boegel boegel changed the title {bio}[system/system] Godon v20210913 {bio}[system/system] Godon v20210913 (x86_64) Nov 9, 2022
@boegelbot
Copy link
Copy Markdown
Collaborator

@boegel: Request for testing this PR well received on login1

PR test command 'EB_PR=16574 EB_ARGS= EB_CONTAINER= /opt/software/slurm/bin/sbatch --job-name test_PR_16574 --ntasks=4 ~/boegelbot/eb_from_pr_upload_generoso.sh' executed!

  • exit code: 0
  • output:
Submitted batch job 9460

Test results coming soon (I hope)...

Details

- notification for comment with ID 1308323523 processed

Message to humans: this is just bookkeeping information for me,
it is of no use to you (unless you think I have a bug, which I don't).

@boegelbot
Copy link
Copy Markdown
Collaborator

Test report by @boegelbot
SUCCESS
Build succeeded for 1 out of 1 (1 easyconfigs in total)
cns2 - Linux Rocky Linux 8.5, x86_64, Intel(R) Xeon(R) CPU E5-2667 v3 @ 3.20GHz (haswell), Python 3.6.8
See https://gist.github.com/4837cbd6fd38400f7091e053bfde2404 for a full test report.

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.

lgtm

@boegel boegel added this to the next release (4.6.3?) milestone Nov 9, 2022
@boegel
Copy link
Copy Markdown
Member

boegel commented Nov 9, 2022

Test report by @boegel
SUCCESS
Build succeeded for 1 out of 1 (1 easyconfigs in total)
node3135.skitty.os - Linux RHEL 8.4, x86_64, Intel(R) Xeon(R) Gold 6140 CPU @ 2.30GHz (skylake_avx512), Python 3.6.8
See https://gist.github.com/eb7950839caa95afba2c568a103f598e for a full test report.

@boegel
Copy link
Copy Markdown
Member

boegel commented Nov 9, 2022

Going in, thanks @SIB-software!

@boegel boegel merged commit ca3d9f9 into easybuilders:develop Nov 9, 2022
@smoretti smoretti deleted the 20221108094459_new_pr_Godon20210913 branch November 9, 2022 08:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

BioHack2022 Related to EU BioHackathon 2022 new

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants