Open vinivendra opened 1 year ago
Looks like we have to bump our macOS version requirement up to macOS 12.0, since SourceKitten bumped theirs in 0.33.0.
...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?
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 😉
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?
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.
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!
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.
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 likeSyntax.as(T.Type)
. This is also why we have to writeself.is(TokenSyntax.self)
instead ofself is TokenSyntax
, because we are calling the SwiftSyntaxSyntax.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) withString(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!
Interestingly without too much effort, my swift-5.8
branch is actually building successfully! ✨
I will start to check tests.
Well, with no changes to anything than fixing compile errors:
That's way better than I thought.
Only unit tests though. I'll need to setup environment for other tests.
For bootstrapping tests, I see the shell script explicitly requires Swift 5.5. Is there anything I should do to run them?
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.
@laosb are there any specific tests you want me to work on?
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.
@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?
I've already got Android Studio so sure I'll get started on that
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.
@laosb what exactly would you like me to work on?
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?
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?
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.
No I didn't, I'll try that.
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).
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
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?
really hope the team can fix this issue soon
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.
Swift 5.9 fails with gryphon init. Hopefully you guys will fix this as well.
...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.
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?
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.
also the unit tests worked on 5.9 development branch without any changes.
sorry for the excessive comments, but I am also using macOS Sonoma beta, if that helps.
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):
development
branch.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.