Skip to content
This repository has been archived by the owner on Oct 13, 2023. It is now read-only.

Don't require a key to install cached program #125

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

flip1995
Copy link
Contributor

@flip1995 flip1995 commented Sep 10, 2020

With the runner name and the program version a unique key can already be generated.

TODO: How to get the runner name? @svartalf

I now use os.{platform,release,arch}() to determine the runner. I'm not sure if this is robust enough to prevent problems.

(This also improves the documentation)

This PR is necessary for actions-rs/install@57e52b9

With the runner name and the program version a unique key can already be generated
@flip1995
Copy link
Contributor Author

I'm very unsure, if using os to get the runner name is a good idea here. 🤔

@thomaseizinger
Copy link

I'm very unsure, if using os to get the runner name is a good idea here. thinking

You could use the runner context instead: https://docs.github.com/en/actions/reference/context-and-expression-syntax-for-github-actions#runner-context

@flip1995
Copy link
Contributor Author

flip1995 commented Jan 29, 2021

The runner context only tells me the OS, not the arch or other important information.

@coltfred
Copy link

@thomaseizinger Was there any feedback to @flip1995's comment about runner? I'm interested in keeping this moving so that actions-rs/install#5 can keep moving. I'm not highly experienced with github actions, but I'm willing to help in any way I can to keep things going.

@thomaseizinger
Copy link

@thomaseizinger Was there any feedback to @flip1995's comment about runner? I'm interested in keeping this moving so that actions-rs/install#5 can keep moving. I'm not highly experienced with github actions, but I'm willing to help in any way I can to keep things going.

Nope, as @flip1995 correctly pointed out, the runner context only provides the OS name. I made my original comment based on the misunderstanding that @flip1995 only wanted an alternative the OS name but thinking more about it, it is likely cleaner to use only one source (the os JS module) to collect all this information instead of stitching it together using GitHub context and os module for arch and version.

@alex
Copy link
Contributor

alex commented Apr 12, 2021

FWIW my read is that using platform+release+arch from OS should cover all the variations that exist (linux vs windows, 32-bit vs 64-bit, arm64 vs. x86-64).

@TDHolmes
Copy link

Any update here? would be great if this could move forward for the PRs that depend on this change

@flip1995
Copy link
Contributor Author

No updates from my side. This is still waiting for review.

All the projects I'm involved in moved away from using actions-rs actions though. I'm still willing to finish this PR, but without a review there's nothing much I can do here.

@flip1995
Copy link
Contributor Author

flip1995 commented Feb 4, 2022

Friendly ping @svartalf: This PR and the actions-rs/install#5 PR are waiting for your review.

Can you give us an update, if you are able to review this in the future?

Since there was no update to this repo in over 1 year, I assume you stopped maintaining the actions-rs repositories. If so, please let us know so that we can explore alternatives.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

5 participants