setup-nextstrain-cli: Use a standalone installation instead of Pip/PyPI #55
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
tsibley
added a commit
to nextstrain/nextstrain.org
that referenced
this pull request
Feb 12, 2024
…loads
This allows URLs like:
https://nextstrain.org/cli/download/>=7.4.0/standalone-x86_64-unknown-linux-gnu.tar.gz
which will let us use the standalone installer, e.g.
curl -fsSL --proto '=https' https://nextstrain.org/cli/installer/linux | bash -s '>=7.4.0'
in automated contexts where we want to be able to declare constraints
like lower version bounds or incompatible versions.
Note that the standalone installer on macOS on aarch64 hardware does
very rudimentary version comparison¹ to decide if the requested version
is older or newer than the first release with actual aarch64 support
(8.2.0), and it won't compare version range constraints correctly:
they'll always be considered greater than (newer/later) than 8.2.0. But
I expect this to be fine in practice and not matter for actual usage.
We depend on an older version of @renovatebot/pep440 (<3) because >=3
adds a dependency on Node >=18 and this codebase is still on Node 16. I
reviewed the changelog for newer versions of the package and nothing
else substantial seems to have changed anyhow.
Related-to: <nextstrain/.github#55>
¹ <nextstrain/cli#358>
<https://github.com/nextstrain/cli/blob/af976b06/bin/standalone-installer-unix#L146-L154>
1 task
tsibley
force-pushed
the
trs/setup-nextstrain-cli/standalone-installation
branch
from
b4855de to
e2e8ced
Compare
tsibley
changed the title
tsibley
force-pushed
the
trs/setup-nextstrain-cli/standalone-installation
branch
from
e2e8ced to
026caab
Compare
It's intended to work there, so test it.
A big caveat on Windows is that no runtime is set up since none are supported given the intersection of our runtime constraints and the constraints of GitHub Actions Windows runners.
Simplifies and speeds up installation. Aligns with our recommended installation used by many users and many of ourselves. Drops the baggage of setting up a Python install as a side-effect. Discussed on Slack a while back.¹ In order to continue supporting PEP 440/508 (i.e. Pip-style) version range constraints in the cli-version input, I added support for those to the https://nextstrain.org/cli/download/… endpoint.² I've audited all existing callsites I could find, internally and externally, for transitive reliance on the setup-python step. None seemed to rely upon it, so it's now gone. See also the added comment about removing the python-version input. ¹ <https://bedfordlab.slack.com/archives/C01LCTT7JNN/p1694627907115309> ² <nextstrain/nextstrain.org#797>
To avail ourselves of the switch to the standalone installer.
tsibley
force-pushed
the
trs/setup-nextstrain-cli/standalone-installation
branch
from
026caab to
ef5c51a
Compare
|
Going to merge this now pre-review so I can see it in action (har har) today and address anything that arises. If it's a big flop, we'll just revert! As always, I'd welcome post-merge review and feedback. |
tsibley
added a commit
to nextstrain/docker-base
that referenced
this pull request
Feb 16, 2024
Deprecated and no longer used as setup-python isn't called. Related-to: <nextstrain/.github#55>
1 task
tsibley
added a commit
to nextstrain/ncov
that referenced
this pull request
Feb 16, 2024
Deprecated and no longer used as setup-python isn't called. Related-to: <nextstrain/.github#55>
victorlin
reviewed
Feb 16, 2024
Comment on lines
+40
to
+49
| # XXX TODO: Remove this in coordination with existing callers. Callers must | ||
| # stop specifying it before we remove it here in order to not break. I've | ||
| # audited all of our internal usages¹ of this action to check that they don't | ||
| # rely on the setup-python call and prepped those usages for the removal of | ||
| # this input. There are also many external usages², mostly existing in forks | ||
| # of our repos. | ||
| # -trs, 13 Feb 2024 | ||
| # | ||
| # ¹ <https://github.com/search?q=org%3Anextstrain+%2Fsetup-nextstrain-cli%40master%2F+%28path%3A*.yaml+OR+path%3A*.yml%29&type=code&p=1> | ||
| # ² <https://github.com/search?q=%28NOT+org%3Anextstrain%29+%2Fsetup-nextstrain-cli%40master%2F+%28path%3A*.yaml+OR+path%3A*.yml%29&type=code&p=1> |
There was a problem hiding this comment.
It looks like all external usages are either forks or exact copies of one of our repos. I did a quick spot check and didn't find any that are actively using the GitHub workflows.
I don't think coordination with the external usages is feasible. I would be ok with dropping it now that internal usages are up to date. In the slim chance that this breaks external usage, they should be able to contact us and we can help them fix it.
There was a problem hiding this comment.
Nod. I'd be fine with that, but didn't want to start something that might cause broader fallout while I'm out. But by all means you can start it… :-)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Simplifies and speeds up installation. Aligns with our recommended installation used by many users and many of ourselves. Drops the baggage of setting up a Python install as a side-effect. Discussed on Slack a while back.¹
In order to continue supporting PEP 440/508 (i.e. Pip-style) version range constraints in the cli-version input, I added support for those to the https://nextstrain.org/cli/download/... endpoint.²
I've audited all existing callsites I could find, internally and externally, for transitive reliance on the setup-python step. None seemed to rely upon it, so it's now gone. See also the added comment about removing the python-version input.
¹ https://bedfordlab.slack.com/archives/C01LCTT7JNN/p1694627907115309
² nextstrain/nextstrain.org#797
Todo
setup-pythonpython-versionandsetup-pythonChecklist