Skip to content

Fix installation of a tag from repos with many tags#15

Merged
alexcrichton merged 3 commits intobytecodealliance:mainfrom
alexcrichton:fix-installing-old-version
Feb 8, 2025
Merged

Fix installation of a tag from repos with many tags#15
alexcrichton merged 3 commits intobytecodealliance:mainfrom
alexcrichton:fix-installing-old-version

Conversation

@alexcrichton
Copy link
Copy Markdown
Member

Attempting to fix #14

@rajatjindal
Copy link
Copy Markdown
Member

Hi @alexcrichton, aplogies i missed this issue earlier. just got notified again. I can check today itself so that you can focus on other priority stuff.

@alexcrichton
Copy link
Copy Markdown
Member Author

Oh no worries! I figured I'd give things a shot and I think I'm close to a fix, just wanted to see some failures on CI first which are now rolling in, so now I get to see if my fix fixes those failures...

@rajatjindal
Copy link
Copy Markdown
Member

Oh no worries! I figured I'd give things a shot and I think I'm close to a fix, just wanted to see some failures on CI first which are now rolling in, so now I get to see if my fix fixes those failures...

thank you.

Don't use `listReleases` but instead use methods which look like they're
targeted for a particular release.
@alexcrichton alexcrichton force-pushed the fix-installing-old-version branch from c6a0eaf to 5933af5 Compare February 7, 2025 03:17
@alexcrichton
Copy link
Copy Markdown
Member Author

Ok looks like that fixed ci, yay! CI is failing currently in this repo due to some actions using historical/deprecated versions and additionally due to #14, so this PR is effectively fixing CI again at the same time as fixing #14.

@rajatjindal if you're able to review that'd be most welcome, but if you're busy as well I'm happy to work on merging/tagging this too!

@alexcrichton
Copy link
Copy Markdown
Member Author

(also no rush of course, feel free to get to this at your leisure!)

@rajatjindal
Copy link
Copy Markdown
Member

Hi @alexcrichton, thanks for this PR.

I think we also need to fix getDownloadLink to handle pagination. and IIRC there was a scenario where getting the latest release may not work (e.g. if it is a monorepo and we publish multiple release-artifacts from the same repo), which is why I had used listReleases function (but I couldn't find a example repo for this during my brief search).

does that make sense? if so, I can submit the PR that makes use of pagination and fixes this issue. But also happy to continue to review this PR if you think otherwise.

@alexcrichton
Copy link
Copy Markdown
Member Author

alexcrichton commented Feb 7, 2025

Sounds good to me, but to confirm you meant the getLatestRelease function right? I've left getDownloadLink as fetching a release directly, but I've restored getLatestRelease to the old behavior (plus new pagination support)

@rajatjindal
Copy link
Copy Markdown
Member

Sounds good to me, but to confirm you meant the getLatestRelease function right? I've left getDownloadLink as fetching a release directly, but I've restored getLatestRelease to the old behavior (plus new pagination support)

I meant to say we may need pagination in both. because in the getDownloadLink we are getting all releases (again) and finding the release corresponding to the tag, so we may endup with a scenario where the release is on next page.

@alexcrichton
Copy link
Copy Markdown
Member Author

Oh hm I may be a bit confused then. In getLatestRelease it makes sense to use pagination iterating the releases, but getDownloadLink was updated to use getReleaseByTag instead of listReleases which I didn't think required any pagination. Can you highlight which part needs updating in getDownloadLink?

@rajatjindal
Copy link
Copy Markdown
Member

Oh hm I may be a bit confused then. In getLatestRelease it makes sense to use pagination iterating the releases, but getDownloadLink was updated to use getReleaseByTag instead of listReleases which I didn't think required any pagination. Can you highlight which part needs updating in getDownloadLink?

ah, I missed that change in the PR. yeah, that should be good IMO.

Thanks Alex.

@alexcrichton alexcrichton merged commit 3b93676 into bytecodealliance:main Feb 8, 2025
@alexcrichton alexcrichton deleted the fix-installing-old-version branch February 8, 2025 17:58
rajatjindal pushed a commit to rajatjindal/bytecodealliance-actions that referenced this pull request Mar 18, 2026
)

* Update versions of actions used

* Fix installing older tags

Don't use `listReleases` but instead use methods which look like they're
targeted for a particular release.

* Restore old behavior of `getLatestRelease`, but use pagination
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.

Installation of too-old Wasmtime version fails

2 participants