-
Notifications
You must be signed in to change notification settings - Fork 663
exrcheck: update CMakeLists.txt to install the tool #1983
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
exrcheck: update CMakeLists.txt to install the tool #1983
Conversation
|
@peterhillman, I know you originally developed exrcheck as an internal tool, but if it's generally useful for debugging/diagnostics, any reason not to officially distribute it? |
|
Yes, we should update to allow |
|
Thanks for the info, @cary-ilm and @peterhillman! I see what you mean about I don't know whether there are any other tools that fall into this category, but along the lines of @peterhillman's suggestion, would a new option like EDIT: removed the suggestion of an additional |
|
Yes, although it's verging on overkill to introduce a new option I think it does make sense in this case. I think the CI would also have to be updated to set that option so we can verify it gets installed. I would expect package managers would want to make the developer tools a separate package to regular tools so it is not normally installed, but still discoverable via a package search or a shell's command_not_found_handle. This change would simplify setting up that package. |
1ad645f to
7bd55b2
Compare
|
Thanks @peterhillman! I took a crack at adding an I also made an attempt at documenting the new option in I did not turn the new option on for any CI tasks yet (I think those are configured separately, outside of the repo?), so there aren't currently any install manifests that include |
7bd55b2 to
058c64b
Compare
peterhillman
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good to me, though I'm not a cmake expert
| and as a result it is not recommended that they be used with untrusted | ||
| input files. A separate ``OPENEXR_INSTALL_DEVELOPER_TOOLS=ON`` option | ||
| should be enabled if installation of such tools is desired. The option | ||
| is off by default. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is it worth mentioning that exrcheck is compiled even if the option is OFF, and can be run directly from the build/bin folder without installing it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good idea! I tried to incorporate that into the verbiage with commit 1adf14d.
Thanks @peterhillman!
This new option will control whether or not the tools considered to be "developer" tools should be installed. These are tools useful for developing and debugging OpenEXR itself that might not be suitable for distribution to end users. exrcheck is currently the only tool considered to be a developer tool. Signed-off-by: Matt Johnson <matt.johnson@epicgames.com>
This makes the CMakeLists.txt for exrcheck look like other tools like exrinfo and uncomments the CMake call to install the tool. One difference though is that exrcheck is considered a developer tool, and as such is only installed when the OPENEXR_INSTALL_DEVELOPER_TOOLS option is enabled. Signed-off-by: Matt Johnson <matt.johnson@epicgames.com>
058c64b to
75558fb
Compare
|
Thanks! |
…oundation#1983) * add an OPENEXR_INSTALL_DEVELOPER_TOOLS option This new option will control whether or not the tools considered to be "developer" tools should be installed. These are tools useful for developing and debugging OpenEXR itself that might not be suitable for distribution to end users. exrcheck is currently the only tool considered to be a developer tool. Signed-off-by: Matt Johnson <matt.johnson@epicgames.com> * exrcheck: update CMakeLists.txt to enable developer tool installation This makes the CMakeLists.txt for exrcheck look like other tools like exrinfo and uncomments the CMake call to install the tool. One difference though is that exrcheck is considered a developer tool, and as such is only installed when the OPENEXR_INSTALL_DEVELOPER_TOOLS option is enabled. Signed-off-by: Matt Johnson <matt.johnson@epicgames.com> --------- Signed-off-by: Matt Johnson <matt.johnson@epicgames.com>
…oundation#1983) * add an OPENEXR_INSTALL_DEVELOPER_TOOLS option This new option will control whether or not the tools considered to be "developer" tools should be installed. These are tools useful for developing and debugging OpenEXR itself that might not be suitable for distribution to end users. exrcheck is currently the only tool considered to be a developer tool. Signed-off-by: Matt Johnson <matt.johnson@epicgames.com> * exrcheck: update CMakeLists.txt to enable developer tool installation This makes the CMakeLists.txt for exrcheck look like other tools like exrinfo and uncomments the CMake call to install the tool. One difference though is that exrcheck is considered a developer tool, and as such is only installed when the OPENEXR_INSTALL_DEVELOPER_TOOLS option is enabled. Signed-off-by: Matt Johnson <matt.johnson@epicgames.com> --------- Signed-off-by: Matt Johnson <matt.johnson@epicgames.com>
* add an OPENEXR_INSTALL_DEVELOPER_TOOLS option This new option will control whether or not the tools considered to be "developer" tools should be installed. These are tools useful for developing and debugging OpenEXR itself that might not be suitable for distribution to end users. exrcheck is currently the only tool considered to be a developer tool. Signed-off-by: Matt Johnson <matt.johnson@epicgames.com> * exrcheck: update CMakeLists.txt to enable developer tool installation This makes the CMakeLists.txt for exrcheck look like other tools like exrinfo and uncomments the CMake call to install the tool. One difference though is that exrcheck is considered a developer tool, and as such is only installed when the OPENEXR_INSTALL_DEVELOPER_TOOLS option is enabled. Signed-off-by: Matt Johnson <matt.johnson@epicgames.com> --------- Signed-off-by: Matt Johnson <matt.johnson@epicgames.com>
…oundation#1983) * add an OPENEXR_INSTALL_DEVELOPER_TOOLS option This new option will control whether or not the tools considered to be "developer" tools should be installed. These are tools useful for developing and debugging OpenEXR itself that might not be suitable for distribution to end users. exrcheck is currently the only tool considered to be a developer tool. Signed-off-by: Matt Johnson <matt.johnson@epicgames.com> * exrcheck: update CMakeLists.txt to enable developer tool installation This makes the CMakeLists.txt for exrcheck look like other tools like exrinfo and uncomments the CMake call to install the tool. One difference though is that exrcheck is considered a developer tool, and as such is only installed when the OPENEXR_INSTALL_DEVELOPER_TOOLS option is enabled. Signed-off-by: Matt Johnson <matt.johnson@epicgames.com> --------- Signed-off-by: Matt Johnson <matt.johnson@epicgames.com>
While trying to diagnose a potential regression, I went looking for a validation command-line tool and came across
exrcheck, but noticed that it does not currently get installed along with the other command-line tools (the CMakeinstall()call is commented out). Spelunking through the ASWF Slack channel, it sounds like this was sort of intentional in that the tool is also oriented towards testing OpenEXR itself and not strictly a file validation tool. The question of whetherexrcheckgets installed seems to have come up enough that it seems worth doing. See for example:https://academysoftwarefdn.slack.com/archives/CMLRW4N73/p1707504987153709
https://academysoftwarefdn.slack.com/archives/CMLRW4N73/p1724241705200189
The change here is mostly just conforming
exrcheck'sCMakeLists.txtto look like other command-line tools, in particularexrinfo's.As far as the regression, once I had
exrcheckinstalled I was able to use it to find that a test file was deemedOKthrough the file API, butbadthrough the stream API (with the-soption), so I'll be digging into that further and possibly opening another issue or PR if I can find anything.