Skip to content

Conversation

@cary-ilm
Copy link
Member

OpenJPH headers are included in application code with the openjph subdirectory:

#include <openjph/ojph_version.h>

However, in the OpenJPH source tree, the headers live under a directory called "common", which means when OpenEXR builds the vendored copy of OpenJPH via add_subdirectory, the headers aren't in the expected "openjph" directory.

The previous solution to this was to create a symlink from "common" to "openjph", but symlinks are fragile on Windows. If the git repo is cloned with core.symlinks=false, the symlink doesn't exist.

This solution simply renames the "common" directory to "openjph" in the vendored OpenJPH code, adjusting references to it as necessary. The vendor_openjph.sh script does the proper renaming in the OpenJPH cmake configuration.

An alternate solution would be to conditionally include <common/ojph_version.h> when building against the vendored code, but that's message. At least this contains the weirdness inside the vendored code.

For a longer term solution, I'll submit a request to change this upstream in OpenJPH.

This address the issue raised on #2193 after it was merged.

OpenJPH headers are included in application code with the openjph subdirectory:

However, in the OpenJPH source tree, the headers live under a
directory called "common", which means when OpenEXR builds the
vendored copy of OpenJPH via `add_subdirectory`, the headers aren't in
the expected "openjph" directory.

The previous solution to this was to create a symlink from "openjph"
to "common", but symlinks are fragile on Windows. If the git repo is
cloned with `core.symlinks=false`, the symlink doesn't exist.

This solution simply renames the "common" directory to "openjph" in
the vendored OpenJPH code, adjusting references to it as necessary.
The `vendor_openjph.sh` script does the proper renaming in the OpenJPH
cmake configuration.

This address the issue raised on AcademySoftwareFoundation#2193 after it was merged.

Signed-off-by: Cary Phillips <[email protected]>
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