Skip to content
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

Support yj's updated download filename #166

Merged
merged 1 commit into from
Jan 4, 2023

Conversation

edmorley
Copy link
Collaborator

@edmorley edmorley commented Jan 3, 2023

As of yj 5.1.0+ the filename of the asset attached to the GitHub release is now yj-linux-arm64 instead of yj-linux.

For example:
https://github.com/sclevine/yj/releases/tag/v5.1.0
vs
https://github.com/sclevine/yj/releases/tag/v5.0.0

This change ensures that the new yj default (as of #164; which updated the default yj from 5.0.0 to 5.1.0) is able to be downloaded, since currently the setup-tools action fails with:

Installing yj 5.1.0
curl: (22) The requested URL returned error: 404
Error: Process completed with exit code 22.

The bash string comparison of the semver version isn't ideal, but it doesn't feel worth adding further complexity given that:

  • all yj releases so far use a full semver version (and not say an X.Y version)
  • yj major releases are infrequent, so it's going to be a while before v10 is out (which would break the comparison) - by which point this backwards compatibility comparison can just be removed (and support for pre v5.1.0 removed).
  • any failure modes will be fairly noisy/obvious, and won't result in a subtle bug.

As of `yj` 5.1.0+ the filename of the asset attached to the GitHub release
is now `yj-linux-arm64` instead of `yj-linux`.

This change ensures that the new yj default (as of buildpacks#164) is able to be
downloaded, since currently the action fails with:

```
Installing yj 5.1.0
curl: (22) The requested URL returned error: 404
Error: Process completed with exit code 22.
```

The bash string comparison of the semver version isn't ideal, but it
doesn't feel worth adding further complexity given that:
- all `yj` releases so far use a full semver version (and not say an `X.Y` version)
- `yj` major releases are infrequent, so it's going to be a while before v10 is out
  (which would break the comparison) - by which point this backwards compatibiluty
  comparison can just be removed (and support for pre 5.1.0 removed).

Signed-off-by: Ed Morley <501702+edmorley@users.noreply.github.com>
@edmorley edmorley force-pushed the support-new-yj-filename branch from 66a27c6 to 48a6c34 Compare January 3, 2023 22:30
@edmorley edmorley marked this pull request as ready for review January 3, 2023 22:31
@edmorley edmorley requested a review from a team as a code owner January 3, 2023 22:31
@jkutner jkutner added type:enhancement A general enhancement semver:patch A change requiring a patch version bump labels Jan 4, 2023
@jkutner jkutner merged commit a75ba27 into buildpacks:main Jan 4, 2023
@edmorley edmorley deleted the support-new-yj-filename branch January 4, 2023 15:04
@edmorley
Copy link
Collaborator Author

edmorley commented Jan 6, 2023

@jkutner Thank you for the review/merge. I don't suppose you could release v5.0.1 of the action, so as to fix the stable latest setup-tools action release?

@jkutner
Copy link
Member

jkutner commented Jan 6, 2023

done. thanks for the nudge

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
semver:patch A change requiring a patch version bump type:enhancement A general enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants