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

Reflect semantic version in find_package version checks #1293

Closed
alliepiper opened this issue Sep 25, 2020 · 1 comment · Fixed by #1298
Closed

Reflect semantic version in find_package version checks #1293

alliepiper opened this issue Sep 25, 2020 · 1 comment · Fixed by #1298
Assignees
Labels
only: cmake CMake changes only. Doesn't need internal NVIDIA CI.
Milestone

Comments

@alliepiper
Copy link
Collaborator

Currently, when a downstream consumer calls find_package(Thrust ...) and passes an explicit version, we handle version compatibility as follows:

  1. If EXACT version matching is requested, all four version components (X.Y.Z.W) must match.
  2. Otherwise, only the first three components (X.Y.Z) must match.

Our contributor guide specifies that Thrust will follow semantic versioning, and as such the above scheme is too restrictive. According to our stated rules for version increases, I propose moving to the following scheme:

  1. If EXACT version matching is requested, X.Y.Z.W must match.
  2. Otherwise, only X must match, and the package's X.Y.Z.W must be >= the requested X.Y.Z.W.

Thanks to @germasch to pointing this out here.

Refs:

@alliepiper alliepiper added the only: cmake CMake changes only. Doesn't need internal NVIDIA CI. label Sep 25, 2020
@alliepiper alliepiper added this to the 1.11.0 milestone Sep 25, 2020
@germasch
Copy link
Contributor

Thanks for taking up my comment. Just FWIW, as long as you're expecting that the cmake config stuff is still settling down, I think it's reasonable to enforce the stricter compatibility requirements.

The nice thing is that you can make the decision when 1.11.0 (or 1.10.1) is released -- if the config is compatible you can allow find_package(Thrust 1.10.0) to accept 1.11.0 by means of the config you distribute with the latter.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
only: cmake CMake changes only. Doesn't need internal NVIDIA CI.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants