Skip to content

Conversation

@patrickfreed
Copy link
Contributor

Implements #60

@patrickfreed
Copy link
Contributor Author

@swift-server-bot test install please

@patrickfreed
Copy link
Contributor Author

@swift-server-bot test install please

esac
}

verify_getopt_install () {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

getopt is a super useful tool for parsing CLI options in bash scripts. It's included in the util-linux package, but in practice basically every Linux distro comes with it pre-installed...except Amazon Linux 2. The alternatives are manual parsing (which gets tricky with stuff like -pubuntu20.04, -p ubuntu20.04, --platform=ubuntu20.04, and --platform ubuntu20.04) or using the shell builtin getopts, which doesn't support long names like --platform. Given that we already require curl to be installed, it didn't seem like a huge problem to add an additional dependency on util-linux.

return "$?"
}

SWIFTLY_INSTALL_VERSION="0.2.0"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the swiftly-install version doesn't need to be tied to swiftly's actual version imo, since we may make updates to it like this independent of changes to swiftly.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to keep the tags on the repo in sync though. A new version of the script should be tagged a release, otherwise managing versions of the script is going to be painful.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I agree we ought to tag releases of swiftly-install, though we shouldn't mark them as a full on "Releases" in the GitHub sense of things, since then we'd need the install script to disambiguate when trying to download binaries.

@patrickfreed patrickfreed marked this pull request as ready for review September 22, 2023 13:47
Copy link
Contributor

@adam-fowler adam-fowler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All looks good.

return "$?"
}

SWIFTLY_INSTALL_VERSION="0.2.0"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to keep the tags on the repo in sync though. A new version of the script should be tagged a release, otherwise managing versions of the script is going to be painful.

@patrickfreed patrickfreed merged commit 438b30c into swiftlang:main Sep 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants