Skip to content
This repository has been archived by the owner on Mar 21, 2024. It is now read-only.

Update FindTBB.cmake to work with latest MSVC. #1281

Merged
merged 1 commit into from Sep 21, 2020
Merged

Update FindTBB.cmake to work with latest MSVC. #1281

merged 1 commit into from Sep 21, 2020

Conversation

alliepiper
Copy link
Collaborator

No description provided.

@alliepiper alliepiper added the only: cmake CMake changes only. Doesn't need internal NVIDIA CI. label Sep 18, 2020
@alliepiper alliepiper added this to the 1.11.0 milestone Sep 18, 2020
@alliepiper alliepiper added this to Inbox in PR Tracking via automation Sep 18, 2020
@alliepiper alliepiper moved this from Inbox to Review [PRs] in PR Tracking Sep 18, 2020
brycelelbach
brycelelbach approved these changes Sep 18, 2020
View reviewed changes
Copy link
Collaborator

@brycelelbach brycelelbach left a comment

Choose a reason for hiding this comment

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

Why not remove the upper bound?

@alliepiper
Copy link
Collaborator Author

If MSVC_VERSION isn't recognized, it's better to have a clear config-time diagnostic instead of a random link error from using the wrong binaries.

The current failure path doesn't prevent anything from working, there are just some additional TBB_ CMake variables that need to be set manually when MSVC_VERSION isn't recognized.

On a side note, it looks like TBB is getting a proper CMake config soon: oneapi-src/oneTBB#6 Hopefully we'll be able to eventually switch to that and get rid of FindTBB.cmake for good.

@alliepiper alliepiper moved this from Review [PRs] to Testing [PRs] in PR Tracking Sep 19, 2020
@alliepiper alliepiper moved this from Testing [PRs] to Integration [PRs] in PR Tracking Sep 19, 2020
@alliepiper alliepiper merged commit 7fe07f4 into NVIDIA:main Sep 21, 2020
PR Tracking automation moved this from Integration [PRs] to Done Sep 21, 2020
@alliepiper alliepiper deleted the bug/fix_FindTBB_for_new_msvc branch September 21, 2020 20:11
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@brycelelbach brycelelbach brycelelbach approved these changes

Assignees
No one assigned
Labels
only: cmake CMake changes only. Doesn't need internal NVIDIA CI.
Projects
PR Tracking
Done
Milestone
1.11.0
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@alliepiper @brycelelbach