Skip to content
This repository has been archived by the owner on Apr 15, 2024. It is now read-only.

Support for Swift 5.8 and Xcode 14.3 #127

Open
6 tasks
vinivendra opened this issue Apr 4, 2023 · 33 comments
Open
6 tasks

Support for Swift 5.8 and Xcode 14.3 #127

vinivendra opened this issue Apr 4, 2023 · 33 comments
Labels
enhancement New feature or request

Comments

@vinivendra
Copy link
Owner

Gryphon should support the newly released Swift 5.8 and Xcode 14.3, as well as the latest macOS (Ventura).

It currently only supports Swift 5.2 to 5.5, which are outdated, meaning users with recent versions of Xcode and Swift are unable to install and run it. This has likely caused several of the issues that were filed in the project: #118, #121, #122, #123, #124, #126.

As I am no longer able to maintain Gryphon, I am looking for new maintainers to help with this problem. This issue is probably going to require considerable effort (based on my previous experiences with supporting new versions of Swift) so anyone willing to contribute even a small part in the development or discussion is welcome.

As a basic roadmap to get the discussion going (and for anyone who doesn’t know where to start):

  • Install Xcode 14.3 (which comes with Swift 5.8) on macOS Ventura, or install Swift 5.8 (standalone) on Linux.
  • Clone this repository and checkout the development branch.
  • Try to compile the project, which will probably raise several warnings and errors.
  • Fix any of the warnings and errors, and send it as a pull request to this project.
  • Repeat until the project compiles and runs successfully.
  • Run the test suite, which will probably raise more errors. Fix any of the errors and send it as a pull request to this project. Repeat until all tests run successfully.

If anybody has any questions during this process, feel free to post them here and I’ll be happy to help when I can. No question is too simple to be asked.

@laosb
Copy link

laosb commented Apr 9, 2023

Looks like we have to bump our macOS version requirement up to macOS 12.0, since SourceKitten bumped theirs in 0.33.0.

@laosb
Copy link

laosb commented Apr 9, 2023

...or maybe we need to have different Package.swift for different swift tools version. The current one doesn't even support macOS(.v12) since it's marked as // swift-tools-version:5.2.

I don't see reason to do so though, since developer utilizing older versions can always use old versions of Gryphon. Any idea on how this should work, @vinivendra?

@vinivendra
Copy link
Owner Author

I agree, I think we should focus now on Swift and Xcode support for only the most recent versions. I believe that should cover most devs, and supporting older versions can sometimes be almost double the effort (in my experience).

This might involve upgrading the // swift-tools-version to 5.8 (since we expect users to have Swift 5.8 anyway) and upgrading any dependencies to their latest versions (SourceKitten etc).

I don’t know the details of macOS version support, but Xcode 14.3 requires macOS 13.0 or later, so we can also use that as a basis.

At one point we’ll also have to look into what Linux version support (the Docker image uses it). No need to focus on it right out of the gate (we can do it after macOS is done), let’s just not forget about it 😉

@laosb
Copy link

laosb commented Apr 10, 2023

OK I'm splitting a branch (https://github.com/laosb/Gryphon/tree/swift-5.8) to actually start to fix things.

I bumped up to SourceKitten 0.34.1 & macOS requirement to v12, and basically did #119.

Now I'm hitting "cannot find" errors in SwiftSyntax_PrintableAsTree.swift, specifically Syntax.getName. From what I can see it's basically used for printing the AST in compiler output, but I failed to find what nodes are added / removed in what specific versions of SwiftSyntax. Is there any specific reason for including and maintaining such a list, instead of just using String(describing: syntaxNodeType), apart from aesthetic reasons?

@vinivendra
Copy link
Owner Author

Oh, it definitely wasn't for aesthetic reasons 😅

Looking at the code now, I think it's because SwiftSyntax uses some internal struct magic to differentiate between its types. Instead of Syntax being a normal Swift class and the other types (in the list you saw) being its subclasses, Syntax is a struct that can be "casted" into its subtypes (which are also structs) using internal SwiftSyntax methods like Syntax.as(T.Type). This is also why we have to write self.is(TokenSyntax.self) instead of self is TokenSyntax, because we are calling the SwiftSyntax Syntax.is(T.Type) method that checks if the cast can be done.

I probably tried doing normal Swift things like calling type(of: self) and it didn't work because of that, so I enumerated the types by hand in order to get decent debug logs. So if you can figure out a way to get a similar result (or a "good enough" result) with String(describing:) or something else like that, go for it.

@hamdivazim
Copy link

Hi @vinivendra,

I'm quite interested in this project as I continue to work in Swift and Kotlin (I'm good at Swift but not as experienced in Kotlin). I see a lot of potential in this project and I suppose I have enough knowledge to contribute (be it small yet not
completely useless). I will probably only be free for 4-5 hours a week to work on this as I have many other commitments I need to work on too.

Thanks!

@vinivendra
Copy link
Owner Author

Hey @hamdivazim, welcome to the project! Any help you can give us will be great.

If you’re looking for somewhere to start from, try cloning @laosb’s swift-5.8 branch, looking into the compilation errors there, and solving whatever you can.

If you’d like to do something else, or if you need any help with that, just let me know.

@laosb
Copy link

laosb commented Apr 15, 2023

Oh, it definitely wasn't for aesthetic reasons 😅

Looking at the code now, I think it's because SwiftSyntax uses some internal struct magic to differentiate between its types. Instead of Syntax being a normal Swift class and the other types (in the list you saw) being its subclasses, Syntax is a struct that can be "casted" into its subtypes (which are also structs) using internal SwiftSyntax methods like Syntax.as(T.Type). This is also why we have to write self.is(TokenSyntax.self) instead of self is TokenSyntax, because we are calling the SwiftSyntax Syntax.is(T.Type) method that checks if the cast can be done.

I probably tried doing normal Swift things like calling type(of: self) and it didn't work because of that, so I enumerated the types by hand in order to get decent debug logs. So if you can figure out a way to get a similar result (or a "good enough" result) with String(describing:) or something else like that, go for it.

OK, I'm just commenting out the list and replace with String(describing:) for now, let's see if that actually works - worst case we can still maintain the list!

I just found out SwiftSyntax has its DocC documentation pushed on Swift Package Index, and I find it quite useful and more stable than the code completion in Xcode: https://swiftpackageindex.com/apple/swift-syntax/508.0.0/documentation/swiftsyntax/

... as a little reminder for myself and anyone else contributing!

@laosb
Copy link

laosb commented Apr 15, 2023

Interestingly without too much effort, my swift-5.8 branch is actually building successfully! ✨

I will start to check tests.

@laosb laosb mentioned this issue Apr 15, 2023
5 tasks
@laosb
Copy link

laosb commented Apr 15, 2023

Well, with no changes to anything than fixing compile errors:
image
That's way better than I thought.

Only unit tests though. I'll need to setup environment for other tests.

@laosb
Copy link

laosb commented Apr 15, 2023

For bootstrapping tests, I see the shell script explicitly requires Swift 5.5. Is there anything I should do to run them?

@hamdivazim
Copy link

Hey @hamdivazim, welcome to the project! Any help you can give us will be great.

If you’re looking for somewhere to start from, try cloning @laosb’s swift-5.8 branch, looking into the compilation errors there, and solving whatever you can.

If you’d like to do something else, or if you need any help with that, just let me know.

Awesome, thanks! I've forked their repository and will get started on it tomorrow.

@hamdivazim
Copy link

@laosb are there any specific tests you want me to work on?

@vinivendra
Copy link
Owner Author

If I recall, the bootstrapping tests were about to be removed from the project. They were initially meant to translate Gryphon’s own codebase and check if the translation was correct, which was a great way to find bugs, but I don’t think that’s possible anymore.

When they were created initially, Gryphon still used Swift’s AST Dump to turn Swift code into an AST, and that just required a command line invocation. This meant that when Gryphon’s code was translated to Kotlin, the Kotlin version could also use that command line invocation and everything worked correctly.

A few versions later, we traded the AST Dump for SwiftSyntax and SourceKit, which obviously do not have a Kotlin versions, so translating the code itself was impossible. The bootstrapping test then became a translation of an old version of Gryphon’s code, the last version that still used AST Dump (and therefore could still be translated into Kotlin). However, that old code only works with an AST Dump from the Swift 5.5 compiler, which is why that check was there.

If we want to keep the bootstrapping test around, we’d have to update the old AST Dump version to work with the Swift 5.8 compiler, which will be a lot of work (the AST Dumps change a lot between Swift versions). That doesn’t seem worth the effort to me.

The only other alternative I can think of is removing this test, and then relying on the other tests (and any bug reports) to find and fix bugs in the future.

(We could also consider setting up new regression tests using any open source code that uses Gryphon from volunteers in the community, but that seems like a more long-term plan to me).

All that to say, I wouldn’t worry about the bootstrapping tests for now. We should prioritize making sure all the unit, integration, acceptance, and Xcode tests are working, and then consider what to do about bootstrapping.

@laosb
Copy link

laosb commented Apr 16, 2023

@vinivendra Got it.

Currently I can confirm that in my branch, unit and acceptance tests are all passing. I can't do Xcode tests for now as my disk is full to a point that I won't risk installing Android Studio on. @hamdivazim can you help me do the Xcode tests?

@hamdivazim
Copy link

I've already got Android Studio so sure I'll get started on that

@hamdivazim
Copy link

A little side note: I noticed that in the install script, it calls .build/debug/Gryphon but seemed to be failing saying the directory was not found for me. I made the g in Gryphon lowercase and it installed successfully.

@hamdivazim
Copy link

@laosb what exactly would you like me to work on?

@vinivendra
Copy link
Owner Author

A little side note: I noticed that in the install script, it calls .build/debug/Gryphon but seemed to be failing saying the directory was not found for me. I made the g in Gryphon lowercase and it installed successfully.

That’s good to know, it might have changed recently.

@laosb what exactly would you like me to work on?

I think the next step is to get the Xcode tests to work successfully. Could you try to run them and see if everything’s ok?

@hamdivazim
Copy link

hamdivazim commented Apr 16, 2023

I think the next step is to get the Xcode tests to work successfully. Could you try to run them and see if everything’s ok?

Whenever I open and try to run this Xcode app (Test files/XcodeTests/iOS/GryphoniOSTest.backup.xcodeproj) it fails saying there is no scheme to build. Am I missing something or is this an error?

@vinivendra
Copy link
Owner Author

Did you run the Xcode tests before opening it? I think that’s necessary, but I’m not sure anymore. You can run the tests by calling the tests script with -x (for just the Xcode tests).

If that doesn’t work, we might have to recreate that project. I think it’s a very simple, empty iOS app. The test sets up the project to use Gryphon, then tries to compile it to make sure everything’s ok.

@hamdivazim
Copy link

No I didn't, I'll try that.

@Aurelio2005
Copy link

Hi guys, tried to install today and received the error:
"error: the library 'GryphonLib' requires macos 10.13, but depends on the product 'SourceKittenFramework' which requires macos 12.0; consider changing the library 'GryphonLib' to require macos 12.0 or later, or the product 'SourceKittenFramework' to require macos 10.13 or earlier."

I'm a bit of a novice so I doubt I can help much (let me know if otherwise).

@DanKamilov
Copy link

Hi guys, tried to install today and received the error: "error: the library 'GryphonLib' requires macos 10.13, but depends on the product 'SourceKittenFramework' which requires macos 12.0; consider changing the library 'GryphonLib' to require macos 12.0 or later, or the product 'SourceKittenFramework' to require macos 10.13 or earlier."

I'm a bit of a novice so I doubt I can help much (let me know if otherwise).

same problem

@DanKamilov
Copy link

Did you run the Xcode tests before opening it? I think that’s necessary, but I’m not sure anymore. You can run the tests by calling the tests script with -x (for just the Xcode tests).

If that doesn’t work, we might have to recreate that project. I think it’s a very simple, empty iOS app. The test sets up the project to use Gryphon, then tries to compile it to make sure everything’s ok.

When you plan to support 5.8?

@maitrungduc1410
Copy link

really hope the team can fix this issue soon

@vinivendra
Copy link
Owner Author

Hey folks, just a reminder that there's no team involved, only contributors from the community that show up to help occasionally. If this works out, it'll be a group effort from everyone who volunteers.

I just merged #119, which gets us a bit further ahead by fixing the build problems on Swift 5.7. I also merged @laosb's branch fixing the compilation problems with Swift 5.8 (great work by the way!) so the development branch is compiling correctly now.

The next step is making the unit tests pass by fixing the issues that appeared in Swift 5.8. Anyone who wants to help with that is welcome to open a PR to the development branch. Please remember that individual PRs don't have to fix everything at once, every little bit helps.

@thackerdynasty
Copy link

Swift 5.9 fails with gryphon init. Hopefully you guys will fix this as well.

@thackerdynasty
Copy link

...so I ran the Xcode tests on 5.9 and there are code signing errors. These are the same errors that appeared with gryphon init, except it cannot sign the gryphon test app instead of my app. I think that the project is using a deprecated code signing command.

@vinivendra
Copy link
Owner Author

That’s weird, because as far as I know the project isn’t using any code signing whatsoever (since we removed the Xcode project from the repo). I haven’t downloaded Xcode 15 or Swift 5.9 yet, so I’m not sure. Can you give us more details about the issue?

@thackerdynasty
Copy link

So the main thing that I saw it prints out is:

The following build commands failed:
CodeSign /Users/myname/Documents/Gryphon/Test\ files/XcodeTests/iOS/build/Release-iphonesimulator/GryphoniOSTest.app (in target 'GryphoniOSTest' from project 'GryphoniOSTest')
(1 failure)

I'm not going to show all the logs because it shows my name too much and it is very long. As you can see, it very much looks like a code signing issue, but it also says build failed, so it also could be that xcodebuild is doing this.

@thackerdynasty
Copy link

also the unit tests worked on 5.9 development branch without any changes.

@thackerdynasty
Copy link

sorry for the excessive comments, but I am also using macOS Sonoma beta, if that helps.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

7 participants