Add support for a prebuilt swift-syntax library for macros #8142
Conversation
Following the pattern used for the early Traits work, added a workspace prebuilts manager to co-ordinate detecting when macros are using swift-syntax, downloading the zip file for the resolved swift-syntax version, and extracting it into the scratch directory. We add the necessary information about that to the workspace state. We then morph the Modules that use it to add build settings to find the necessary modules and libraries.
dschaefer2
requested review from
bnbarham,
MaxDesiatov,
jakepetroules,
francescomikulis,
xedin,
shawnhyam,
bripeticca,
plemarquand and
owenv
as code owners
|
Also, most inspiration comes from binary artifacts. Still signs of the clone in this PR :). Still lots of clean up to do including taking advantage of the caching and checking checksums. And, of course, tests. |
This resulted in some minor changes to the prebuilts manifest to make it easier to build.
Also required renaming of the arm64 arches to aarch64 since that's what they are called on Windows.
The -l argument translates into a .lib file where we are producing .a libraries.
We weren't getting called if there was a Package.resolved file which didn't allow us to refresh things if the user deleted the scratch folder.
Added generating of the available Linux platforms using docker.
Need to find out why but FileManager couldn't detect tar.exe is executable causing the AsyncProcess to fail. As a work around for now, just use the absolute path since it's always in system32.
dschaefer2
changed the title
dschaefer2
changed the title
Had to convert most of the slashes in paths to the opposite.
|
OK, feels like it's done. Awaiting the tests. |
|
BTW, there will be at least one more PR when I get back from holidays in January to complete the BuildPrebuilts script to add in more docker images and platforms as they come available, and of courses to fix any issues we find during testing, and then to enable this feature by default. I'll post to the forums on how to test with it once this PR finishes review. |
|
All tests pass. Let me know if there's anything else @jakepetroules. |
| @@ -48,7 +70,9 @@ public struct ZipArchiver: Archiver, Cancellable { | |||
| } | |||
|
|
|||
| #if os(Windows) | |||
| let process = AsyncProcess(arguments: ["tar.exe", "xf", archivePath.pathString, "-C", destinationPath.pathString]) | |||
| // FileManager lost the ability to detect tar.exe as executable. | |||
There was a problem hiding this comment.
I wonder if that's related to swiftlang/swift-foundation#860
| #if os(Windows) | ||
| try await shell("tar -acf \(zipFile.pathString) \(contentDirs.joined(separator: " "))") | ||
| #else | ||
| try await shell("zip -r \(zipFile.pathString) \(contentDirs.joined(separator: " "))") |
There was a problem hiding this comment.
(non-blocking): Why not just use tar everywhere? I think it's more commonly installed by default than zip is.
There was a problem hiding this comment.
Looks good, Doug! I don't have any further blocking feedback. Thanks for your patience on the review. :)
I had one last thing I'd like to understand a little better though, before we land this: #8142 (comment)
| ), | ||
| prebuiltsURL: URL( | ||
| string: | ||
| "https://github.com/dschaefer2/swift-syntax/releases/download" |
There was a problem hiding this comment.
Any thoughts on how we'd provide the ability to override this with eg. a user's own proxy or local path?
There was a problem hiding this comment.
Something we can think about for testing. Of course we'll put the real URL there once we know what it is. I'll add that to the piece to look at next.
| let manifest = manifests.allDependencyManifests[ | ||
| prebuilt.packageRef.identity | ||
| ], | ||
| let packageVersion = manifest.manifest.version, |
There was a problem hiding this comment.
What is this version 🤔? I might be missing something, but aren't we replacing all swift-syntax dependencies with prebuilts, is version resolution even still happening for it?
There was a problem hiding this comment.
That's the version we resolved swift-syntax to, e.g. 600.0.1. If there isn't a download for that version we leave things like they are and build from source.
…#8142) Following the pattern used for the early Traits work, added a workspace prebuilts manager to co-ordinate detecting when macros are using swift-syntax, downloading the zip file for the resolved swift-syntax version, and extracting it into the scratch directory. We add the necessary information about that to the workspace state. We then morph the Modules that use it to add build settings to find the necessary modules and libraries. --------- Co-authored-by: Doug Schaefer <[email protected]>
Following the pattern used for the early Traits work, added a workspace prebuilts manager to co-ordinate detecting when macros are using swift-syntax, downloading the zip file for the resolved swift-syntax version, and extracting it into the scratch directory. We add the necessary information about that to the workspace state. We then morph the Modules that use it to add build settings to find the necessary modules and libraries.