Merge installation logic into swiftly as an init subcommand #127

cmcgee1024 · 2024-06-19T15:16:28Z

Instead of relying on a separate shell script with certain issues swiftly can perform its own user initialization with many of the features of the script, including prompts, flags, and options. It also prepares swiftly for possible delivery through system package managers.

Provide a design for the new installation system in the DESIGN.md.

Add an implementation, tests and documentation of an init subcommand that is capable of finishing the installation of swiftly (after the download step) into the user's environment, including swiftly home, toolchains directory, the swiftly binary (if it's user downloaded), and symbolic links.

Contribute documentation describing how to install swiftly and toolchains automatically in the context of a build/CI system.

Instead of relying on a separate shell script with certain issues swiftly can perform its own user initialization with many of the features of the script, including prompts, flags, and options. It also prepares swiftly for possible delivery through system package managers. Provide a design for the new installation system in the DESIGN.md. Change SwiftlyVersion to make it more conformant to the semantic version specification (https://semver.org) with respect to pre releases. Add test cases that cover typical scenarios. Update the swiftly version to a "dev" pre-release.

adam-fowler · 2024-06-25T15:10:18Z

DESIGN.md

- Amazon Linux 2
+We'll need an initialization mode which, detects information about the OS and distribution in the case of Linux. The initialization mode is also responsible for setting up the directory structure for the toolchains (if necessary), setting up the shell environment for the user, and determining any operating system level dependencies that are required, but missing. Swiftly can perform these tasks itself with the capabilities provided by the Swift language and libraries, such as rich argument parsing, and launching system processes provided that the binary is somehow delivered to the user.
+
+The delivery of the binary onto the user's system can be accomplished in a number of ways, such as direct download from a trusted website with some guidance for the user to pick the right one for their OS and architecture. There might also be a copy/paste of a simple shell command-line for some more automation. Swiftly might be provided as a system-level package (e.g. homebrew, apt, yum, macOS pkg, etc). In some cases delivery might come from manual compilation from this git repository and the "swift build" command, useful for swiftly development and testing.


Delivery of the binary is where this falls down to some degree (at least on Linux). Previously this was all done automatically by the script. macOS has pkg which avoids this issue, but unless we are using some package management system it will up to the user to copy the binary into a folder in their path which isn't an ideal situation. Or are you expecting swiftly-init to do this for the user?

All of the path/environment management, creating the swiftly configuration, detection of missing required system packages, and most of what the shell script does today would be incorporated into swiftly's new init subcommand in this design. It's up to the delivery mechanisms to help the user to get to that entry point. I think that with some of the conventions described here those mechanisms have some mechanisms to provide optimal user experiences. For example, I think that swift.org can provide a copy-able command-line that would be equivalent to what is there today, but a little more complex.

I think it would be good to breakdown the assumed different responsibilities of the different delivery methods to ensure that we're covering all bases and don't have any missing features or regressions in behaviours. This would help us hash out what needs to be done for different platforms, what needs to be be done for security etc

DESIGN.md

Sources/SwiftlyCore/SwiftlyCore.swift

DESIGN.md

…r entirely Describe in more detail the anticipated delivery methods

cmcgee1024 · 2024-06-28T14:31:59Z

@adam-fowler @patrickfreed how does this new revision of the design look? I've enumerated the expected delivery methods, simplified things, and fixed up some of the wording.

adam-fowler

I think I'm pretty happy with this

adam-fowler · 2024-06-28T14:51:26Z

DESIGN.md

- CentOS 7
- CentOS 8
- Amazon Linux 2
+Initialization also covers creating symbolic links for various toolchain commands (e.g. swift, swiftc, clang, clang++, etc.) so that swiftly can configure the targets. These need to be installed into a user controlled directory so that swiftly can operate on the without super user permissions. Also, the swiftly binary itself is copied into this location if it is not run from a system location, managed by a system package manager.


If the binary is copied we should message the user that they can delete the version they have in their current directory. Or at least message that it has been copied so the user understands they can delete that copy.

Thanks, I've reworked this paragraph to notify the user about the case where swiftly is copied into the swiftly bin directory so that they are aware.

patrickfreed

sorry for the late review, mostly looks good just have a few minor suggestions

Sources/SwiftlyCore/SwiftlyCore.swift

DESIGN.md

…wiftly binary

DESIGN.md

cmcgee1024 · 2024-07-22T19:30:12Z

@swift-ci test macOS

cmcgee1024 · 2024-07-24T16:25:48Z

@swift-ci test macOS

cmcgee1024 · 2024-07-24T17:04:45Z

@swift-ci test macOS

cmcgee1024 · 2024-07-24T17:51:14Z

@swift-ci test macOS

…hain

cmcgee1024 · 2024-07-24T19:40:37Z

@swift-ci test macOS

… shell environment

cmcgee1024 · 2024-07-26T16:05:09Z

@swift-ci test macOS

Automate the installing of the swift gpg signatures into the user keyring

cmcgee1024 · 2024-07-26T17:47:51Z

@swift-ci test macOS

Fix error message if swiftly is run outside of an installation

cmcgee1024 · 2024-07-29T11:47:57Z

@swift-ci test macOS

cmcgee1024 · 2024-07-29T12:35:45Z

@swift-ci test macOS

cmcgee1024 · 2024-07-29T12:39:30Z

@adam-fowler @patrickfreed this should be ready for review now. The macOS tests all pass, but there's some kind of CI system problem that throws errors at the end. Thank you.

adam-fowler

Some minor change requests, but in general looks good.

adam-fowler · 2024-08-02T07:18:12Z

Sources/Swiftly/Swiftly.swift

@@ -45,8 +53,22 @@ public struct Swiftly: SwiftlyCommand {

 public protocol SwiftlyCommand: AsyncParsableCommand {}

+extension Data {
+    func append(file: URL) throws {


It isn't very clear what this function is doing from its name

Suggested change

func append(file: URL) throws {

func append(to file: URL) throws {

Also where is this used, might be better to move this to nearer its usage

I've added this change to the signature.

adam-fowler · 2024-08-02T07:25:55Z

Sources/Swiftly/Init.swift

+
+        if let config = config, !overwrite && config.version != SwiftlyCore.version {
+            // We don't support downgrades, and we don't yet support upgrades
+            throw Error(message: "An existing swiftly installation was detected. You can try overwriting it.")


What is expected of the user here?

Ah I understand, you need to add --overwrite. Maybe make the error more explicit. Also when running with --overwrite on Linux it returns a different error

Error: The operation could not be completed. A file with the same name already exists.

I've made the message more explicit about the --overwrite flag since it's in the init subcommand and the error can directly reference the subcommand's flag.

adam-fowler · 2024-08-02T07:39:51Z

Sources/Swiftly/Config.swift

        } catch {
            let msg = """
            Could not load swiftly's configuration file at \(Swiftly.currentPlatform.swiftlyConfigFile.path) due to
            error: \"\(error)\".
-            To use swiftly, modify the configuration file to fix the issue or perform a clean installation.
+            To begin using swiftly you can install it by running 'swiftly init'.
            """


The error messages you get when a config is not there includes a lot of extraneous info.

error: "Error Domain=NSCocoaErrorDomain Code=260 "The file “config.json” couldn’t be opened because there is no such file." UserInfo={NSFilePath=/Users/adamfowler/Library/Application Support/swiftly/config.json, NSUnderlyingError=0x133207dc0 {Error Domain=NSPOSIXErrorDomain Code=2 "No such file or directory"}}"

Can we translate some of these into more user friendly strings

I've pulled out this inner error since it's likely to cause more confusion. The focus should be on the instruction to run the init.

adam-fowler · 2024-08-02T08:05:25Z

Sources/LinuxPlatform/Linux.swift

+        }
+
+        guard let id = id, let idlike = idlike else {
+            let message = "Unable to find release information from file \(releaseFile)"


Suggested change

let message = "Unable to find release information from file \(releaseFile)"

let message = "Unable to find OS release information from file \(releaseFile)"

Thanks, this is much clearer.

adam-fowler · 2024-08-02T08:05:42Z

Sources/LinuxPlatform/Linux.swift

+
+        let data = FileManager.default.contents(atPath: releaseFile)
+        guard let data = data else {
+            let message = "Unable to read release information from file \(releaseFile)"


Suggested change

let message = "Unable to read release information from file \(releaseFile)"

let message = "Unable to read OS release information from file \(releaseFile)"

adam-fowler · 2024-08-02T08:19:00Z

Sources/Swiftly/Init.swift

+            Note that the locations can be changed with SWIFTLY_HOME and SWIFTLY_BIN environment variables and run
+            this again.
+
+            Proceed with the installation?


Can we change this to a [Y/n]?

I've changed the prompt to a yes or no instead of numbered options.

adam-fowler · 2024-08-02T08:24:54Z

README.md

@@ -11,6 +11,11 @@ To download swiftly and install Swift, run the following in your terminal, then
 curl -L https://swiftlang.github.io/swiftly/swiftly-install.sh | bash


We either need to edit this script to download the correct version of swiftly (Linux or mac) and run swiftly init or get rid of this line in the docs.

This is tricky since this README is one of the places where people see how to install swiftly, but it's a few commits newer than the last release. Also, it's unclear if we are going to cut a release or not that still makes use of the shell script, which will still technically work.

I think we should stick to one installation method. If we add swiftly init then the install script should become history, or at least be reduced to copy the correct version of swiftly off the internet.

reduced to copy the correct version of swiftly off the internet.

Yeah that's probably a good idea ... i think the maximum installation complexity i'd expect is "choose macOS or linux". Even that can probably be avoided in the script. Hopefully no manual architecture detection "are you on arm64 or amd64?!". manually choosing the arch is not the end of the world, but it can be harder than it looks, to beginners.

adam-fowler · 2024-08-02T08:31:52Z

Documentation/SwiftlyDocs.docc/automated-install.md

+```
+To begin using installed swiftly from your current shell, first run the following command:
+
+. "/root/.local/share/swiftly/env.sh"


Need to make it clear this is for Linux, maybe provide macOS version as well

env.sh is not accurate, for fish, it'll be env.fish.

env.sh is not accurate, for fish, it'll be env.fish.

Right, I'm hoping to make this an illustrative example of something that an integrator would look for from the swiftly output so that they can automate it. I'll see if I can make the description above highlight that this is just an example and can be different depending on the environment (e.g. Windows or Fish)

adam-fowler · 2024-08-02T08:33:26Z

Documentation/SwiftlyDocs.docc/getting-started.md

@@ -6,6 +6,12 @@ To download swiftly and install Swift, run the following in your terminal, then
 curl -L https://swiftlang.github.io/swiftly/swiftly-install.sh | bash


We either need to edit this script to download the correct version of swiftly (Linux or mac) and run swiftly init or get rid of this line in the docs.

It's the same kind of problem with the readme. People might be reading this from the main branch of swiftpackageindex.com and want the latest release, which is using the shell script installer. We'll need to sweep all of the documentation when we release to switch over to the new method.

Ok we should create a separate PR with documentation changes

Sources/Swiftly/Install.swift

Sources/LinuxPlatform/Linux.swift

adam-fowler

I've added a suggested fix for an issue on Linux when downloading gpg keys

Sources/LinuxPlatform/Linux.swift

Sources/Swiftly/Init.swift

adam-fowler · 2024-08-03T07:50:03Z

Sources/SwiftlyCore/Platform.swift

+
+        try process.run()
+        // Attach this process to our process group so that Ctrl-C and other signals work
+        let pgid = tcgetpgrp(STDOUT_FILENO)


Can we add the same process group support to runProgram

I've added this functionality to the runProgram too.

adam-fowler · 2024-08-03T13:33:56Z

Sources/Swiftly/Init.swift

+
+            if swiftlyBin.fileExists() {
+                if !overwrite {
+                    throw Error(message: "Swiftly binary already exists. You can try again with overwrite to replace it.")


Suggested change

throw Error(message: "Swiftly binary already exists. You can try again with overwrite to replace it.")

throw Error(message: "Swiftly binary already exists. You can try again with `--overwrite` to replace it.")

I've fixed the wording here. Thank you.

cmcgee1024 · 2024-08-06T13:26:02Z

@swift-ci test macOS

cmcgee1024 · 2024-08-07T13:11:28Z

@swift-ci test macOS

cmcgee1024 · 2024-08-07T13:20:55Z

@adam-fowler @patrickfreed although the macOS platform check is failing, that is happening after the tests due to some kind of problem with the CI system itself. The tests are all reporting as successful there.

cmcgee1024 force-pushed the self-install branch from a9d0bad to 2504348 Compare June 19, 2024 17:50

cmcgee1024 force-pushed the self-install branch from 2504348 to 9eb8c91 Compare June 20, 2024 11:50

cmcgee1024 requested review from patrickfreed and adam-fowler June 24, 2024 12:43

adam-fowler reviewed Jun 25, 2024

View reviewed changes

patrickfreed reviewed Jun 26, 2024

View reviewed changes

DESIGN.md Outdated Show resolved Hide resolved

DESIGN.md Outdated Show resolved Hide resolved

Sources/SwiftlyCore/SwiftlyCore.swift Show resolved Hide resolved

DESIGN.md Outdated Show resolved Hide resolved

DESIGN.md Outdated Show resolved Hide resolved

Design rework based on feedback, removing the swiftly-init accelerato…

c272921

…r entirely Describe in more detail the anticipated delivery methods

adam-fowler approved these changes Jun 28, 2024

View reviewed changes

Add a note about the case when swiftly is copied during init

2801b53

0xTim approved these changes Jul 3, 2024

View reviewed changes

patrickfreed reviewed Jul 3, 2024

View reviewed changes

Sources/SwiftlyCore/SwiftlyCore.swift Show resolved Hide resolved

DESIGN.md Outdated Show resolved Hide resolved

DESIGN.md Outdated Show resolved Hide resolved

DESIGN.md Outdated Show resolved Hide resolved

cmcgee1024 added 2 commits July 5, 2024 13:53

Separate the swiftly version changes from this PR

f2609dc

Provide more specific details about SWIFTLY_BIN_DIR, and moving the s…

3af19ac

…wiftly binary

patrickfreed reviewed Jul 8, 2024

View reviewed changes

DESIGN.md Outdated Show resolved Hide resolved

DESIGN.md Outdated Show resolved Hide resolved

cmcgee1024 added 2 commits July 9, 2024 10:12

Update the design based on feedback

eeb9d36

Merge branch 'main' of github.com:cmcgee1024/swiftly into self-install

b64f4bd

cmcgee1024 added 2 commits July 24, 2024 12:05

DRAFT: Add init implementation and tests

dd15324

Fix formatting problems and remove problematic check in e2e install test

1d3bb26

Output the shell and version in the e2e test

1cd01be

Force a hash recalculation in the e2e test

c8cb477

cmcgee1024 added 2 commits July 24, 2024 14:02

Provide more details of the failed URL when trying to install a toolc…

763b284

…hain

Fix the URL assembly for Linux x86 toolchain downloads

72b7025

Directly source the env script in e2e test rather than relying on the…

a2a6d4b

… shell environment

Add swiftly system prerequisite checks for ca-certificates on Linux

11d112a

Automate the installing of the swift gpg signatures into the user keyring

cmcgee1024 marked this pull request as ready for review July 26, 2024 17:49

Fallback to copy if move of swiftly binary fails

373be6a

Fix error message if swiftly is run outside of an installation

Fix error message for the error case in the system managed binary check

886865f

cmcgee1024 changed the title ~~DRAFT: Merge installation logic into swiftly as an init subcommand~~ Merge installation logic into swiftly as an init subcommand Jul 29, 2024

This was referenced Jul 29, 2024

macOS: Generate config.json in swiftly-install.sh #138

Closed

swiftly-install.sh Handle paths containing spaces #135

Closed

adam-fowler requested changes Aug 2, 2024

View reviewed changes

adam-fowler reviewed Aug 2, 2024

View reviewed changes

Sources/Swiftly/Install.swift Show resolved Hide resolved

vsarunas mentioned this pull request Aug 2, 2024

Ubuntu 24.04, 23.10 Fedora 39 and Debian 12 #115

Closed

adam-fowler reviewed Aug 2, 2024

View reviewed changes

Sources/LinuxPlatform/Linux.swift Show resolved Hide resolved

adam-fowler requested changes Aug 2, 2024

View reviewed changes

Sources/LinuxPlatform/Linux.swift Outdated Show resolved Hide resolved

Sources/LinuxPlatform/Linux.swift Show resolved Hide resolved

adam-fowler reviewed Aug 2, 2024

View reviewed changes

Sources/Swiftly/Init.swift Outdated Show resolved Hide resolved

adam-fowler reviewed Aug 2, 2024

View reviewed changes

Sources/Swiftly/Init.swift Outdated Show resolved Hide resolved

cmcgee1024 added 2 commits August 2, 2024 12:15

Review feedback and fixes

0aa21e0

Fix the tests

7081738

adam-fowler reviewed Aug 3, 2024

View reviewed changes

Review feedback

361bc2f

adam-fowler approved these changes Aug 8, 2024

View reviewed changes

cmcgee1024 merged commit eba9bd9 into swiftlang:main Aug 8, 2024
5 of 6 checks passed

adam-fowler mentioned this pull request Aug 8, 2024

Swiftly doesn't reliably install dependencies #144

Closed

	func append(file: URL) throws {
	func append(to file: URL) throws {

	let message = "Unable to find release information from file \(releaseFile)"
	let message = "Unable to find OS release information from file \(releaseFile)"

	let message = "Unable to read release information from file \(releaseFile)"
	let message = "Unable to read OS release information from file \(releaseFile)"

		@@ -11,6 +11,11 @@ To download swiftly and install Swift, run the following in your terminal, then
		curl -L https://swiftlang.github.io/swiftly/swiftly-install.sh \| bash

		@@ -6,6 +6,12 @@ To download swiftly and install Swift, run the following in your terminal, then
		curl -L https://swiftlang.github.io/swiftly/swiftly-install.sh \| bash

	throw Error(message: "Swiftly binary already exists. You can try again with overwrite to replace it.")
	throw Error(message: "Swiftly binary already exists. You can try again with `--overwrite` to replace it.")

Merge installation logic into swiftly as an init subcommand #127

Merge installation logic into swiftly as an init subcommand #127

Uh oh!

Conversation

cmcgee1024 commented Jun 19, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

cmcgee1024 commented Jun 28, 2024

Uh oh!

adam-fowler left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

patrickfreed left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

cmcgee1024 commented Jul 22, 2024

Uh oh!

cmcgee1024 commented Jul 24, 2024

Uh oh!

cmcgee1024 commented Jul 24, 2024

Uh oh!

cmcgee1024 commented Jul 24, 2024

Uh oh!

cmcgee1024 commented Jul 24, 2024

Uh oh!

cmcgee1024 commented Jul 26, 2024

Uh oh!

cmcgee1024 commented Jul 26, 2024

Uh oh!

cmcgee1024 commented Jul 29, 2024

Uh oh!

cmcgee1024 commented Jul 29, 2024

Uh oh!

cmcgee1024 commented Jul 29, 2024

Uh oh!

adam-fowler left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

cmcgee1024 commented Jun 19, 2024 •

edited

Loading