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

Allow umbrella headers to share a "root" directory in C++ Interop. #7185

Closed
furby-tm opened this issue Dec 11, 2023 · 14 comments
Closed

Allow umbrella headers to share a "root" directory in C++ Interop. #7185

furby-tm opened this issue Dec 11, 2023 · 14 comments

Comments

@furby-tm
Copy link

furby-tm commented Dec 11, 2023

Description

Hello SwiftPM team,

First off, C++ introp included with Swift 5.9 is a major success, after successfully building much of the computer graphics landscape in the pursuit of building Pixar's USD for the Swift programming language with the SwiftUSD project.

This is a project I'd like to tie into the official OpenUSD project, however, due to the existing directory structure of OpenUSD, SwiftPM imposes the restructuring of the entire project as you can see in the SwiftUSD project.

The reason this restructuring is necessary is due to the rule that all Swift target headers must not be visible to other Swift targets with the publicHeadersPath field. However, I believe that even if a shared "root" include path is used, we can still uniquely identify the paths to each target's respective "contextual headers" within a shared "root" include path.

Is there any possibility of adding more flexibility to how SwiftPM accepts these headers, one that would allow for the directory structure of OpenUSD as well as other project directory structures as seen across the ASWF landscape - I believe with this feature we can see rapid (or, shall I say swift 😉) adoption of both the Swift programming language, and the power of SwiftPM across the industry.

Thank you,

Tyler Furby

Swift & OS version (output of swift --version && uname -a)

swift-driver version: 1.87.3 Apple Swift version 5.9.2 (swiftlang-5.9.2.2.56 clang-1500.1.0.2.5)
Target: arm64-apple-macosx14.0
Darwin wabitwo.local 23.2.0 Darwin Kernel Version 23.2.0: Fri Oct 13 09:27:54 PDT 2023; root:xnu-10002.60.54~14/RELEASE_ARM64_T6000 arm64

@neonichu
Copy link
Member

Do you have a more concrete (ideally smaller) example of which header structure currently doesn't work?

@furby-tm
Copy link
Author

Do you have a more concrete (ideally smaller) example of which header structure currently doesn't work?

Sure, I will provide a simple example repository with a Package.swift, to replicate that Swift targets are not allowed to have a shared include directory visible to other swift targets.

@furby-tm
Copy link
Author

furby-tm commented Dec 12, 2023

@neonichu Here is an example repository, when running swift build, the following error message says the following:

error: 'helloswiftpm': target 'HelloSwiftPM' has invalid header layout: umbrella header found at '/Users/furby/Wabi/HelloSwiftPM/HelloSwiftPM/HelloSwiftPM.h', but directories exist next to it: /Users/furby/Wabi/HelloSwiftPM/HelloSwiftPM/SayHello, /Users/furby/Wabi/HelloSwiftPM/HelloSwiftPM/SayHelloAgain; consider removing them
error: 'helloswiftpm': target 'SayHello' has overlapping sources: /Users/furby/Wabi/HelloSwiftPM/HelloSwiftPM/SayHello/SayHello.cpp
error: 'helloswiftpm': target 'HelloSwiftPM' has invalid header layout: umbrella header found at '/Users/furby/Wabi/HelloSwiftPM/HelloSwiftPM/HelloSwiftPM.h', but directories exist next to it: /Users/furby/Wabi/HelloSwiftPM/HelloSwiftPM/SayHello, /Users/furby/Wabi/HelloSwiftPM/HelloSwiftPM/SayHelloAgain; consider removing them
error: 'helloswiftpm': target 'SayHello' has overlapping sources: /Users/furby/Wabi/HelloSwiftPM/HelloSwiftPM/SayHello/SayHello.cpp

@neonichu
Copy link
Member

You should be able to avoid the invalid header layout errors by supplying your own module map instead of relying on the generator.

@furby-tm
Copy link
Author

furby-tm commented Dec 12, 2023

Would you be able to show me a module.modulemap file that would work for this example? When I set it up I receive an error that 'helloswiftpm': target 'SayHello' has overlapping sources.

It consists of 3 modules,

HelloSwiftPM which has no dependencies.

SayHello which depends on HelloSwiftPM

and SayHelloAgain which depends on HelloSwiftPM and SayHello.

I can't seem to be able to create a module map which does not attempt to overlap the sources, or at least one that SwiftPM will see as 3 separate non-overlapping sources.

@furby-tm
Copy link
Author

furby-tm commented Dec 12, 2023

I have updated the module maps with the closest I was able to get to compile just fine and it also runs, however, I am unable to refer back to the root include directory of HelloSwiftPM for the other C++ targets, as the paths must now all be stripped down to their own respective modules such as:

