Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Include Headers in the lib #655

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Include Headers in the lib #655

wants to merge 6 commits into from

Conversation

Treata11
Copy link
Contributor

Greetings,

When I built the library for iOS, I realized that there were no Header directories included in the framework bundle.

cmake -S. -B _build -DPUGIXML_BUILD_APPLE_FRAMEWORK=TRUE -DPUGIXML_INSTALL=TRUE -DCMAKE_SYSTEM_NAME=iOS -DPUGIXML_USE_VERSIONED_LIBDIR=ON

sudo cmake --build _build --config Release --target install

I searched the entire build dir & didn't find any trace of the headers.

So, I included the headers in the lib & also added them in the framework's bundle.

@@ -182,8 +189,10 @@ if (PUGIXML_BUILD_APPLE_FRAMEWORK)
set_target_properties(${libs} PROPERTIES
FRAMEWORK TRUE
FRAMEWORK_VERSION ${PROJECT_VERSION}
XCODE_ATTRIBUTE_PRODUCT_BUNDLE_IDENTIFIER com.zeux.pugixml
MACOSX_FRAMEWORK_IDENTIFIER com.zeux.pugixml
XCODE_ATTRIBUTE_PRODUCT_BUNDLE_IDENTIFIER "github.com/zeux/pugixml"
Copy link
Owner

@zeux zeux Jan 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure how these get determined but why did this change?

Copy link
Contributor Author

@Treata11 Treata11 Jan 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I updated the identifiers to match the real URL (It'll be included in the .plist of the framework).
And PUBLIC_HEADER has to be specified. I overlooked it in the previous PR.

@@ -135,7 +141,8 @@ endif()

if (NOT BUILD_SHARED_LIBS OR PUGIXML_BUILD_SHARED_AND_STATIC_LIBS)
add_library(pugixml-static STATIC
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why did this change?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there another way to include the headers in the built library?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

target_include_directories could be used instead... There isn't much of a difference, if any.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Static libraries don't include header files, so I'm not sure what this is changing. I'll need to look at how other projects do xcframework setup but it's surprising that any change is necessary here, beyond the change for framework installation.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also worth noting I guess is that xcframework files are dynamic libraries... I'll need to test this fully to make sure I understand what is going on here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Static libraries don't include header files, so I'm not sure what this is changing. I'll need to look at how other projects do xcframework setup but it's surprising that any change is necessary here, beyond the change for framework installation.

iOS-derived frameworks are actually static & they require the header files... dylibs are not allowed on iOS.

Copy link
Contributor Author

@Treata11 Treata11 Jan 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also worth noting I guess is that xcframework files are dynamic libraries...

xcframeworks are just a bunch of built frameworks bundled in a single package using XCode-CLI.

I'll need to test this fully to make sure I understand what is going on here.

Of course, I actually appreciate that.
https://cmake.org/cmake/help/latest/prop_tgt/FRAMEWORK.html#prop_tgt:FRAMEWORK

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.

2 participants