Skip to content

Add Support to Local Swift Packages from SPM, Xcode Project & Workspaces#10

Open
rogerluan wants to merge 24 commits into
maxchuquimia:masterfrom
rogerluan:feature/local-swift-packages
Open

Add Support to Local Swift Packages from SPM, Xcode Project & Workspaces#10
rogerluan wants to merge 24 commits into
maxchuquimia:masterfrom
rogerluan:feature/local-swift-packages

Conversation

@rogerluan

@rogerluan rogerluan commented Sep 5, 2021

Copy link
Copy Markdown
Collaborator

Description

This PR resolves #8

NOTE: This PR branches off of #7 , please review that one first. Since I can only work off of GitHub Forks, I can't point this PR to that other branch 😞

Discussion

Feel free to ask as many questions as needed @maxchuquimia 🙇 There're a looot of changes haha

Please don't mind about code style issues (issues that would be caught by a linter) right now. We can fix those as we move forward with the linter PR 👍

Resources

These docs have helped me a lot: https://www.rubydoc.info/gems/xcodeproj/

Demo

xcgrapher --workspace="SomeApp.xcworkspace" --scheme="SomeAppScheme" --spm && open /tmp/xcgrapher

image

…ages

# Conflicts:
#	Sources/XCGrapherLib/XCGrapher.swift

@maxchuquimia maxchuquimia left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

This is a huuuge amount of work and it looks good, well done! I've read through the code and left a few comments but haven't had time to play with it in Xcode yet - which I'd like to do after it's been rebased on top of our more recent merges, so will find some time for it later.

But, looking good so far!

Comment thread Sources/XCGrapherLib/Helpers/ShellTask.swift
Comment thread Sources/XCGrapherLib/ShellTasks/Xcodeproj.swift
Comment thread Sources/XCGrapherLib/ShellTasks/SwiftPackage.swift Outdated
Comment thread Sources/XCGrapherLib/ShellTasks/Xcodeproj.swift Outdated
@maxchuquimia

Copy link
Copy Markdown
Owner

Okay, I had a play with it in Xcode and looked at the diffs - seems it's almost there!

A couple of things still to do:

  • SomeApp's Podfile needs to be updated to reflect the target name change
  • sample-package-description.json has two absolute paths to a directory on your computer, causing some test failures hehe
  • Even after adding a temporary fix for the above, many tests are failing for me... I spent quite some time stepping through testSomeAppSPM and it seems defaultXCGrapherPluginLocation() no longer produces a path to a file - in fact, XCGrapherModuleImportPlugin.framework suddenly doesn't seem to exist at all! 🤔

On this last point, I compared what happens after manually deleting XCGrapher's derived data while the project is open in Xcode and then running File > Swift Packages > Resolve Package Versions. For master it seems that doing this correctly (?) places a XCGrapherModuleImportPlugin.framework inside $DERIVED_DATA/Build/Products/PackageFrameworks but for this branch it does not... the only thing I can think that's changed is the addition of resource processing... have you got any ideas why this is now happening?

@rogerluan

Copy link
Copy Markdown
Collaborator Author

Podfile

Nice catch!

sample-package-description.json

😳 what a newb mistake! haha but after thinking it through, there's no way to really mock a real path, right? What do you recommend changing it to?

the addition of resource processing

Hum, that's a good hunch. That may be the case, yeah. Idk how to reproduce that issue you just mentioned, though, but we can move the .json sample file to a simple string in the codebase - so no resources. What do you think @maxchuquimia ? Could you test if that change makes it work for you?

@maxchuquimia

Copy link
Copy Markdown
Owner

Oh man I went down a massive rabbit hole of going through commits to see which one introduced the issue -turns out the .framework wasn't there just because I had the xcgrapher scheme selected instead of the xcgrapher-Package scheme when hitting Command+U 😂 🤦 . I'm guessing this came around as part of 2557e1a - can we add a point to the "Development Notes" section of the ReadMe about needing this scheme selected?

Regarding the json - maybe put a token like $TEST_PATH in it instead of the URL and perform substitution of TEST_PATH to what you expect on the second line of PackageDescriptionTests.testInitialization()?

Now that I can actually run tests, there seems to be a common failure: in XCGrapherWorkspaceTests, testSomeAppSPM, testSomeAppPodsAndSPM, testSomeAppAppleAndSPM and testSomeAppAppleAndSPMAndPods are all failing with the same error:
image

Is there some additional setup I am maybe missing before it should work?

Also, I have noticed another issue: it seems --help no longer prints help... I think it might have something to do with the validate() arguments function having an else? Maybe we need to have die throw instead of exit or something...

@rogerluan

Copy link
Copy Markdown
Collaborator Author

Hey @maxchuquimia sorry for the radio silence, I've been busy lately (but also on vacation 😂 ) so I've been MIA and will still be for some time. Feel free to address necessary changes if/when you can, and I'll review/come back to this when I can 🙏

@maxchuquimia

Copy link
Copy Markdown
Owner

No problem @rogerluan, millions of things going on here too so I haven't done any coding over the weekends at all, think it's gonna be a while until I get a good chunk of time to do anything anyway hahah

@mthole

mthole commented May 17, 2022

Copy link
Copy Markdown

@rogerluan @maxchuquimia Any chance of wrapping this up sometime soon? Looks very useful!

@maxchuquimia

Copy link
Copy Markdown
Owner

Well, I was able to make a bit of progress in resolving the outstanding issues... see progress.patch.txt (resolves test paths and help not being printed)

I can't seem to get the same result as the sample png in this PR's description - mine simply ignores the local dependency.. so not sure that this is ready to be merged yet

1 similar comment
@maxchuquimia

This comment was marked as duplicate.

@rogerluan

Copy link
Copy Markdown
Collaborator Author

I don't remember what state I left this PR in @maxchuquimia 😅 I'd have to revisit it...

@eldaroid

Copy link
Copy Markdown

Useful feature, I wish I could already use it 👍

@rogerluan

Copy link
Copy Markdown
Collaborator Author

Unfortunately work has been halted here @eldaroid 🙈 I eventually used this branch to successfully generate charts for me, but ended up not following up to get this PR through completion 🫠

Feel free to checkout my branch and run it locally

@maxchuquimia

Copy link
Copy Markdown
Owner

Hey @eldaroid if this branch works for you let me know and I'll consider manual testing and merging without the unit tests completed as I probably won't be focused on updating this project anytime soon 🫠

@jonnyijapan

Copy link
Copy Markdown

Does this branch fix the "[main] The folder “checkouts” doesn’t exist." error?

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.

Support Local Swift Packages

5 participants