#include "HelloSwiftPM.h"
#include "SayHello/SayHello.h"
#include "SayHelloAgain/SayHelloAgain.h"

The end result I am looking for is something like the following to reference each of the include paths in the C++ targets:

#include "HelloSwiftPM/HelloSwiftPM.h"
#include "HelloSwiftPM/SayHello/SayHello.h"
#include "HelloSwiftPM/SayHelloAgain/SayHelloAgain.h"

It is stupid and silly for this example, however, with large existing projects this becomes a problem with being able to add a Package.swift in an existing project, especially with rather large ones such as USD, USD to tie it back to this example, references all includes project-wide from the "root" in that project's case the pxr/ directory which is synonymous to the HelloSwiftPM/ directory in the code example, in terms of project directory structure and how it references that root path in all C++ source code.

In addition to this, after messing with the exclude paths, attempting to build them all from the same "root" by omitting only each target's respective source from the exclusions list, seems to wrongly complain about overlapping sources which leads me to believe that the path parameter takes precedence over the exclude parameter, should that be the case since the exclude parameter takes precedence over the sources parameter?

@furby-tm
Copy link
Author

Here is that revision:

@neonichu furby-tm/HelloSwiftPM@e50d877

@furby-tm furby-tm changed the title Allow more flexibility of umbrella headers in C++ Interop. Allow umbrella headers to share a "root" directory in C++ Interop. Dec 12, 2023
@furby-tm
Copy link
Author

furby-tm commented Dec 12, 2023

As another attempt I was able to get things to work with everything sharing that root prefix, but it feels like more of a hack that requires moving all project directories into a secondary root directory of the same name (and the reason a module.modulemap file cannot go in the repository root is because SwiftPM will pick up on the other root level directories such as .git and not allow it, despite those paths being added to the exclude parameter array):

@neonichu
furby-tm/HelloSwiftPM@bf0b7a8

@egorzhdan
Copy link
Collaborator

@furby-tm you could also try to make the USD target a systemLibrary in SwiftPM, that would make SwiftPM a bit more relaxed when it comes to the paths of headers.

@furby-tm
Copy link
Author

@egorzhdan Thanks! I'll take a look at systemLibrary as well, I'm awfully close in that last revision though to get exactly what I need, I just need to be able to refer to that top level HelloSwiftPM directory without having to create that secondary HelloSwiftPM within it, do you think I'm about at the limits with that approach?

Im thinking there's got to be a way that I can slap a module.modulemap at the repo root, because that should fix it, but I wasn't sure if you've ever seen or attempted to have a module map at the repositories root before.

@furby-tm
Copy link
Author

furby-tm commented Dec 12, 2023

@egorzhdan In perhaps the strangest of ways, I was able to bring the "root include" in by setting one of the swift target's path (with it's public headers directory) to the root of the swift package repo, now when other targets depend on it - that target includes the root.

It's real ugly! But it appears that this is working, any possible side effects of doing this?
furby-tm/HelloSwiftPM@081f988

And if there is any possibility this won't have to be the case longer term? Because I hate it 😆 I will try to pull together a list of some other projects where this sort of structure is common as well, to gauge how beneficial it could be for SwiftPM to alleviate this overhead (if at all possible), anything to promote the ease of use when it comes to incorporating a Package.swift within existing C++ projects is something that I believe everyone can benefit from.

@furby-tm
Copy link
Author

furby-tm commented Dec 13, 2023

Hey folks! I really appreciate your time and help, currently I am able to build OpenUSD, no changes from the official repo structuring, and things are building great with this Swift Package, it required the strange workarounds that I noted above, but I'm just glad to see everything working!

~ Keep up the great work with SwiftPM 🎉

@furby-tm
Copy link
Author

furby-tm commented Dec 14, 2023

Quick update with the above workaround, after spending more time with it and messing around with it a bit more, that does not seem to be a suitable fix for large projects with dependencies due to modules getting re-imported and those modules additionally getting re-imported into other dependencies' namespace blocks, sort of turning into a gigantic mess, this wall of errors is only experienced once you are in Swift and you import the CXX interop module.

It appears all SwiftPM is missing here is the ability to support building CXX interop targets without a "project source directory" usually marked by src; or in Swift project convention a Sources directory.

For some reason, SwiftPM picks up on directories such as .build and .git at the root of the project repo residing next to a CXX interop target path at the root of the project repo if you attempt to build a CXX interop target with a target configuration of path: "." as a parameter, complaining that you should "consider reducing the number directories to one", regardless if you exclude all other directories (like .build and .git) from the project repository root in the exclude: [] parameter, I believe this is also only the case when there is a umbrella header in that root directory as well.

@furby-tm
Copy link
Author

furby-tm commented May 1, 2024

Closing in favor of #7413

@furby-tm furby-tm closed this as completed May 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants