Skip to content

{vis}[foss/2019b] fix XCrySDen v1.6.2 linking to libdl and Togl#14160

Merged
lexming merged 3 commits intoeasybuilders:developfrom
schiotz:20211015135834_new_pr_XCrySDen162
Oct 15, 2021
Merged

{vis}[foss/2019b] fix XCrySDen v1.6.2 linking to libdl and Togl#14160
lexming merged 3 commits intoeasybuilders:developfrom
schiotz:20211015135834_new_pr_XCrySDen162

Conversation

@schiotz
Copy link
Copy Markdown
Contributor

@schiotz schiotz commented Oct 15, 2021

(created using eb --new-pr)

lexming
lexming approved these changes Oct 15, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@lexming lexming left a comment

Choose a reason for hiding this comment

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

@schiotz Thanks for the bug fixes. I confirm both issues on my end

  • Togl installs shared objects in a subfolder of lib that is not added to the environment
  • XCrySDen fails to build on foss due to missing linker flags to dl

Since XCrySDen builds just fine on intel, the solution of setting LDLIB in the easyconfig for foss is appropriate. I just made a minor change removing the export, as it is not needed to pass the variable into make.

LGTM

@lexming lexming added this to the next release (4.5.0?) milestone Oct 15, 2021
@lexming lexming changed the title Bug fix: xcrysden should link correctly to libdl and Togl. {vis}[foss/2019b] fix XCrySDen v1.6.2 linking to libdl and Togl Oct 15, 2021
@lexming
Copy link
Copy Markdown
Contributor

lexming commented Oct 15, 2021

@boegelbot: please test @ generoso

@boegelbot
Copy link
Copy Markdown
Collaborator

boegelbot commented Oct 15, 2021

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

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

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

Test results coming soon (I hope)...

Details

- notification for comment with ID 944811224 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

boegelbot commented Oct 15, 2021

Test report by @boegelbot
SUCCESS
Build succeeded for 3 out of 3 (2 easyconfigs in total)
cnx1 - Linux rocky linux 8.4, x86_64, Intel(R) Xeon(R) CPU E5-2690 v3 @ 2.60GHz (haswell), Python 3.6.8
See https://gist.github.com/3a5d83cfd3496f736f33c79556536026 for a full test report.

@lexming
Copy link
Copy Markdown
Contributor

lexming commented Oct 15, 2021

Test report by @lexming
SUCCESS
Build succeeded for 2 out of 2 (2 easyconfigs in total)
node300.hydra.os - Linux centos linux 7.9.2009, x86_64, Intel(R) Xeon(R) Gold 6148 CPU @ 2.40GHz, Python 2.7.5
See https://gist.github.com/ffa70ab9d5af14521a2720be78dbe838 for a full test report.

@lexming
Copy link
Copy Markdown
Contributor

lexming commented Oct 15, 2021

Going in, thanks @schiotz !

@lexming lexming enabled auto-merge October 15, 2021 23:49
@lexming lexming merged commit 444e578 into easybuilders:develop Oct 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@lexming lexming lexming approved these changes

Assignees

No one assigned

Labels

bug fix

Projects

None yet

Milestone

4.5.0

Development

Successfully merging this pull request may close these issues.

3 participants

@schiotz @lexming @boegelbot