Export additional include path

[Rookfighter]

This PR adds the ability to also include the headers of the base imgui library with a imgui/ prefix, i.e. the following is possible after this PR:

#include <imgui/imgui.h>

Basically it is now possible to include the imgui headers with and without the prefix.

I encountered this issue when I ported the box2d testbed here for build2, which includes imgui using this prefix. Also note that box2d includes the backends again without prefix, which seems a bit inconsistent.

Open question: should the backends also support being included with a imgui/backends/ prefix? This would involve moving the sym linked source files in all backend packages into a imgui/backends/ directory. I decided against this since I have not encountered the use-case yet. Any opinions?

[Klaim]

Any opinions?

I'd wait for the case to present itself.

Otherwise LGTM, I suspected that we would hit this issue.

[Swat-SomeBug]

Any opinions?

In general I prefer to have the hierarchy even for backends. Accessing backend headers like:

#include <imgui/backends/imgui_impl_glfw.h>

is more organized than the alternative. Just wish that upstream already did this. Upstream indirectly prefers the headers to be directly dropped in the working directory as there is no support for a specific build tool. I believe most projects would follow this convention, effectively requiring a non-hierarchical include of headers.

At the moment, we could wait to see if there is a use case of for hierarchical backend includes as @Klaim suggests.

[Klaim]

BTW would it work if there was a config option to enable no-prefix-dir includes and enable it only in projects that turn it on? I think it might help contain the potential issues with this, but I'm not sure it would work well with current way build2 projects work.

[Rookfighter]

BTW would it work if there was a config option to enable no-prefix-dir includes and enable it only in projects that turn it on? I think it might help contain the potential issues with this, but I'm not sure it would work well with current way build2 projects work.

Possible, but there is still the problem that upstream does not provide any real convention for this. E.g. the box2d testbed actually uses a mixed convention as it includes the base library with prefix but backends without. On the other hand the Debian package installs the headers in paths with prefixes for the base library and backends. So there is no real either-or scheme possible.

[Klaim]

Yes I meant always enabling prefix and an option to enable no-prefix in addition to with prefix. Not exclusif.

[boris-kolpackov]

I would suggest against making this configurable, it would just complicate things without any benefit, IMO. But if wanting to include backends with a prefix seems like a plausible thing to (and I would personally want to do it consistently between the library and backends, if I were to use imgui), I would suggest we do this now rather than later since doing this later we will also need to worry about not breaking backwards compatibility.

[Swat-SomeBug]

I'm wondering if it's really is an issue supporting both by default?

It would mean an additional -I switch when compiling consumers. I understand this is generally not good and can include some unintended files. But the package itself is self-contained and the additional -I switch should always point to directories directly under this package's control. In this specific case, having additional include paths don't seem like a problem.

Reason:

• Support both use cases
• Upstream examples will mostly fail if hierarchy is used -> Would require patching upstream
• Allow us to follow Debian packaging by supporting hierarchical includes.

[Klaim]

It's either supporting both by default or an option to enable no prefix (as it's the less safe), because projects using imgui are using both anyway, so it's inescapable.

[Rookfighter]

So the point is that we cannot provide imgui only with prefixed headers because the backend headers include the base imgui header already without prefix (e.g. see here)

#include "imgui.h"

I personally would prefer that our imgui packages just provide both include paths (prefixed and unprefixed) by default, where prefix is imgui/ for the base library and imgui/backends/ for the backends. IMO, having two include paths for the same files is far from good architecture, but I guess reality beats theory here and it shouldn't be a problem within the imgui packages. If you guys agree, I will update the PR accordingly during the next days.

[boris-kolpackov]

I personally would prefer that our imgui packages just provide both include paths (prefixed and unprefixed) by default, where prefix is imgui/ for the base library and imgui/backends/ for the backends.

I agree, we basically have no choice but to support the "loose" inclusion style (because that's what upstream uses) and also providing support for the "tight" inclusion style does not pose any problems.

[Rookfighter]

@boris-kolpackov I currently get the following error when building with clang on Windows. Basically clang tries to resolve a include #include <vulkan/vulkan.h> in the directory imgui/backends/, where the header obviously does not exist. I can replicate the behaviour on my local Windows setup. Note that the Windows build with MSVC works (GitHub Actions and my local machine). Also note that the MacOS and Ubuntu (gcc and clang) work.

I am pretty sure that this error is caused by the introduction of the new include directories for imgui/backends/, because the error did not happen before this PR. I already tried fiddling with forward and backward slashes, trailing slashes and so on. I also tried adding the absolute include directory for the vulkan headers on my system right in the buildfile of the vulkan backend (maybe search order is a problem?). Do you have any further ideas on this?

clang++ -IC:\develop\imgui-cc\libimgui-platform-glfw\imgui/backends -IC:\develop\imgui\libimgui-platform-glfw\imgui/backends -IC:\develop\imgui-cc\libimgui-platform-glfw -IC:\develop\imgui\libimgui-platform-glfw -IC:\develop\imgui-cc\glfw-3.3.8\include -IC:\develop\imgui-cc\glfw-3.3.8\include -D_GLFW_BUILD_DLL -IC:\develop\imgui-cc\libimgui\imgui -IC:\develop\imgui\libimgui\imgui -IC:\develop\imgui-cc\libimgui -IC:\develop\imgui\libimgui -IC:\develop\imgui-cc\libimgui-render-vulkan\imgui/backends/ -IC:\develop\imgui\libimgui-render-vulkan\imgui/backends/ -IC:\develop\imgui-cc\libimgui-render-vulkan -IC:\develop\imgui\libimgui-render-vulkan -IC:\VulkanSDK\1.3.211.0/include/ -DVK_PROTOTYPES -g -std=c++2a -finput-charset=UTF-8 -D_MT -D_DLL -w -x c++ -MQ ^ -M -MG C:\develop\imgui\imgui-examples\example_glfw_vulkan\main.cpp
In file included from C:\develop\imgui\imgui-examples\example_glfw_vulkan\main.cpp:14:
C:\develop\imgui\libimgui-render-vulkan\imgui/backends\imgui_impl_vulkan.h:43:10: fatal error: cannot open file 'C:\develop\imgui\libimgui-render-vulkan\imgui/backends\vulkan/vulkan.h': permission denied
#include <vulkan/vulkan.h>
         ^
1 error generated.
info: failed to update C:\develop\dir{imgui\}

bdep init was as follows:

bdep init -C @cc cc config.config.load=build-config\clang.debug.build

[boris-kolpackov]

Basically clang tries to resolve a include #include <vulkan/vulkan.h> in the directory imgui/backends/, where the header obviously does not exist.

This is quite bizarre and something tells me it could be due to the git's issue with Windows directory symlinks (I think what this error means is that Clang just cannot look inside this directory): https://build2.org/article/symlinks.xhtml#windows

Can you try to add libimgui-render-vulkan/imgui/backends/vulkan to .gitattributes (and any other directory symlink there might be) and see if that helps?

EDIT: according to find . -type l -xtype d this is the only directory symlink.

[Rookfighter]

@boris-kolpackov Thanks for the input. Actually I just removed the directory symlink as it was not used anyways. @Klaim and @Swat-SomeBug, if you do not have any additional points, I would like to merge these changes.

[Swat-SomeBug]

Looks good 👍🏽

[Klaim]

LGTM 👍🏽