Add XCLocalSwiftPackageReference Support

art-divin commented 1 year ago

Resolves https://github.com/krzysztofzablocki/Sourcery/issues/1206

Short description 📝

This PR implements parsing of new element type named XCLocalSwiftPackageReference which seems to be introduced with the release of Xcode 15.

Solution 📦

To support the new element type, I simply copied everything related to XCRemoteSwiftPackageReference and removed all code related to VersionMatching and "URL". Instead, only path argument is supported for instantiating XCLocalSwiftPackageReference.

Implementation 👩‍💻👨‍💻

[x] Adopt XCRemoteSwiftPackageReference to XCLocalSwiftPackageReference
[x] Add XCLocalSwiftPackageReference to PBXObjectParser switch statement
[x] Add caching of XCLocalSwiftPackageReference objects in PBXObjects
[x] Add XCLocalSwiftPackageReference support in PBXProject
[x] Add integration tests to PBXProjEncoderTests

netlify[bot] commented 1 year ago

Deploy Preview for xcodeproj ready!

Name	Link
Latest commit	61c3f903617d4700f211a73d1918f68d9a905126
Latest deploy log	https://app.netlify.com/sites/xcodeproj/deploys/65287f2d07f6280008b884d1
Deploy Preview	https://deploy-preview-799--xcodeproj.netlify.app
Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

art-divin commented 1 year ago

Hello @kwridan, @pepicrft, @yonaskolb 👋🏻

Firstly, thank you very much for supporting this project. It is used by me in many different Swift scripts I wrote.

Could you please review this PR? It is mentioned in Sourcery (https://github.com/krzysztofzablocki/Sourcery/issues/1206) and other projects which face this issue on Xcode 15. I am surprised that it was already implemented in Xcodeproj, but not here 🕵🏻

I hope I did not miss anything, but if I did, could you please be so kind informing me, I'll apply the change as soon as I can.

Thank you 🙏🏻

art-divin commented 1 year ago

👋🏻 Hello @kwridan ,

thank you very much for your test!

Apparently, decoding was not set properly. So I have added an integration test to PBXProjEncoderTests and the missing implementation to PBXProjEncoder.swift.

What I have discovered is that addSwiftPackage is missing for "the new way" of adding local references.

I guess it is better to split this feature into multiple PRs, and firstly, implement support to unblock further work.

What do you think? Any other tests I'd be better adding?

Thank you 🙏🏻

art-divin commented 1 year ago

Hey @kwridan ,

I have added one more integration test, and seems that everything[^1] is settled now.

Please let me know if this PR can be merged or still it is missing somethig.

P.S. when run locally, I got failed unit tests related to git checks, specifically, locally I have enabled all commits signed by default with gpg. So when run, these unit tests fail with the following error:

test_read_write_produces_no_diff(): failed: caught error: "Error Domain=NSPOSIXErrorDomain Code=128 "Unknown error: 128""

in file XCTestCase+Shell.swift on line 21. Console states the following:

Test Case '-[XcodeProjTests.XcodeProjIntegrationTests test_read_write_produces_no_diff]' started.
error: cannot run gpg: No such file or directory
error: gpg failed to sign the data
fatal: failed to write commit object

[^1]: except for addSwiftPackage which lacks functionality to add local package as XCLocalSwiftPackageReference.

art-divin commented 1 year ago

cc @pepicrft, @yonaskolb

art-divin commented 1 year ago

👋🏻 Hey @kwridan ,

Thank you for your throughout review 🤝

I have addressed all the points mentioned and hopefully this time PR is ready.

However, I noticed that some other PRs also include changes related to contributors. Should I do it here as well? Anything else?

Thank you

kwridan commented 1 year ago

@all-contributors add @art-divin for code

allcontributors[bot] commented 1 year ago

@kwridan

I've put up a pull request to add @art-divin! :tada:

luispadron commented 1 year ago

One nit would be to squash this PR before merging there are few commits in here

art-divin commented 1 year ago

Thank you for your approvals 🤝

Please merge this MR at your convenience, I do not have enough privileges, thank you 👍🏻

ileitch commented 1 year ago

@pepicrft Any chance this change can be released soon? People creating new projects in Xcode 15 that use local package references are currently unable to use XcodeProj (or tools that depend on it) because project parsing fails with an error.

tuist / XcodeProj