Skip to content

Conversation

hjmjohnson
Copy link

The header locations in apple frameworks require manipulation of the directory name in a '<pkg/myhdr.h>' include defintion.

The header file is located at <pkg.framework/Headers/myhdr.h> in a framework.

hjmjohnson added a commit to hjmjohnson/PythonQt that referenced this pull request Jun 27, 2025
Add specific framework path handling

Refactor include directory parsing to support multiple candidate paths
for headers and introduce macOS-specific logic for resolving
framework-relative header paths. Enhance generator flexibility and
robustness for cross-platform usage.

Push upstream fix via:
danmar/simplecpp#448
@danmar
Copy link
Owner

danmar commented Jun 28, 2025

Thanks for your contribution.

  • An issue would be nice.
  • And some test.
  • We haven't switched to C++11 yet so that this simplecpp library will be as portable as possible. However I think we can start supporting C++11. I think C++11 should be widely supported by now. :-)

@hjmjohnson
Copy link
Author

It might be a few weeks before I circle back to push changes upstream.

@danmar
Copy link
Owner

danmar commented Jul 2, 2025

It might be a few weeks before I circle back to push changes upstream.

ok thanks understood.. for your information we have a PR to start using C++11 so if you get back in few weeks I will hopefully not complain about you using C++11 at least.

hjmjohnson added a commit to hjmjohnson/PythonQt that referenced this pull request Jul 16, 2025
Add specific framework path handling

Refactor include directory parsing to support multiple candidate paths
for headers and introduce macOS-specific logic for resolving
framework-relative header paths. Enhance generator flexibility and
robustness for cross-platform usage.

Push upstream fix via:
danmar/simplecpp#448
hjmjohnson added a commit to hjmjohnson/PythonQt that referenced this pull request Jul 16, 2025
Add specific framework path handling

Refactor include directory parsing to support multiple candidate paths
for headers and introduce macOS-specific logic for resolving
framework-relative header paths. Enhance generator flexibility and
robustness for cross-platform usage.

Push upstream fix via:
danmar/simplecpp#448
hjmjohnson added a commit to hjmjohnson/PythonQt that referenced this pull request Jul 16, 2025
Add specific framework path handling

Refactor include directory parsing to support multiple candidate paths
for headers and introduce macOS-specific logic for resolving
framework-relative header paths. Enhance generator flexibility and
robustness for cross-platform usage.

Push upstream fix via:
danmar/simplecpp#448
The header locations in apple frameworks require manipulation
of the directory name in a '<pkg/myhdr.h>' include defintion.

The header file is located at <pkg.framework/Headers/myhdr.h>
in a framework.
This is an outline of a test. The test currently fails.  I could not figure out how these tests are working.

I would greatly appreciate a little help to understand how to use the test framework.
@hjmjohnson hjmjohnson force-pushed the enh-add-apple-framework-header-support branch from 31bffbf to de9d233 Compare July 16, 2025 21:40
@hjmjohnson
Copy link
Author

@danmar I have updated this PR, but I am struggling to figure out how to make a test to exercise this code. I tried to decipher how other tests were working, but could not identify how the "cache" would need to be setup to test the new functionality. Any guidance would be greatly appreciated.

Thanks,
Hans

@hjmjohnson
Copy link
Author

@glankk Thank you for the C++11 updates! This is great work!

This PR is adjacent to some code that you recently refactored. It would be great to have your input.

Thanks,
Hans

@glankk
Copy link
Contributor

glankk commented Jul 17, 2025

I suggest we emulate gcc's darwin-specific -F option, and possibly -iframework as well. See https://gcc.gnu.org/onlinedocs/gcc/Darwin-Options.html. The -F paths are interleaved with -I and searched left-to-right, so the order of interleaved normal include paths and frameworks should be preserved. For this we could change the DUI::includePaths from being a vector of path strings to being a vector of structs with a path string and, on darwin, a framework flag that determines what search rules to use.

@danmar
Copy link
Owner

danmar commented Jul 20, 2025

I will go on vacation for a week. Please ping me later if you want review..

@hjmjohnson
Copy link
Author

@glankk This is likely a great suggestion, but it is increasing the scope of effort beyond what I can contribute in the short term.

Do you have the bandwidth to make the changes that you recommend, if so, please take whatever is useful from my suggestion and incorporate it.

Copy link
Owner

@danmar danmar left a comment

Choose a reason for hiding this comment

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

Well I have no idea how it should work. So I'll just have to trust you that this is the proper method.
I don't know.. do you @glankk know if this looks like a proper method.. inserting ".framework/Headers" like that in the filename?

I would like that the test is enabled. the "test.cpp" is supposed to be unit/module tests.. i.e. don't depend on some header files on the system.. you could for instance unit test your changes by breaking out your code into a separate function.
imho it would be better to write the test in "integration_test.py". do you have any experience with python+pytest?

@firewave
Copy link
Collaborator

firewave commented Aug 26, 2025

The behavior should not be limited to builds made on Apple systems. It should be platform-agnostic and controlled by an explicit DUI option or the presence of __APPLE__ in the provided preprocessor macros (or both?).

@firewave
Copy link
Collaborator

The tests would probably be best implemented in Python.

@firewave
Copy link
Collaborator

And shouldn't this be done for system headers only? I doubt you want to look up random local includes.

@jcfr
Copy link
Contributor

jcfr commented Aug 26, 2025

This pull request should be closed as it is superseded by:

For additional context, @hjmjohnson and I have been working together on this.

@hjmjohnson
Copy link
Author

Comprehensive solution addressed in #511.

@hjmjohnson hjmjohnson closed this Aug 27, 2025
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.

5 participants