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

Xcode 12 fixes #1615

Closed
wants to merge 6 commits into from
Closed

Xcode 12 fixes #1615

wants to merge 6 commits into from

Conversation

mman
Copy link
Contributor

@mman mman commented May 14, 2021

This is just a simple housekeeping PR to silence compiler warnings on Xcode 12 and make build clean.

@stale
Copy link

stale bot commented Jul 13, 2021

This issue has been automatically marked as stale because it has not had recent activity. If you believe it should stay open, please let us know! As always, we encourage contributions, check out the Contributing Guide

@stale stale bot added the Stale label Jul 13, 2021
@stale stale bot closed this Jul 21, 2021
@cbaker6 cbaker6 reopened this Oct 11, 2021
@stale stale bot removed the Stale label Oct 11, 2021
@mtrezza
Copy link
Member

mtrezza commented Jan 19, 2022

@mman what is the state of this PR? is this outdated or still relevant?

@mman
Copy link
Contributor Author

mman commented Jan 19, 2022

@mtrezza That's a good question actually. I had included Parse-SDK-iOS-OSX directly as a subproject since Xcode 12 days (given the fact that Cocoapods and Carthage builds were not working at that time for macCatalyst) and I had used all of the fixes mentioned in this PR since then. But I do not think it is polished and ready to go as is as I am simply removing the stuff that does not work and that I do not use. I will close this PR as I do not think it is ready.

I think the direction to go SPM and just ignore Carthage and Cocoapods was clear and is clear so I am meanwhile working on trying to bring SPM support to Bolts and Parse-SDK-iOS-OSX in a similar way @drdaz proposed.

My forks are now here:

Bolts: https://github.com/mman/Bolts-ObjC/tree/spm
Parse-SDK-iOS-OSX: https://github.com/mman/Parse-SDK-iOS-OSX/tree/spm

Both of these branches are in a state that they can be included in a SPM project and compile for all supported architectures and seem to work nicely (omitting some functionality that I had to temporarily compile out). But the code is organized in such a complex way that I could not get tests running yet, and the compilation produces lot of unnecessary warnings on watchOS and macOS.

Not sure what is the best way forward, but SPM is a must for myself...

@mman mman closed this Jan 19, 2022
@mtrezza
Copy link
Member

mtrezza commented Jan 19, 2022

It seems there are many people interested in SPM support (unsurprisingly), so I think we could gather some helping hands behind this effort. Maybe we should really ignore the carthage build and focus on SPM?

To get an overview, I found these issues / PRs and assume they are related like this:

Issue: Support Swift Package Manager #1453
PR: SPM Support #1616

Issue: FR: Carthage Xcode 12 #1591
PR: Xcode 12 compatibility #1561
PR: Xcode 12 fixes #1615

Is #1591 a requirement to get SPM working, or could we close it and related PRs?
cc @drdaz

@mman
Copy link
Contributor Author

mman commented Jan 19, 2022

@mtrezza #1591 is not required for swift package manager compatibility. #1561 addresses Xcode 12 requirements by building the SDK binaries manually using rake. So my belief is that these two can be closed.

#1453 is I think where the discussion should continue, with #1616 indicating the direction.

#1615 does contain several fixes silencing various compiler warnings without actually affecting the functionality, so yes, part of that should probably be included for clean builds.

@drdaz
Copy link
Member

drdaz commented Jan 19, 2022

Maybe we should really ignore the carthage build and focus on SPM?

I definitely feel like SPM has a brighter future than Carthage.

Is #1591 a requirement to get SPM working, or could we close it and related PRs?

XCFrameworks aren't a requirement for SPM compat.

@drdaz
Copy link
Member

drdaz commented Jan 19, 2022

#1453 is I think where the discussion should continue, with #1616 indicating the direction.

Agreed. The state of #1616 is that there's a POC where the main Parse project should work via SPM. But probably in an older SDK at this point. Tests are broken, but I think I might know the cure. It might mean re-creating the changes in that PR, but with some changes and an extra step or 3 in there.

As far as I can tell, when running the tests, the system doesn't use SPM to build stuff. So unless we can make that happen, there needs to be a complete set of imports for SPM, and a complete set of imports for the more traditional build. I think it's a non-trivial transformation 🙂

@mtrezza
Copy link
Member

mtrezza commented Jan 19, 2022

To get an idea of how urgent this is, how does Parse ObjC SDK currently build successfully with / without workaround and where does it fail? Is this correct:

Xcode cocoapods carthage add as subproject
< 12 ok ok ok
>= 12 ok only --use-xcframework ok

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.

4 participants