Implement the `install` command on Linux #9

patrickfreed · 2022-09-07T21:33:07Z

This PR implements the core functionality of the install command on Linux. The main steps are the following:

reach out to the GitHub API to find the latest version if an incomplete one was provided (e.g. 5.5)
Download the toolchain to a temporary file using AsyncHTTPClient
Extract the downloaded archive to ~/.swiftly/toolchains/<toolchain name> using libarchive
Add an entry to config.json

You can try this out (on Linux) by creating a ~/.swiftly directory that looks like the following:

.swiftly/
  - config.json (see the example included in this PR)
  - toolchains/

And then run:

swift run swiftly install 5.6

Here's a screenshot of it in action:

this will need to change, doing it all in memory leads to OOM

this commit also updates stable version lookups to properly search through GitHub's pagined API results.

patrickfreed · 2022-09-07T21:34:57Z

Package.swift

+                "CLibArchive",
+            ]
+        ),
+        .systemLibrary(


I decided to use the libarchive C library rather than any Swift solutions like SWCompression, since all of them needed the entire archive to be present in memory, both compressed and decompressed, and this led to OOM issues with bigger toolchains. I also avoided shelling out to something like tar to avoid having to deal with the possibility that it isn't installed or is missing features (e.g. gzip support).

patrickfreed · 2022-09-07T21:37:15Z

Sources/LinuxPlatform/Linux.swift

+    }
+
+    public var fullName: String {
+        self.platform.fullName


Unfortunately, the download links contain two different formats for each platform, so I needed to add a new entry in config.json for tracking this. e.g. for Ubuntu 20.04, there's both ubuntu2004 and ubuntu20.04 in this download URL:

https://download.swift.org/swift-5.6.3-release/ubuntu2004/swift-5.6.3-RELEASE/swift-5.6.3-RELEASE-ubuntu20.04.tar.gz

yeah that is a pain, not easy one to get around

patrickfreed · 2022-09-07T21:38:18Z

Package.swift

@@ -22,6 +23,7 @@ let package = Package(
                .product(name: "ArgumentParser", package: "swift-argument-parser"),
                .target(name: "SwiftlyCore"),
                .target(name: "LinuxPlatform", condition: .when(platforms: [.linux])),
+                .product(name: "SwiftToolsSupport-auto", package: "swift-tools-support-core"),


I started to implement a basic loading bar type thing, but then I decided to check out what SwiftPM does, and happily they have a separate library which contains this stuff.

patrickfreed · 2022-09-07T21:38:53Z

Sources/Swiftly/Install.swift

+                reportProgress: { progress in
+                    let now = Date()
+
+                    guard lastUpdate.distance(to: now) > 0.25 || progress.receivedBytes == progress.totalBytes else {


To avoid spamming the terminal too hard, only print updates every 250ms.

patrickfreed · 2022-09-07T21:39:57Z

Sources/Swiftly/Install.swift

    var version: String

+    @Option(help: ArgumentHelp(


The GitHub API has extremely low rate limits for unauthenticated users, so I added an option to specify an authorization token to side step those. Once we move to official Apple APIs, this issue should go away.

patrickfreed · 2022-09-07T21:43:11Z

Sources/SwiftlyCore/HTTP.swift

+    /// The `fetch` closure is used to retrieve the next page of results. It accepts the page number as its argument.
+    /// The `filterMap` closure maps the input GitHub tag to an output. If it returns nil, it will not be included
+    /// in the returned array.
+    private static func mapGithubTags<T>(


I use separate GitHub API endpoints to fetch releases vs snapshots even though the /tags one would suffice because there are a lot fewer releases than tags (42 releases vs ~2k tags), and this helps avoid running up against the API rate limits. It's also a decent bit faster for the "happy path".

This convoluted method helps avoid duplicating logic between the two of them.

Does GitHub API response parsing belong in a file called HTTP.swift?

Moved the GitHub specific code to HTTPClient+GitHubAPI.swift (also renamed HTTP.swift to HTTPClient.swift). I kept the methods that used the API response parsing, since they should be stable even after we migrate to Apple APIs.

adam-fowler

I haven't had a chance to compile and run this yet, but I have left some comments for you
I also think we should be attempting to add tests for everything where we can.

Sources/SwiftlyCore/ToolchainVerison.swift

adam-fowler · 2022-09-15T12:32:28Z

Sources/LinuxPlatform/Linux.swift

+    }
+
+    public var fullName: String {
+        self.platform.fullName


yeah that is a pain, not easy one to get around

Sources/LinuxPlatform/Linux.swift

adam-fowler · 2022-09-15T12:46:13Z

Sources/LinuxPlatform/Linux.swift

@@ -39,10 +70,16 @@ public struct Linux: Platform {

    public func currentToolchain() throws -> ToolchainVersion? { nil }

+    public func getTempFilePath() -> URL {
+        let path = URL(fileURLWithPath: "/tmp/swiftly-\(UUID())")


you cannot assume the temp folder is /tmp. I think you can use FileManager.default.temporaryDirectory

Updated. I also changed this to just return the path and delegated actually creating the file to Install.swift.

adam-fowler · 2022-09-15T12:48:50Z

Sources/SwiftlyCore/HTTP.swift

+    /// The `fetch` closure is used to retrieve the next page of results. It accepts the page number as its argument.
+    /// The `filterMap` closure maps the input GitHub tag to an output. If it returns nil, it will not be included
+    /// in the returned array.
+    private static func mapGithubTags<T>(


Does GitHub API response parsing belong in a file called HTTP.swift?

adam-fowler · 2022-09-15T12:49:38Z

Sources/SwiftlyCore/HTTP.swift

+        public let url: String
+    }
+
+    public static func downloadToolchain(


This could just be called download I guess given it could be used on any file

This was originally called that, but I restricted it (via name) for downloading toolchains specifically in order for my 404 detection hack based on Content-Type to make more sense. If we end up needing to download files other than toolchains then I'll name this something more generic and deal with 404s in some other place.

adam-fowler · 2022-09-15T12:50:30Z

Sources/SwiftlyCore/HTTP.swift

    }
 }

-/// Model of a GitHub REST API release object.
-public struct GitHubRelease: Decodable {


As mentioned above it is probably a good idea to separate all the GH code into a separate file

Sources/Swiftly/Install.swift

patrickfreed

Added tests for the installation behavior, LMK if the coverage looks good.

Sources/LinuxPlatform/Linux.swift

patrickfreed · 2022-09-15T22:31:55Z

Sources/LinuxPlatform/Linux.swift

@@ -39,10 +70,16 @@ public struct Linux: Platform {

    public func currentToolchain() throws -> ToolchainVersion? { nil }

+    public func getTempFilePath() -> URL {
+        let path = URL(fileURLWithPath: "/tmp/swiftly-\(UUID())")


Updated. I also changed this to just return the path and delegated actually creating the file to Install.swift.

Sources/Swiftly/Install.swift

patrickfreed · 2022-09-15T23:06:55Z

Sources/SwiftlyCore/HTTP.swift

+    /// The `fetch` closure is used to retrieve the next page of results. It accepts the page number as its argument.
+    /// The `filterMap` closure maps the input GitHub tag to an output. If it returns nil, it will not be included
+    /// in the returned array.
+    private static func mapGithubTags<T>(


Moved the GitHub specific code to HTTPClient+GitHubAPI.swift (also renamed HTTP.swift to HTTPClient.swift). I kept the methods that used the API response parsing, since they should be stable even after we migrate to Apple APIs.

patrickfreed · 2022-09-15T23:08:50Z

Sources/SwiftlyCore/HTTP.swift

+        public let url: String
+    }
+
+    public static func downloadToolchain(


This was originally called that, but I restricted it (via name) for downloading toolchains specifically in order for my 404 detection hack based on Content-Type to make more sense. If we end up needing to download files other than toolchains then I'll name this something more generic and deal with 404s in some other place.

patrickfreed · 2022-09-15T23:08:54Z

Sources/SwiftlyCore/HTTP.swift

    }
 }

-/// Model of a GitHub REST API release object.
-public struct GitHubRelease: Decodable {


Sources/SwiftlyCore/ToolchainVerison.swift

patrickfreed · 2022-10-08T15:01:59Z

.github/workflows/tests.yml

@@ -0,0 +1,37 @@
+name: tests


This is just a barebones config used to verify the tests pass as written. It will be fleshed out more to cover all the required platforms (#16) and to use the latest release of swiftly to install Swift (#17) instead of manually doing it.

adam-fowler · 2022-10-14T13:25:30Z

I tried to run this on my linux machine (compiling on macOS with vanilla 5.7 needs Ventura which I don't have). There seems to be a lot of places where it assumes a config file exists. If the file doesn't exist everything dies. Also it is loaded repeatedly, maybe it should be cached.

Sources/SwiftlyCore/HTTPClient+GitHubAPI.swift

adam-fowler · 2022-10-14T13:14:55Z

Sources/Swiftly/Install.swift

    }

    internal static func execute(version: ToolchainVersion) async throws {
-        let file = try await Swiftly.currentPlatform.download(version: version)
-        try Swiftly.currentPlatform.install(from: file, version: version)
+        guard try !Config.load().installedToolchains.contains(version) else {


Config.load() can throw an error if the file does not exist. You need to deal with the situation where it doesn't exist. Currently it just quits

In the current design, quitting with an error is the desired behavior when no config is present, since swiftly won't have any idea of what platform its running on. The bootstrapping script which downloads and installs swiftly is in charge of creating the configuration file and populating the platform information.

That said, the error this currently fails with is quite ugly, so I've updated it to look something more like the following (this is when the config doesn't exist):

$ swift run swiftly list Error: Could not load swiftly's configuration file due to error: "Error Domain=NSPOSIXErrorDomain Code=2 "No such file or directory"". To use swiftly, modify the configuration file to fix the issue or perform a clean installation.

adam-fowler · 2022-10-14T13:20:03Z

Sources/LinuxPlatform/Linux.swift

    public static let currentPlatform: any Platform = {
        do {
            let config = try Config.load()
-            return Linux(name: config.platform.name, namePretty: config.platform.namePretty)
+            return Linux(platform: config.platform)
        } catch {
            fatalError("error loading config: \(error)")


What happens if there is no config because this is the first time you ran.

It's supposed to exit with an error. See my other comment for more info.

patrickfreed

There seems to be a lot of places where it assumes a config file exists. If the file doesn't exist everything dies.

Yep, this is intended behavior. See my threaded comment for more info. If you want to get swiftly running just to test it out, you can run swift test (with a few env variables set) or create ~/.swiftly/config.json using the example one I committed here (that one is for Ubuntu 20.04 but similar ones can work for other Linux variants).

Also it is loaded repeatedly, maybe it should be cached.

Yeah I agree. I actually already have a TODO comment about this in Config.swift. For the purposes of this PR, I think it's okay to not implement the caching behavior (seeing as this is already a big change), and I filed #18 to cover the future work for that.

Sources/SwiftlyCore/HTTPClient+GitHubAPI.swift

patrickfreed · 2022-10-18T22:53:35Z

Sources/Swiftly/Install.swift

    }

    internal static func execute(version: ToolchainVersion) async throws {
-        let file = try await Swiftly.currentPlatform.download(version: version)
-        try Swiftly.currentPlatform.install(from: file, version: version)
+        guard try !Config.load().installedToolchains.contains(version) else {


In the current design, quitting with an error is the desired behavior when no config is present, since swiftly won't have any idea of what platform its running on. The bootstrapping script which downloads and installs swiftly is in charge of creating the configuration file and populating the platform information.

That said, the error this currently fails with is quite ugly, so I've updated it to look something more like the following (this is when the config doesn't exist):

$ swift run swiftly list Error: Could not load swiftly's configuration file due to error: "Error Domain=NSPOSIXErrorDomain Code=2 "No such file or directory"". To use swiftly, modify the configuration file to fix the issue or perform a clean installation.

patrickfreed · 2022-10-18T22:53:54Z

Sources/LinuxPlatform/Linux.swift

    public static let currentPlatform: any Platform = {
        do {
            let config = try Config.load()
-            return Linux(name: config.platform.name, namePretty: config.platform.namePretty)
+            return Linux(platform: config.platform)
        } catch {
            fatalError("error loading config: \(error)")


It's supposed to exit with an error. See my other comment for more info.

ubuntu-base-config.json

adam-fowler · 2022-10-14T13:45:21Z

Tests/SwiftlyTests/InstallTests.swift

+import XCTest
+
+final class InstallTests: SwiftlyTests {
+    func testInstallLatest() async throws {


Can we get comments detailing what each test is testing for

adam-fowler · 2022-10-14T13:48:23Z

Sources/SwiftlyCore/HTTPClient.swift

+        let expectedBytes = response.headers.first(name: "content-length").flatMap(Int.init)
+
+        var receivedBytes = 0
+        for try await buffer in response.body {


Nice use of the new async/await APIs

Sources/SwiftlyCore/HTTPClient+GitHubAPI.swift

patrickfreed added 19 commits August 2, 2022 09:51

wip

7e0ef2b

finish download, hook in progress bar from SPM

6705837

wip use swift for decompression

08f0743

this will need to change, doing it all in memory leads to OOM

use libarchive for extraction

18b9534

wip snapshot, fix up progress bar

ca25db9

support installing snapshots, fix loading bar

1c3797f

add template config.json

248fcd3

check if toolchain installed before proceeding

c8417e9

support looking up snapshots from partial selectors

a42fe0e

this commit also updates stable version lookups to properly search through GitHub's pagined API results.

support passing GitHub authorization token to install

8f90f11

print before lookup

3178661

conform StableRelease to CustomStringConvertible

4c12011

more flexible release parsing

e5a67b6

conform Snapshot to CustomStringConvertible

5209022

check to see if tarball exists before continuing

c4b058a

clean up extraction code

89bed85

make the correct type of request

f8199f6

various cleanup

1f4f073

flush file to disk in download

7a4b370

patrickfreed commented Sep 7, 2022

View reviewed changes

patrickfreed requested a review from adam-fowler September 7, 2022 21:56

kmahar self-requested a review September 7, 2022 22:45

adam-fowler reviewed Sep 15, 2022

View reviewed changes

patrickfreed added 7 commits September 15, 2022 18:32

use FileManager.default.temporaryDirectory

df774b1

cleanup temp files after install finishes

5a1789f

rename HTTP.swift -> HTTPClient.swift

697a6d2

move GitHub API specific logic to separate file

fffb759

fullName -> nameFull

0a70a7c

add install tests

c151437

validate snapshot installations by looking up commit

db5c70b

patrickfreed added 4 commits October 8, 2022 10:32

add documentation to validateInstalledToolchains

c56eef2

add basic GH actions configuration

f8547d4

install libarchive in GitHub action

acb6de6

correctly set environment variables in actions

3fac1c5

patrickfreed mentioned this pull request Oct 8, 2022

Test against all supported Linux platforms in CI #16

Closed

patrickfreed commented Oct 8, 2022

View reviewed changes

patrickfreed requested a review from adam-fowler October 8, 2022 15:03

adam-fowler requested changes Oct 14, 2022

View reviewed changes

patrickfreed added 2 commits October 18, 2022 18:14

consistent GitHub capitalization

0460324

exit cleanly when config file isn't present

b62d3b6

patrickfreed commented Oct 18, 2022

View reviewed changes

patrickfreed added 3 commits October 18, 2022 21:45

update base config to use nameFull

fe22502

add tests that duplicates cannot be installed

2cb1366

Apple -> swift.org

715d996

adam-fowler approved these changes Oct 27, 2022

View reviewed changes

patrickfreed added 2 commits October 27, 2022 12:52

add comments to each test case

0ae982b

fix formatting

a99ed39

patrickfreed merged commit d070b1e into main Oct 30, 2022

patrickfreed mentioned this pull request Oct 30, 2022

Set up automated testing #7

Closed

patrickfreed deleted the install branch March 28, 2023 03:52

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement the `install` command on Linux #9

Implement the `install` command on Linux #9

patrickfreed commented Sep 7, 2022 •

edited

patrickfreed Sep 7, 2022

patrickfreed Sep 7, 2022

adam-fowler Sep 15, 2022

patrickfreed Sep 7, 2022

patrickfreed Sep 7, 2022

patrickfreed Sep 7, 2022

patrickfreed Sep 7, 2022

adam-fowler Sep 15, 2022

patrickfreed Sep 15, 2022

adam-fowler left a comment

adam-fowler Sep 15, 2022

adam-fowler Sep 15, 2022

patrickfreed Sep 15, 2022

adam-fowler Sep 15, 2022

adam-fowler Sep 15, 2022

patrickfreed Sep 15, 2022

adam-fowler Sep 15, 2022

patrickfreed Sep 15, 2022

patrickfreed left a comment

patrickfreed Sep 15, 2022

patrickfreed Sep 15, 2022

patrickfreed Sep 15, 2022

patrickfreed Sep 15, 2022

patrickfreed Oct 8, 2022

adam-fowler commented Oct 14, 2022

adam-fowler Oct 14, 2022

patrickfreed Oct 18, 2022

adam-fowler Oct 14, 2022

patrickfreed Oct 18, 2022

patrickfreed left a comment

patrickfreed Oct 18, 2022

patrickfreed Oct 18, 2022

adam-fowler Oct 14, 2022

adam-fowler Oct 14, 2022

Implement the install command on Linux #9

Implement the install command on Linux #9

Conversation

patrickfreed commented Sep 7, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

adam-fowler left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

patrickfreed left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

adam-fowler commented Oct 14, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

patrickfreed left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Implement the `install` command on Linux #9

Implement the `install` command on Linux #9

patrickfreed commented Sep 7, 2022 •

edited