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.

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

Extend checked_find_package with VERSION_MIN and VERSION_MAX #1303

Merged
merged 1 commit into from
Nov 27, 2020

Conversation

lgritz
Copy link
Collaborator

@lgritz lgritz commented Nov 22, 2020

Upon reading cmake docs mre carefully, it's clear that when you pass a
version to find_package (which does much of the heavy lifting inside
checked_find_package), it DOESN'T MEAN you want that version or
later. It means you want to find a version that is "compatible" with
the named version. The meaning of "compatible" is up to the package.
Some packages define it as being >= the requested version. Others
define it as being the same major release, or same major AND minor
release, or exact.

But generally speaking, we only care about the minimum version (e.g.,
"I want FooBar 8.0 or later"). And it's not guaranteed that is what
the package's compatibility will do. In fact, FooBar may be using
compatibility mode "SameMajorVersion", which will make our find of
FooBar succeed for 8.0, 8.1, 8.2, but if a user encounters FooBar 9.0,
our build will reject it, even though it's probably fine.

So I augmented our checked_find_package wrapper to take optional
VERSION_MIN to give the real minimum version, when that's the semantic
we really want (in which case we would omit the ordinary version
number used for the compatibility check and let VERSION_MIN do all the
work). There's also an optional (and usually omitted) VERSION_MAX that
you can use if you are sure that we must be less than a certain
version (the max is exclusive, you must be <, not <=, whereas the min
is inclusive >=).

Signed-off-by: Larry Gritz [email protected]

Upon reading cmake docs mre carefully, it's clear that when you pass a
version to find_package (which does much of the heavy lifting inside
checked_find_package), it DOESN'T MEAN you want that version or
later. It means you want to find a version that is "compatible" with
the named version. The meaning of "compatible" is up to the package.
Some packages define it as being >= the requested version. Others
define it as being the same major release, or same major AND minor
release, or exact.

But generally speaking, we only care about the minimum version (e.g.,
"I want FooBar 8.0 or later"). And it's not guaranteed that is what
the package's compatibility will do. In fact, FooBar may be using
compatibility mode "SameMajorVersion", which will make our find of
FooBar succeed for 8.0, 8.1, 8.2, but if a user encounters FooBar 9.0,
our build will reject it, even though it's probably fine.

So I augmented our checked_find_package wrapper to take optional
VERSION_MIN to give the real minimum version, when that's the semantic
we really want (in which case we would omit the ordinary version
number used for the compatibility check and let VERSION_MIN do all the
work). There's also an optional (and usually omitted) VERSION_MAX that
you can use if you are sure that we must be less than a certain
version (the max is exclusive, you must be <, not <=, whereas the min
is inclusive >=).

Signed-off-by: Larry Gritz <[email protected]>
@lgritz lgritz merged commit 962cc73 into AcademySoftwareFoundation:master Nov 27, 2020
lgritz added a commit to lgritz/OpenShadingLanguage that referenced this pull request Nov 27, 2020
…SoftwareFoundation#1303)

Upon reading cmake docs mre carefully, it's clear that when you pass a
version to find_package (which does much of the heavy lifting inside
checked_find_package), it DOESN'T MEAN you want that version or
later. It means you want to find a version that is "compatible" with
the named version. The meaning of "compatible" is up to the package.
Some packages define it as being >= the requested version. Others
define it as being the same major release, or same major AND minor
release, or exact.

But generally speaking, we only care about the minimum version (e.g.,
"I want FooBar 8.0 or later"). And it's not guaranteed that is what
the package's compatibility will do. In fact, FooBar may be using
compatibility mode "SameMajorVersion", which will make our find of
FooBar succeed for 8.0, 8.1, 8.2, but if a user encounters FooBar 9.0,
our build will reject it, even though it's probably fine.

So I augmented our checked_find_package wrapper to take optional
VERSION_MIN to give the real minimum version, when that's the semantic
we really want (in which case we would omit the ordinary version
number used for the compatibility check and let VERSION_MIN do all the
work). There's also an optional (and usually omitted) VERSION_MAX that
you can use if you are sure that we must be less than a certain
version (the max is exclusive, you must be <, not <=, whereas the min
is inclusive >=).

Signed-off-by: Larry Gritz <[email protected]>
@lgritz lgritz deleted the lg-depwarn branch November 27, 2020 20:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant