Add XCLocalSwiftPackageReference Support #799

art-divin · 2023-10-08T22:35:33Z

Resolves krzysztofzablocki/Sourcery#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 👩‍💻👨‍💻

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

…iftPackageReference)

netlify · 2023-10-08T22:35:38Z

✅ Deploy Preview for xcodeproj ready!

Name	Link
🔨 Latest commit	`61c3f90`
🔍 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 · 2023-10-08T22:42:33Z

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 (krzysztofzablocki/Sourcery#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 🙏🏻

kwridan

Thanks for contributing this @art-divin

I wasn't aware Xcode 15 changed this 😅, local packages were previously supported as file references and Xcode somehow deals with them correctly. It seems this is still the case in Xcode 15 if you add a local package via dragging from Finder to Xcode, however if you use the Package editor UI and select "Add Local" this is where this new format is used.

From testing locally, it seems we may be missing a few pieces. I created a project with a local package, read it via XcodeProj and wrote it back to disk again - the expectation was that it would result in no diffs, however noticed a few things got omitted 🤔

-                       packageReferences = (
-                               FBE04FC42AD3E7F70014D6BD /* XCLocalSwiftPackageReference "../LocalPackage" */,
-                       );

 /* End XCConfigurationList section */
 
-/* Begin XCLocalSwiftPackageReference section */
-               FBE04FC42AD3E7F70014D6BD /* XCLocalSwiftPackageReference "../LocalPackage" */ = {
-                       isa = XCLocalSwiftPackageReference;
-                       relativePath = ../LocalPackage;
-               };
-/* End XCLocalSwiftPackageReference section */
-

It'll be helpful to include a small fixture of a project with a local package using this new format in Fixtures folder and performing a similar integration test (see some of the example test in XcodeProjIntegrationTests).

art-divin · 2023-10-10T23:00:07Z

👋🏻 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 · 2023-10-11T10:25:56Z

Hey @kwridan ,

I have added one more integration test, and seems that everything¹ 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

except for addSwiftPackage which lacks functionality to add local package as XCLocalSwiftPackageReference. ↩

art-divin · 2023-10-11T10:49:14Z

cc @pepicrft, @yonaskolb

kwridan

I have added one more integration test, and seems that everything1 is settled now.

Thanks for the updates @art-divin and for adding the fixture, note the fixture can be simplified and slimmed down to focus primarily on the local package (i.e. it wouldn't need any of the extra schemes / breakpoint files / remote packages etc...) Locally I created a dummy project with a local package and used that test.

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:

Interesting, indeed looks like it may be related to the signed commits, as locally and on CI those seem to be ok. Perhaps locally only to get it working you can modify the test helper that invokes the git command to include the appropriate extra arguments?

On the topic of this sort of test, I re-ran it against a local project I created with a local swift package and it seems a diff is still produced with the comments:

(Xcode uses relative path, vs the changes in PR utilise the name)

@@ -117,7 +117,7 @@
                        );
                        mainGroup = FBF72AFE2AD3E600003BEEEF;
                        packageReferences = (
-                               FBE04FC42AD3E7F70014D6BD /* XCLocalSwiftPackageReference "../LocalPackage" */,
+                               FBE04FC42AD3E7F70014D6BD /* XCLocalSwiftPackageReference "LocalPackage" */,
                        );
                        productRefGroup = FBF72B082AD3E600003BEEEF /* Products */;
                        projectDirPath = "";
@@ -366,7 +366,7 @@
 /* End XCConfigurationList section */
 
 /* Begin XCLocalSwiftPackageReference section */
-               FBE04FC42AD3E7F70014D6BD /* XCLocalSwiftPackageReference "../LocalPackage" */ = {
+               FBE04FC42AD3E7F70014D6BD /* XCLocalSwiftPackageReference "LocalPackage" */ = {
                        isa = XCLocalSwiftPackageReference;
                        relativePath = ../LocalPackage;
                };

it might be worth including that sort of test against the fixture?

kwridan · 2023-10-12T07:35:56Z

Sources/XcodeProj/Objects/Project/PBXProject.swift

+ /// Remote Swift packages.
+ public var remotePackages: [XCRemoteSwiftPackageReference] {


to ensure we don't introduce a breaking API change can we maintain the old variable and mark it as deprecated?

Suggested change

/// Remote Swift packages.

public var remotePackages: [XCRemoteSwiftPackageReference] {

/// Remote Swift packages.

@available(*, deprecated, message: "use remotePackages or localPackages.")

public var packages: [XCRemoteSwiftPackageReference] {

remotePackages

}

/// Remote Swift packages.

public var remotePackages: [XCRemoteSwiftPackageReference] {

Done, thank you ✅

Tests/XcodeProjTests/Objects/Project/PBXProjEncoderTests.swift

art-divin · 2023-10-12T23:22:18Z

👋🏻 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 · 2023-10-13T07:13:10Z

@all-contributors add @art-divin for code

allcontributors · 2023-10-13T07:13:20Z

@kwridan

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

kwridan

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

Awesome thank you for the great work on this, testing locally it's all working nicely now 👍

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

Done via #800

luispadron · 2023-10-13T16:38:54Z

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

art-divin · 2023-10-14T18:52:42Z

Thank you for your approvals 🤝

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

ileitch · 2023-10-29T16:27:55Z

@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.

art-divin added 4 commits October 9, 2023 02:10

Added temporary XCLocalSwiftPackageReference file (copy of XCRemoteSw…

559585f

…iftPackageReference)

Added missing cache

cb6ef3f

Removed all "remote" package reference-related code

208fdd6

Added unit tests for XCLocalSwiftPackageReferenceTests

37180e7

art-divin mentioned this pull request Oct 8, 2023

XCLocalSwiftPackageReference is not supported krzysztofzablocki/Sourcery#1206

Closed

art-divin changed the title ~~Add XCLocalSwiftPackageReferenceSupport 👍~~ Add XCLocalSwiftPackageReference Support 👍 Oct 8, 2023

kwridan reviewed Oct 9, 2023

View reviewed changes

art-divin added 3 commits October 11, 2023 02:14

Added pbxproj fixture with XCLocalSwiftPackageReference

67683c2

Added integration test to PBXProjEncoderTests

8a25a9c

Fixed failing integration test

38c78b8

art-divin added 3 commits October 11, 2023 09:41

added CHANGELOG entry

4339dd9

Added additional integration test

444d897

Fixed failing unit test

2a674d1

art-divin requested a review from kwridan October 11, 2023 13:56

tommylau-exe mentioned this pull request Oct 11, 2023

Error: XCLocalSwiftPackageReference is not supported peripheryapp/periphery#660

Closed

kwridan reviewed Oct 12, 2023

View reviewed changes

art-divin added 9 commits October 13, 2023 02:45

removed redundant fixtures

cfc0faa

Adjusted "name" variable computation for XCLocalSwiftPackageReference

7313e1e

Added fixture package in subdirectory

c5849d1

Added fixture of local package with relative path

07fee03

Added fixture with multiple local packages

569efb9

Removed force uwraping

24670cc

Exposed two new fixtures

b6a082a

Fixed wrong unit test

d10cb21

Added two integration tests for project encoding

fb612ae

art-divin added 2 commits October 13, 2023 03:19

Added deprecated packages to PBXProject

655721b

Added missing comparison in equality check

61c3f90

allcontributors bot mentioned this pull request Oct 13, 2023

docs: add art-divin as a contributor for code #800

Merged

kwridan approved these changes Oct 13, 2023

View reviewed changes

kwridan requested a review from pepicrft October 13, 2023 07:17

luispadron approved these changes Oct 13, 2023

View reviewed changes

luispadron changed the title ~~Add XCLocalSwiftPackageReference Support 👍~~ Add XCLocalSwiftPackageReference Support Oct 13, 2023

kwridan merged commit 8b5b4cf into tuist:main Oct 15, 2023
9 checks passed

kevin-kp mentioned this pull request Nov 13, 2023

Cannot remove remotePackage #802

Closed

yonaskolb mentioned this pull request Feb 14, 2024

swift run xcodegen fails yonaskolb/XcodeGen#1440

Closed

yonaskolb mentioned this pull request Apr 7, 2024

Xcode 15 local swift packages support yonaskolb/XcodeGen#1396

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add XCLocalSwiftPackageReference Support #799

Add XCLocalSwiftPackageReference Support #799

art-divin commented Oct 8, 2023 •

edited

Loading

netlify bot commented Oct 8, 2023 •

edited

Loading

art-divin commented Oct 8, 2023

kwridan left a comment

art-divin commented Oct 10, 2023

art-divin commented Oct 11, 2023

art-divin commented Oct 11, 2023

kwridan left a comment •

edited

Loading

kwridan Oct 12, 2023

art-divin Oct 12, 2023

art-divin commented Oct 12, 2023

kwridan commented Oct 13, 2023

allcontributors bot commented Oct 13, 2023

kwridan left a comment

luispadron commented Oct 13, 2023

art-divin commented Oct 14, 2023

ileitch commented Oct 29, 2023

		/// Remote Swift packages.
		public var remotePackages: [XCRemoteSwiftPackageReference] {

Add XCLocalSwiftPackageReference Support #799

Add XCLocalSwiftPackageReference Support #799

Conversation

art-divin commented Oct 8, 2023 • edited Loading

Short description 📝

Solution 📦

Implementation 👩‍💻👨‍💻

netlify bot commented Oct 8, 2023 • edited Loading

✅ Deploy Preview for xcodeproj ready!

art-divin commented Oct 8, 2023

kwridan left a comment

Choose a reason for hiding this comment

art-divin commented Oct 10, 2023

art-divin commented Oct 11, 2023

Footnotes

art-divin commented Oct 11, 2023

kwridan left a comment • edited Loading

Choose a reason for hiding this comment

kwridan Oct 12, 2023

Choose a reason for hiding this comment

art-divin Oct 12, 2023

Choose a reason for hiding this comment

art-divin commented Oct 12, 2023

kwridan commented Oct 13, 2023

allcontributors bot commented Oct 13, 2023

kwridan left a comment

Choose a reason for hiding this comment

luispadron commented Oct 13, 2023

art-divin commented Oct 14, 2023

ileitch commented Oct 29, 2023

art-divin commented Oct 8, 2023 •

edited

Loading

netlify bot commented Oct 8, 2023 •

edited

Loading

kwridan left a comment •

edited

Loading