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

OpenEXRConfig.h.in uses version extracted from openexr_version.h #1527

Merged
merged 5 commits into from
Aug 28, 2023

Conversation

cary-ilm
Copy link
Member

The logic was already in place to extract the version from openexr_version.h in the root-level CMakeLists.txt, so the OpenEXRConfig.h.in file just needs to reference the proper CMake variables for configuration.

This is still a bit awkward, since it would be better to simply #include "openexr_version.h" from OpenEXRConfig.h, but the cmake configuration doesn't readily allow that.

The alternative of simply stripping all version #define's from OpenEXRConfig.h.in and the requiring an #include "openexr_version.h" for access to the version is a cleaner option, but might break existing application code, so consider the settings in OpenEXRConfig.h.in as backwards compatibility.

This also duplicates the OpenEXR_VERSION_* CMake settings to OPENEXR_VERSION_*.

The logic was already in place to extract the version from
`openexr_version.h`, so the `.in` file just needed to reference the
proper CMake variables. This also duplicates the `OpenEXR_VERSION_*`
settings to `OPENEXR_VERSION_*`.

Signed-off-by: Cary Phillips <cary@ilm.com>
Signed-off-by: Cary Phillips <cary@ilm.com>
Signed-off-by: Cary Phillips <cary@ilm.com>
@cary-ilm cary-ilm requested a review from meshula August 28, 2023 05:14
@cary-ilm
Copy link
Member Author

@Vertexwahn, is there a straightforward way to propagate the version info from openexr_version.h into BUILD.bazel, without having to explicitly duplicate the settings?

Signed-off-by: Cary Phillips <cary@ilm.com>
@cary-ilm
Copy link
Member Author

Also added a fix for messy formatting of the version test error message in OpenEXRCoreTest, it was including unnecessary quotes around the "extra" part.

Copy link
Contributor

@meshula meshula left a comment

Choose a reason for hiding this comment

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

Thank you for the clean up, and completing the version work. This all looks much better than before.

Signed-off-by: Cary Phillips <cary@ilm.com>
@cary-ilm cary-ilm merged commit df91c16 into AcademySoftwareFoundation:main Aug 28, 2023
29 checks passed
cary-ilm added a commit that referenced this pull request Aug 28, 2023
* OpenEXRConfig.h.in uses version extracted from openexr_version.h

The logic was already in place to extract the version from
`openexr_version.h`, so the `.in` file just needed to reference the
proper CMake variables. This also duplicates the `OpenEXR_VERSION_*`
settings to `OPENEXR_VERSION_*`.

Signed-off-by: Cary Phillips <cary@ilm.com>

* bazel

Signed-off-by: Cary Phillips <cary@ilm.com>

* comment

Signed-off-by: Cary Phillips <cary@ilm.com>

* fix formatting of core version test error message

Signed-off-by: Cary Phillips <cary@ilm.com>

* fix extra comparison test

Signed-off-by: Cary Phillips <cary@ilm.com>

---------

Signed-off-by: Cary Phillips <cary@ilm.com>
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.

None yet

2 participants