Skip to content

Remove redundant $CPATH entry of tbb modules + fix --module-only#2511

Merged
ocaisa merged 4 commits intoeasybuilders:developfrom
Flamefire:20210708112951_new_pr_gOZUQuoMzr
Jul 21, 2021
Merged

Remove redundant $CPATH entry of tbb modules + fix --module-only#2511
ocaisa merged 4 commits intoeasybuilders:developfrom
Flamefire:20210708112951_new_pr_gOZUQuoMzr

Conversation

@Flamefire
Copy link
Copy Markdown
Contributor

@Flamefire Flamefire commented Jul 8, 2021

(created using eb --new-pr)

migueldiascosta
migueldiascosta previously approved these changes Jul 13, 2021
View reviewed changes
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 next release (4.4.2?) milestone Jul 13, 2021
@migueldiascosta
Copy link
Copy Markdown
Member

migueldiascosta commented Jul 13, 2021

hm, --module-only fails because self.libpath is UNKNOWN (only set in install_step)...

@Flamefire, would you care to also fix that?

@Flamefire Flamefire force-pushed the 20210708112951_new_pr_gOZUQuoMzr branch from 0d777cc to d2549be Compare July 13, 2021 08:10
@Flamefire
Copy link
Copy Markdown
Contributor Author

Flamefire commented Jul 13, 2021

@Flamefire, would you care to also fix that?

Done a first approach. Testing a few ECs now because something is really fishy here. IMO we should always have lib/tbb*.so and include/tbb/* and maybe some bin. The comments don't match what is done (which is why I dislike such comments) and there are major differences between the SYSTEM version and the source version. Trying to get them closer together.

Checked e.g. endree and that searches /lib not /tbb/lib, and we even set TBB_ROOT=/tbb if that exists else to / only (always with the install-prefix ofc)
So from how it looks to me the tbb subdir is bogus

@Flamefire Flamefire force-pushed the 20210708112951_new_pr_gOZUQuoMzr branch from 5b1e03e to a540f62 Compare July 13, 2021 14:27
@Flamefire
Copy link
Copy Markdown
Contributor Author

Flamefire commented Jul 13, 2021

Test report by @Flamefire

Overview of tested easyconfigs (in order)

  • SUCCESS tbb-2018.4.222.eb
  • SUCCESS tbb-2019_U4-GCCcore-8.2.0.eb
  • SUCCESS tbb-2020.3-GCCcore-10.2.0.eb

Build succeeded for 3 out of 3 (3 easyconfigs in total)
taurusa5 - Linux centos linux 7.7.1908, x86_64, Intel(R) Xeon(R) CPU E5-2603 v4 @ 1.70GHz (broadwell), Python 2.7.5
See https://gist.github.com/d05030739a864e3dfc6eb6b8c282b91c for a full test report.

@Flamefire
Copy link
Copy Markdown
Contributor Author

Flamefire commented Jul 13, 2021

@migueldiascosta Ok done. Wasn't exactly easy to do it correctly but it should now be as it was supposed to be

@ocaisa
Copy link
Copy Markdown
Member

ocaisa commented Jul 21, 2021

Test report by @ocaisa

Overview of tested easyconfigs (in order)

  • SUCCESS tbb-2019_U4-GCCcore-8.2.0.eb
  • SUCCESS tbb-2020.3-GCCcore-10.2.0.eb

Build succeeded for 2 out of 2 (2 easyconfigs in total)
generoso - Linux centos linux 8.2.2004, x86_64, Intel(R) Xeon(R) CPU E5-2667 v3 @ 3.20GHz, Python 3.6.8
See https://gist.github.com/c7637ff02a541481322e068d72a01bc4 for a full test report.

@ocaisa
Copy link
Copy Markdown
Member

ocaisa commented Jul 21, 2021

Test report by @ocaisa

Overview of tested easyconfigs (in order)

  • SUCCESS tbb-2019_U4-GCCcore-8.2.0.eb
  • SUCCESS tbb-2020.3-GCCcore-10.2.0.eb

Build succeeded for 2 out of 2 (2 easyconfigs in total)
generoso - Linux centos linux 8.2.2004, x86_64, Intel(R) Xeon(R) CPU E5-2667 v3 @ 3.20GHz, Python 3.6.8
See https://gist.github.com/bcc5c9077c2f19b717b092d5adde6110 for a full test report.

ocaisa
ocaisa approved these changes Jul 21, 2021
View reviewed changes
Copy link
Copy Markdown
Member

@ocaisa ocaisa left a comment •
edited
Loading

Choose a reason for hiding this comment

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

LGTM - also checked with --module-only

@ocaisa ocaisa merged commit e0e03b7 into easybuilders:develop Jul 21, 2021
@Flamefire Flamefire deleted the 20210708112951_new_pr_gOZUQuoMzr branch July 21, 2021 13:49
@boegel boegel changed the title Remove redundant CPATH entry of tbb modules Remove redundant $CPATH entry of tbb modules + fix --module-only Aug 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ocaisa ocaisa ocaisa approved these changes

@migueldiascosta migueldiascosta migueldiascosta left review comments

Assignees

No one assigned

Labels

bug fix

Projects

None yet

Milestone

4.4.2

Development

Successfully merging this pull request may close these issues.

3 participants

@Flamefire @migueldiascosta @ocaisa