search

AcademySoftwareFoundation / openexr

The OpenEXR project provides the specification and reference implementation of the EXR file format, the professional-grade image storage format of the motion picture industry.
http://www.openexr.com/
BSD 3-Clause "New" or "Revised" License
1.6k stars 606 forks source link

[OpenEXRCore] angle bracket includes within the OpenEXR library should have an OpenEXR/ prefix #1095

Open anderslanglands opened 3 years ago

anderslanglands commented 3 years ago

https://github.com/kdt3rd/openexr/blob/017f056239e817cbe2ab601efe99dc9f4c9e7f52/src/lib/OpenEXRCore/openexr_conf.h#L10

Is there a reason to use angled brackets for including headers rather than quotes? This appears to break clang. I can work around it by passing both -I${OPENEXR_ROOT}/include and -I${OPENEXR_ROOT}/include/OpenEXR but this is a bit hacky.

meshula commented 3 years ago

That's typical for ASWF libraries and ASWF adjacent libraries. Like OIIO for example, uses angle brackets. If you're noticing an issue with OpenEXR in particular, is it because other libraries like OIIO are installed site wide, and you've got a -I for your site headers, but OpenEXR is not in the site wide location?

lgritz commented 3 years ago

But it's not supposed to be "bare".

#include <OpenEXR/OpenEXRConfig.h>

is fine. But

#include <OpenEXRConfig.h>

is not going to find it in the include directory.

anderslanglands commented 3 years ago

Yep that is indeed the issue.

On Sat, 17 Jul 2021 at 17:45, Larry Gritz @.***> wrote:

But it's not supposed to be "bare".

include <OpenEXR/OpenEXRConfig.h>

is fine. But

include

is not going to find it in the include directory.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/AcademySoftwareFoundation/openexr/issues/1095#issuecomment-881835117, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAOYQXIOVH6ILREYHJ67CVLTYEKHBANCNFSM5AQMYUOQ .

meshula commented 3 years ago

Ah, I'll update the issue title.

cary-ilm commented 2 years ago

I'm not entirely sure the correct solution here. #1097 changes the <> to "" inside of all exported headers, for the few files that were using <>. #include <OpenEXR/OpenEXRConfig.h> does not work; it might work in application code, but it doesn't work in building OpenEXR itself.

OpenEXR.pc includes both -I -I<include/OpenEXR>, which I think we did for backwards compatibility.

What about includes of Imath from inside OpenEXR headers? We're inconsistent, with some included via "" and others with <>. Should they all be ""?

anderslanglands commented 2 years ago

I think long-term, Imath headers should be <Imath/ImathBox.h>, but you’re still supporting everything being installed under include/OpenEXR for now right? In which case using “ImathBox.h” and adding both -Iimath/include and -Iimath/include/Imath works.

OIIO is able to use the <OpenImageIO/imageio.h> form because it splits source and public headers in the build, whereas everything’s together in OpenEXR, which means “OpenEXRConfig.h” rather than <OpenEXR/OpenEXRConfig.h> is the only way to make both build and dependencies work with a single include path without restructuring the source tree.

Probably the cleanest solution would be to wait until you're ready to break compatibility with the old "everything under include/OpenEXR" method of installation and then revisit, choosing either modifying all public includes to quotes, or restructuring the soruce tree such that <OpenEXR/OpenEXRConfig.h> would work.

In the meantime, this is only really an issue for projects like ours like are not using CMake for getting include paths and have to remember to specify both include paths (and I'm probably going to modify our stuff to use CMake for grabbing includes anyway at some point before too long, we already do it for link args).

On Mon, 19 Jul 2021 at 07:03, Cary Phillips @.***> wrote:

I'm not entirely sure the correct solution here. #1097 https://github.com/AcademySoftwareFoundation/openexr/pull/1097 changes the <> to "" inside of all exported headers, for the few files that were using <>. #include <OpenEXR/OpenEXRConfig.h> does not work; it might work in application code, but it doesn't work in building OpenEXR itself.

OpenEXR.pc includes both -I -I<include/OpenEXR>, which I think we did for backwards compatibility.

What about includes of Imath from inside OpenEXR headers? We're inconsistent, with some included via "" and others with <>. Should they all be ""?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/AcademySoftwareFoundation/openexr/issues/1095#issuecomment-882102999, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAOYQXIBYIUOB25S7R5FORLTYMQR5ANCNFSM5AQMYUOQ .