Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fixes missing pthread link and replaces with Threads::Threads #2888

Merged
merged 3 commits into from Feb 17, 2022
Merged

Fixes missing pthread link and replaces with Threads::Threads #2888

merged 3 commits into from Feb 17, 2022

Conversation

jp-dark
Copy link
Contributor

@jp-dark jp-dark commented Feb 16, 2022

  • Replaces hard-coded link to pthread with link to Threads::Threads library using built-in CMake module FindThreads.
  • Adds missing link to Threads::Threads for unit_dynamic_memory.

TYPE: BUG
DESC: Adds missing pthreads link to dynamic memory unit test

@jp-dark
Fixes missing pthread link and replaces with Threads::Threads
6599313 
* Replaces hard-coded link to pthread with link to Threads::Threads
library using built-in CMake module FindThreads.

* Adds missing link to Threads::Threads for `unit_dynamic_memory`.
@shortcut-integration
Copy link

This pull request has been linked to Shortcut Story #14293: Fix pthread dependency for unit_dynamic_memory test.

eric-hughes-tiledb
eric-hughes-tiledb approved these changes Feb 16, 2022
View reviewed changes
Copy link
Contributor

@eric-hughes-tiledb eric-hughes-tiledb left a comment

Choose a reason for hiding this comment

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

We only really need the documentation in one place, but whatever.

LGTM

ihnorton
ihnorton reviewed Feb 16, 2022
View reviewed changes
Copy link
Member

@ihnorton ihnorton left a comment

Choose a reason for hiding this comment

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

From prior issues, my only concern here is to make sure that the static build still works correctly for downstream release products, so we'll do a tiledb-py wheel build test against this branch (cc @nguyenv).

tiledb/common/dynamic_memory/CMakeLists.txt Outdated Show resolved Hide resolved
@nguyenv
Copy link
Contributor

nguyenv commented Feb 17, 2022

From prior issues, my only concern here is to make sure that the static build still works correctly for downstream release products, so we'll do a tiledb-py wheel build test against this branch (cc @nguyenv).

I've manually tested the wheels on both WIndows 11 and WSL Ubuntu 20.04, and they are working.

@Shelnutt2 Shelnutt2 merged commit 641bada into dev Feb 17, 2022
@Shelnutt2 Shelnutt2 deleted the jpd/sc-14293/cmake-pthreads branch February 17, 2022 15:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Shelnutt2 Shelnutt2 Shelnutt2 approved these changes

@ihnorton ihnorton ihnorton approved these changes

@nguyenv nguyenv nguyenv approved these changes

@eric-hughes-tiledb eric-hughes-tiledb eric-hughes-tiledb approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@jp-dark @nguyenv @Shelnutt2 @ihnorton @eric-hughes-tiledb