ociocheck to check OpenColorIO library version first

[meimchu]

I brought this up as a question recently in ASWF OpenColorIO's slack channel. If I were to use ociocheck built from an earlier version of OCIO (v2.2 for example) and try to load in a OCIO config file built with a later version of OCIO (v2.4 for example), the operation will stop immediately with a generic message ERROR: Error could not read.... I thought it would be nice to have a more deterministic error message by checking for ocio_profile_version: value first. This is especially so when the various DCCs can have different OCIO versions loaded. This would be just a nice little quality of life improvement for debugging.

ociocheck already prints out OpenColorIO Library Version: at the beginning so my plan is to just expand on that by checking it against ocio_profile_version prior to OCIO::CreateFromFile().

[meimchu]

Turns out I was mistaken at first. ociocheck does call Config::CreateFromFile() which checks the config version against the library version. I believe my mistake was that I passed in config path that did not exist and got a generic error message of Error could not read .... So I would like to amend and propose that I do a file path existence check instead after the builtin config portion of Config::CreateFromFile().

On that front, I could start utilizing C++17's filesystem that was mentioned in #2256 to help with this and take a pass at Config.cpp to remove pystring usages. Let me know if that is desirable.

[doug-walker]

Hi Mei, I'm not following what you are proposing. If I use CreateFromFile to try to open a config that doesn't exist it gives the error: "Error could not read 'foo.ocio' OCIO profile." If I use it to open a config with a new version than the library, it gives: "... is version 2.6. This version of the OpenColorIO library (2.5.0) is not able to load that config version.
The minor version 6 is not supported for major version 2. Maximum minor version is 5."

Doesn't that seem like the correct behavior?

[meimchu]

Hey Doug, you are correct on that! I realized that the error message "Error could not read 'foo.ocio' OCIO profile." was indicative of the file not existing. I was just hoping to make the message more clear as I didn't realize that during my usage. So I was proposing to do a file existence check using filesystem first (or just modify that existing error message after ifstream.fail()).

I also realized that there was an issue about replacing some pystring::os::path usages with filesystem. So I was wondering whether or not there's any decision made on that front. If that is desired, I can take this opportunity to update some of that usages here in Config.cpp.

[doug-walker]

Sure Mei, I agree that the wording of the message is confusing since "read" could either mean a parsing problem or the file doesn't exist. The messages should be clear that those are different errors.

Please work with Kevin regarding the Pystring issue.