Skip to content

feat!: add support for asdf >= 0.16 #590

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 14 commits into from
Mar 14, 2025
Merged

Conversation

Dabolus
Copy link
Contributor

@Dabolus Dabolus commented Feb 12, 2025

This PR adds support for asdf >= 0.16, which has been rewritten from scratch in Go and is no longer simply cloneable from git. It does so by downloading the pre-built release from GitHub for the correct platform and architecture.

The change adds support for a new optional parameter, asdf_version, which can be used to set a specific version to download and install. If not provided, the latest release will be downloaded by default.

I decided to add a new parameter because the existing one (asdf_branch) allows to specify not only a given version tag, but also any kind of branch. To support the same parameter with the rewritten asdf, the action should build asdf from source, but I don't think that's needed.

As of today, asdf_branch can be used to download the older, bash-based versions of asdf, so it's still possible to use the actions with those versions by simply adding e.g. asdf_branch: v0.15.0.

Note that, even though the older asdf versions are still downloadable using the parameter above, by default the new behavior is used, so this will most probably need a major version bump.

Let me know if the implementation makes sense and/or if I missed something.

Fixes #587

@Dabolus Dabolus requested a review from jthegedus as a code owner February 12, 2025 20:06
@Dabolus Dabolus changed the title Add support for asdf >= 0.16 feat!: add support for asdf >= 0.16 Feb 12, 2025
@takirala
Copy link

Pinging @jthegedus to get some traction on this! 🙏

@wmcnamee-coreweave
Copy link

another ping to get some traction on this: @jthegedus 🙏

@DeadManPoe
Copy link

DeadManPoe commented Mar 13, 2025

Another ping for @jthegedus as this is kinda blocking work we are doing in my company and it would be fantastic to merge this and enable asdf v16 in github actions

Copy link
Contributor

@jthegedus jthegedus left a comment

Choose a reason for hiding this comment

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

@Dabolus I agree with the need for a major version bump 👍

The new var makes sense.

Thanks for doing this work.

[Edit 1]
GitHub does not allow workflows that are older than 30 days to be manually triggered, so I had to update the PR. I bumped the version of this Action in the docs.

[Edit 2]
Currently looking into the CICD errors reported and solutions available.


Apologies for the delayed review everyone.

@Dabolus
Copy link
Contributor Author

Dabolus commented Mar 13, 2025

Thank you so much for the review 🙏🏻

I see that the actions failed because of the GitHub API rate limit. Any idea on how to avoid it? Maybe we can optionally accept the GitHub Token to use in the request, not sure if there is a better way to handle it.

EDIT: for context, here are the rate limits of the GitHub REST API. Unauthenticated have 60 req/h, tracked by IP, while authenticated users have 5000 req/h, tracked by user

@jthegedus
Copy link
Contributor

jthegedus commented Mar 13, 2025

Yes, you are right. I have been doing some reading. It seems we will need to support the "optional" use of github_token value from the Action context as we are doing in the plugin_test action

github_token:
description: Token used to avoid rate limit when asdf calls the GitHub API
required: false
default: ${{ github.token }}

And then pass that to the @actions/http-client [I assume] as shown in the test case

https://github.com/actions/toolkit/blob/253e837c4db937cac18949bc65f0ffdd87496033/packages/http-client/__tests__/auth.test.ts#L33-L39

    const token = 'scbfb44vxzku5l4xgc3qfazn3lpk4awflfryc76esaiq7aypcbhs'
    const ph: am.PersonalAccessTokenCredentialHandler =
      new am.PersonalAccessTokenCredentialHandler(token)

    const http: httpm.HttpClient = new httpm.HttpClient('http-client-tests', [
      ph
    ])

If you are able to add this change that would be great, I won't be able to implement or merge for another 9hrs, if not I can do it then. Sorry for the delay all. We will get this out to you soon.

Thanks again for this contribution.

@Dabolus
Copy link
Contributor Author

Dabolus commented Mar 13, 2025

Thank you so much for the resources!

I implemented and tested the changes and everything seems to be working fine in my experiments. Let me know if you need anything else on my side 🙏🏻

@jthegedus
Copy link
Contributor

Works as expect, thanks!

I just need to update a few of the test workflows 👍

@jthegedus
Copy link
Contributor

Okay, so, the remaining failures are actually not related to this change.

The test/plugin_is_tested failures are because v0.16.0 of asdf made it a hard requirement for plugins to have a bin/download script. Once the fix for this Action is deployed and people use v0.16.0 of asdf in their plugin repositories they will be aware of the error and change.

The CodeQL and other maintenance updates I will perform in a different PR once I merge this.

Thanks 🙇

@jthegedus jthegedus merged commit 1117842 into asdf-vm:master Mar 14, 2025
17 of 20 checks passed
@jthegedus
Copy link
Contributor

To use this change immediately you can reference the specific SHA from master:

steps:
  # Reference a specific commit (most strict, for the supply-chain paranoid)
  - uses: asdf-vm/actions/install@1117842ea70e2711a0072e3a71265cbfe2c830be

The final v4 release requires additional maintenance in other areas before release.

Thanks for your collective patience.

@wmcnamee-coreweave
Copy link

this was merged, but no release/tag was made. What's your release cycle?

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.

Notice message about 0.16.0 asdf version
5 participants