Use Quoted Includes for Header-to-Header Inclusion

[omochi]

This PR is part of an effort to support SwiftPM (#293), but to keep things organized, I am splitting it into smaller, focused changes. This particular PR addresses one such change: modifying how XLXmlParser.hpp includes XLException.hpp by switching from angle brackets to quotes.

This change is necessary for SwiftPM compatibility.

To provide some context, there are two types of builds to consider:

1. Building OpenXLSX itself: In this case, there is no issue.

   • Both headers and sources are compiled together.
   • The OpenXLSX codebase requires the following search paths:
     • OpenXLSX/
     • OpenXLSX/headers/
     • OpenXLSX/external/pugixml/
     • OpenXLSX/external/zippy/
   • For example, the third search path is necessary because XLCell.hpp includes #include <pugixml.hpp>.

2. Building targets that use OpenXLSX: Here, SwiftPM's technical limitations introduce issues.

   • Unfortunately, SwiftPM's C++ targets can only configure one include path for a library, known as the public header directory.
   • Setting this path to OpenXLSX/ allows the following includes:

     #include <OpenXLSX.hpp>
     #include <headers/XLCell.hpp>
     #include <external/pugixml/pugixml.hpp>

   • However, these includes are not possible:

     #include <XLCell.hpp>
     #include <pugixml.hpp>

     This is because OpenXLSX/headers/ and OpenXLSX/external/pugixml/ are not included in the include paths.

Luckily, this issue affects only the single location addressed by this patch. This is because, during the build of targets using OpenXLSX, headers are part of the compilation, but source files are not.

In the current codebase, pugixml includes are already written differently for headers and source files:

• In source files:

  #include <pugixml.hpp>

• In header files:

  #include <external/pugixml/pugixml.hpp>

To address this, we need to fix how XLXmlParser.hpp includes XLException.hpp. Both files are in the same directory, so using a quoted include resolves the path as a relative path:

#include "XLException.hpp"

In fact, the following files already include XLException.hpp using quoted includes:

• XLCellValue.hpp
• XLComments.hpp
• XLDateTime.hpp
• XLRowData.hpp
• XLSheet.hpp
• XLXmlParser.hpp
• XLCellIterator.cpp
• XLCellReference.cpp
• XLCellValue.cpp
• XLColor.cpp
• XLDateTime.cpp

XLXmlParser.hpp is the only exception to this pattern, making it an outlier.

[aral-matrix]

As I said in the other PR, the decision on whether to support an entirely different build system is up to @troldal - as for the includes, I will not touch them project-wide - I have broken things by changing some include logic in the past, this would require extensive testing to see if it makes sense.
As I also explained, there is no guaranteed difference between using angle brackets and quotes for include statements, so I wouldn't change those anyways.

[omochi]

Thank you for the explanation. I’ll wait for @troldal’s comment.

there is no guaranteed difference between using angle brackets and quotes for include statements

There is an important difference.
Angle brackets instruct the compiler to search in the specified search path list. In contrast, double quotes also allow resolution via relative paths.

For example, if a.c, a.h, and b.h are in the same directory, within a.c or a.h, you can write:

#include "b.h"

and it will be resolved. On the other hand:

#include <b.h>

will only work if the directory is included in the compiler’s search paths.

The key difference is that the former (using double quotes) guarantees resolution based solely on the library’s source tree structure, while the latter (using angle brackets) depends on the build configuration of the library user.

This proposed change specifically focuses on addressing this distinction.

[aral-matrix]

I also once misunderstood the include preprocessor directive like you seemingly do. But that is not how it works:
https://en.cppreference.com/w/cpp/preprocessor/include

Check the Explanation for (2):

The named source file is searched for in an implementation-defined manner.

[omochi]

I’m not trying to discuss strict standards here.
The topic is about the behavior specific to clang integrated with the Swift compiler, as used in SwiftPM builds.

This compiler clearly produces the following error message.

Moreover, if there are no differences with other compilers, there would be no reason to oppose this change from the perspective of maintaining the codebase.

[aral-matrix]

Fix your include path configuration and the error goes away.

Specifically, in your setup, you did not completely replicate these include directories from OpenXLSX/CMakeLists.txt:

#=======================================================================================================================
# EXTERNAL LIBRARIES
#   Define external libraries used by OpenXLSX. The libraries (Zippy, PugiXML, and NoWide) are header-only, so
#   INTERFACE libraries should be defined.
#=======================================================================================================================

if (OPENXLSX_ENABLE_NOWIDE)
    add_library(NoWide INTERFACE IMPORTED)
    target_include_directories(NoWide SYSTEM INTERFACE $<BUILD_INTERFACE:${CMAKE_CURRENT_LIST_DIR}/external/nowide/>)
endif()

add_library(Zippy INTERFACE IMPORTED)
target_include_directories(Zippy SYSTEM INTERFACE $<BUILD_INTERFACE:${CMAKE_CURRENT_LIST_DIR}/external/zippy/>)
if (OPENXLSX_ENABLE_NOWIDE)
    target_compile_definitions(Zippy INTERFACE ENABLE_NOWIDE)
endif ()

add_library(PugiXML INTERFACE IMPORTED)
target_include_directories(PugiXML SYSTEM INTERFACE $<BUILD_INTERFACE:${CMAKE_CURRENT_LIST_DIR}/external/pugixml/>)

[..]

#=======================================================================================================================
# STATIC LIBRARY
#   Define the static library
#=======================================================================================================================
if ("${OPENXLSX_LIBRARY_TYPE}" STREQUAL "STATIC")
    add_library(OpenXLSX STATIC "")
    add_library(OpenXLSX::OpenXLSX ALIAS OpenXLSX)
    target_sources(OpenXLSX PRIVATE ${OPENXLSX_SOURCES})
    target_include_directories(OpenXLSX
            PUBLIC
            $<BUILD_INTERFACE:${CMAKE_CURRENT_LIST_DIR}>
            $<BUILD_INTERFACE:${CMAKE_CURRENT_LIST_DIR}/headers>
            $<BUILD_INTERFACE:${CMAKE_CURRENT_BINARY_DIR}>)     # For export header
    target_link_libraries(OpenXLSX

[..]

[omochi]

Yes, that’s correct.

If I can replicate the include path configuration that CMake sets up, I can build without modifying the codebase.

Specifically, in this case, it’s necessary to add the OpenXLSX/headers/ directory to the path.
In the CMake source, this is configured with:

$<BUILD_INTERFACE:${CMAKE_CURRENT_LIST_DIR}/headers>

Unfortunately, due to the limitations of SwiftPM, it is not possible to replicate this.

SwiftPM only allows setting one directory for a library’s public headers.
As a result, only OpenXLSX/ can be set as the path.

[aral-matrix]

In that case, this sounds like you should open a pull request / issue at https://github.com/swiftlang/swift-package-manager for supporting the functionality that every C++ compiler has.

[omochi]

That’s one reasonable approach.
I’ve created an issue for it.
swiftlang/swift-package-manager#8130

[aral-matrix]

It looks like your issue on the swift package manager is well received by at least one developer. Is it okay if you withdraw / close this pull request?

[omochi]

No, setting aside the issue with SwiftPM, I’d like to hear @troldal’s opinion on the following matter:

I believe the use of angle brackets for including XLException.hpp from XLXmlParser.hpp, while all other headers include XLException.hpp using quotes, is likely just an implementation oversight.

It would be more desirable for the codebase to maintain consistency in this regard. This patch is unrelated to SwiftPM and represents a valuable improvement for the OpenXLSX codebase itself.