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

Reuse ReferenceProxy references when possible #1354

Merged
merged 3 commits into from
May 17, 2023

Conversation

OdNairy
Copy link
Contributor

@OdNairy OdNairy commented May 1, 2023

XcodeGen should reuse PBXReferenceProxy for external targets when possible. In the current state, Xcode will remove all duplicated dependencies as soon as any change in pbxproj happens.

Screen.Recording.2023-05-01.at.10.49.16.mov

@OdNairy OdNairy force-pushed the bugfix/project-references branch from 64c0399 to 6a85466 Compare May 1, 2023 07:51
@OdNairy OdNairy force-pushed the bugfix/project-references branch from 6a85466 to 49dba0c Compare May 1, 2023 08:01
@OdNairy
Copy link
Contributor Author

OdNairy commented May 16, 2023

@yonaskolb Is there anything I can do to help you to review this PR?

Copy link
Owner

@yonaskolb yonaskolb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @OdNairy, looks good

CHANGELOG.md Outdated Show resolved Hide resolved
@yonaskolb yonaskolb merged commit 372f20f into yonaskolb:master May 17, 2023
@OdNairy OdNairy deleted the bugfix/project-references branch May 17, 2023 08:39
@antonyalkmim
Copy link

Thanks for this fix @OdNairy 👏.

I'm facing a problem using targetTemplates that I think this PR solves. Unfortunately I cant run some targets because the dependencies are not being imported on some targets that uses the same template. Clone and run xcodegen locally solves my problem.

@yonaskolb are you waiting for more PRs to add on next release or there is a release date?
Can you release this fix for us? 😅

@liamnichols
Copy link
Contributor

I was currently investigating something similar to #1339 in my project file and I wanted to test with the latest master to try and dig into the issue, but I think something in this commit might have added to the problem.

Prior to this change, I had one unused object reference for a PBXGroup that I was investigating, but now there are a lot of unused PBXContainerItemProxy references in the generated project file:

Screenshot 2023-07-25 at 17 42 53

(R.swift has a linting tool that is picking this up for me)

The references seem to be related to targets from a different Xcode project of mine:

Screenshot 2023-07-25 at 17 43 45

TofuClient, XCTestKit, APIKit, ImageKit etc are all targets in a referenced project called Frameworks.xcodeproj

Perhaps the missing piece here is that we're still creating new PBXContainerItemProxy objects when we reuse the PBXReferenceProxy which is leading to the orphaned object?

@liamnichols
Copy link
Contributor

Ah yes, further up in the file, we're always adding the container item when we create it:

let productProxy = addObject(
PBXContainerItemProxy(
containerPortal: .fileReference(projectFileReference),
remoteGlobalID: targetObject.product.flatMap(PBXContainerItemProxy.RemoteGlobalID.object),
proxyType: .reference,
remoteInfo: target
)
)

But really I think that we need to do something like this:

diff --git a/Sources/XcodeGenKit/PBXProjGenerator.swift b/Sources/XcodeGenKit/PBXProjGenerator.swift
index 6e77c3f..9ad0c91 100644
--- a/Sources/XcodeGenKit/PBXProjGenerator.swift
+++ b/Sources/XcodeGenKit/PBXProjGenerator.swift
@@ -407,13 +407,11 @@ public class PBXProjGenerator {
             )
         )
 
-        let productProxy = addObject(
-            PBXContainerItemProxy(
-                containerPortal: .fileReference(projectFileReference),
-                remoteGlobalID: targetObject.product.flatMap(PBXContainerItemProxy.RemoteGlobalID.object),
-                proxyType: .reference,
-                remoteInfo: target
-            )
+        let productProxy = PBXContainerItemProxy(
+            containerPortal: .fileReference(projectFileReference),
+            remoteGlobalID: targetObject.product.flatMap(PBXContainerItemProxy.RemoteGlobalID.object),
+            proxyType: .reference,
+            remoteInfo: target
         )
 
         var path = targetObject.productNameWithExtension()
@@ -437,6 +435,7 @@ public class PBXProjGenerator {
         if let existingValue = existingValue {
             productReferenceProxy = existingValue
         } else {
+            addObject(productProxy)
             productReferenceProxy = addObject(
                 PBXReferenceProxy(
                     fileType: productReferenceProxyFileType,

It fixed the issue in my project file, but I need to run through and see if there is more to it than that or not.

@yonaskolb
Copy link
Owner

@liamnichols would you mind opening a PR for this?

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