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

Karim/exclude non yoga search paths #1779

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

karim-alweheshy
Copy link

in rules_swift_package_manager this is necessary to exclude these from search path for public headers that will fail to compile eventually

Copy link

vercel bot commented Jan 21, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
yoga-website ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jan 21, 2025 10:27am

@facebook-github-bot
Copy link
Contributor

Hi @karim-alweheshy!

Thank you for your pull request and welcome to our community.

Action Required

In order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you.

Process

In order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.

Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with CLA signed. The tagging process may take up to 1 hour after signing. Please give it that time before contacting us about it.

If you have received this in error or have any questions, please contact us at [email protected]. Thanks!

@karim-alweheshy karim-alweheshy force-pushed the karim/exclude_non_yoga_search_paths branch from 229325e to b2e5d7c Compare January 21, 2025 10:22
@facebook-github-bot
Copy link
Contributor

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Meta Open Source project. Thanks!

@facebook-github-bot facebook-github-bot added CLA Signed Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team. labels Jan 21, 2025
@NickGerleman
Copy link
Contributor

Could you help provide a bit more context? What error are you seeing?

We have a modulemap for generated umbrella header for Swift usage with the list of public header: https://github.com/facebook/yoga/blob/main/yoga/module.modulemap Wonder if rules_swift_package_manager might not be picking this up?

includes within the target shouldn't ever reference anything outside the yoga directory then.

@karim-alweheshy
Copy link
Author

Could you help provide a bit more context? What error are you seeing?

We have a modulemap for generated umbrella header for Swift usage with the list of public header: https://github.com/facebook/yoga/blob/main/yoga/module.modulemap Wonder if rules_swift_package_manager might not be picking this up?

includes within the target shouldn't ever reference anything outside the yoga directory then.

Because the public headers search path is set to be on the whole repo, rules_swift_package_manager compiles all public headers from all folders. This compilation fails because imported symbols in these headers are not part of the source.

Excluding the other folders enables rules_swift_package_manager to use the public header only in the yoga folder and not the rest

@NickGerleman
Copy link
Contributor

NickGerleman commented Jan 24, 2025

Describing the set of public headers, for the purpose of creating an umbrella header, is what the modulemap is for, at least when consuming as a Clang module. Or is rules_swift_package_manager combining headers for some non Clang module purpose? Having a copy of the error would be helpful here.

Getting the SwiftPM package to work with Bazel seems like good goal, but this behavior seems funky compared to what SwiftPM itself does/expects. Trying to understand more, to know if maybe this is a bug with rules_swift_package_manager that should be reported.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants