parse-community / Parse-SDK-iOS-OSX

The Apple SDK for Parse Platform (iOS, macOS, watchOS, tvOS)
https://parseplatform.org
Other
2.81k stars 871 forks source link

fix: SPM build issues with Xcode 16 #1795

Closed roostr closed 2 months ago

roostr commented 2 months ago

New Pull Request Checklist

Issue Description

There is an issue with the Package.swift file where it uses wildcard globbing to refer to all the private header directories that should be included when building.

It turns out that it was a mistake that Xcode ever supported wildcard globbing in the first place, as described here:

https://forums.swift.org/t/recursive-headersearchpath/59769

Closes: #1793

Approach

To fix it, we are replacing the wildcard glob with a list of all header directories.

I generated the list of directories with:

find Internal -type d | sort | pbcopy

This issue was first observed on Xcode 16.0.0 beta 1. I confirmed that it now builds correctly on Xcode 16.0.0 beta 2.

TODOs before merging

I would appreciate if someone could verify that this correctly compiles on both Xcode 15 and Xcode 16, using SPM for both.

parse-github-assistant[bot] commented 2 months ago

Thanks for opening this pull request!

mtrezza commented 2 months ago

This issue was first observed on Xcode 16.0.0 beta 1. I confirmed that it now builds correctly on Xcode 16.0.0 beta 2.

Has this only been a temporary bug in a beta version of Xcode 16? If I understand correctly, you are saying that the bug has been fixed in a later beta version and this PR here would not be necessary anymore?

cc @bmueller

codecov[bot] commented 2 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 72.28%. Comparing base (dd05d41) to head (9f82f46). Report is 13 commits behind head on master.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #1795 +/- ## ========================================== + Coverage 64.24% 72.28% +8.04% ========================================== Files 201 148 -53 Lines 23233 17268 -5965 ========================================== - Hits 14926 12483 -2443 + Misses 8307 4785 -3522 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

roostr commented 2 months ago

This has been confirmed to still be an issue in the latest Xcode 16 beta, which is beta 3 (not by me, but I heard it from someone else).

It seems Xcode 16 actually fixed a bug, and that's why we need this PR.

In this Swift forums thread from 2022, someone asked about the globbing behavior, and this is the relevant reply from NeoNacho:

I think the Xcode behavior is technically a bug here, the package manifest model currently does not define support for globbing. What's happening is likely that the string value is directly put into Xcode's native HEADER_SEARCH_PATHS build setting which does support this type of globbing.

this was never intended to work.

It sounds like earlier versions of SPM unintentionally supported globbing, and Apple has likely fixed that and removed globbing support. So we would need to list all directories for forward compatibility.

I looked at a few other SPM modules I use, and they all list out all directories they use, just as this PR does.

bmueller commented 2 months ago

I just confirmed that this pull request is working on Xcode 16 beta 3. Thanks Russ!

@mtrezza yes, this PR is necessary to get Parse building via SPM in the Xcode beta. (Also still need to manually register subclasses as per issue #1792)

roostr commented 2 months ago

This issue was first observed on Xcode 16.0.0 beta 1. I confirmed that it now builds correctly on Xcode 16.0.0 beta 2.

Has this only been a temporary bug in a beta version of Xcode 16? If I understand correctly, you are saying that the bug has been fixed in a later beta version and this PR here would not be necessary anymore?

Whoops, now I see how my statement was poorly worded. Let me re-state that.

Issue #1793 was first seen in Xcode 16.0.0 beta 1. It was confirmed to still exist in Xcode 16.0.0 beta 2 and beta 3. We should expect this to not be addressed in future versions of Xcode 16 beta, because we are probably seeing this as a result of a fix by Xcode of a previously undesired behavior.

With the PR fix applied, we have verified that the build is now fixed in Xcode 16.0.0 beta 2 and Xcode 16.0.0 beta 3.

I also just confirmed that with the PR fix applied, it continues to build correctly in Xcode 15.4.0.

mtrezza commented 2 months ago

Sounds good to me. Xcode 16 is in beta, but we do not run our CI with prerelease versions. Since everyone confirmed that this PR fixes the issue we can merge it, but in order to prevent regression we will need to add Xcode 16 to our CI once it's released.

mtrezza commented 2 months ago

@bmueller could you please confirm that this PR closes issue #1793 before we merge?

bmueller commented 2 months ago

Confirmed.

parseplatformorg commented 2 months ago

🎉 This change has been released in version 4.1.1