Use GitHub Actions cache for installed binaries #5

flip1995 · 2020-06-25T14:25:25Z

Fixes #4

This uses the changes from actions-rs/core#92 to install

For easier review, I'll comment some of the code changes.

This is the first time I'm writing TS code ever, so I'm not confident that this is the right way to do it. I think, after merging actions-rs/core#92, I have to rebuild and commit dist/index.js?

This change has to be merged together with/after actions-rs/core#92

Drawbacks

This removes the use-cache key, since with this implementation it is useless. I wouldn't consider this breaking change a problem though, since this is an experimental action and this key was unused anyway.

This was moved there, because it was also required for caching installed binaries

src/download.ts

action.yml

Blisto91 · 2020-07-16T17:16:17Z

Do i understand it correctly that this would work with any crate installed through actions-rs/install and not just specific precompiled binaries like it is currently?

flip1995 · 2020-07-16T17:49:53Z

@Blisto91 exactly. It will cache every binary in a cache named <binary-name>-<key>, if you provide a key (like in the actions/cache action)

You can still fall back to actions-rs/tool-cache if you want, though. This is useful for 2 reasons:

backwards compat for the use-tool-cache key
You don't have to cache the binary yourself, if it is in the tool-cache action.

Blisto91 · 2020-07-16T17:53:48Z

@flip1995 Sounds awesome!

Hope it gets merged soon :).

svartalf · 2020-08-06T20:17:34Z

@flip1995 I published new actions-rs/core version with your changes in it (should be 0.1.4), can you update your PR?

flip1995 · 2020-08-06T21:13:32Z

@svartalf did something during publishing go wrong?

src/download.ts:5:10 - error TS2305: Module '"../node_modules/@actions-rs/core/dist/core"' has no exported member 'resolveVersion'.

5 import { resolveVersion } from "@actions-rs/core";
           ~~~~~~~~~~~~~~

src/input.ts:21:31 - error TS2339: Property 'getInputAsArray' does not exist on type 'typeof import("/home/runner/work/install/install/node_modules/@actions-rs/core/dist/input")'.

21     const restoreKeys = input.getInputAsArray("restore-keys") || undefined;
                                 ~~~~~~~~~~~~~~~

src/main.ts:59:13 - error TS2554: Expected 1-2 arguments, but got 4.

59             options.primaryKey,
               ~~~~~~~~~~~~~~~~~~~
60             options.restoreKeys
   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

all of this should be in the new release 🤔

svartalf · 2020-08-07T05:48:40Z

@flip1995 ah yes, sorry, that was an incorrect release indeed. Just published 0.1.5, can you try that one?

flip1995 · 2020-08-07T10:57:17Z

@svartalf CI looks good 👍

action.yml

svartalf · 2020-08-16T07:39:54Z

src/input.ts

 }

 export function get(): Input {
    const crate = input.getInput("crate", { required: true });
    const version = input.getInput("version", { required: true });
    const useToolCache = input.getInputBool("use-tool-cache") || false;
-    const useCache = input.getInputBool("use-cache") || true;
+    const primaryKey = input.getInput("key") || undefined;
+    const restoreKeys = input.getInputAsArray("restore-keys") || undefined;


I don't think it is a good idea to force users to provide all necessary cache keys, as Action can do that by itself; grab the crate name, version and current runner name (ex. ubuntu-18.04), join them together - we got a cache key.
That also mean that we will need to use resolveVersion if there is no version specifically set in the Action arguments or set to "latest", for example.

Ideally, from the user side I would expect to write

- uses: actions-rs/[email protected] with: crate: cargo-make

and it will handle all the stuff by itself.

My thought behind adding those keys was that there is currently no way to force-update a cache. If a new cache should be created, the cache has to get a new name. With actions/cache in rust projects, this is for example done by hashing the Cargo.lock files.

But when caching binaries I guess only naming the hash with the version number should do the trick. There should be no need to add a hash to the cache name.

Should I remove the keys completely or should I just make them optional?

The part with resolving the version is already implemented. But I still have to add the runner name.

@svartalf Do you have a suggestion how I might get the name of the runner? I looked at the @actions/github and @actions/core package, but couldn't find any functions with which I'm able to access the runner name.

If we're able to get the runner name $somehow, we can get rid of the key/restoreKeys arguments of the action and go back to useCache. This change has to be implemented in actions-rs/core though.

This depends on a change in @actions-rs/core

flip1995 · 2020-09-10T20:14:07Z

@svartalf I removed the key arguments from the action again and introduced the old useCache argument. This requires the changes in actions-rs/core#125 though. I'm not sure how to the the name of the current runner, it would be great if you could help me with that.

svartalf · 2020-09-28T13:41:28Z

@flip1995 I don't think there is any reliable and already existing way to do that, our best shot would be to use combination of process.platform and os.version probably; latter one should handle the different Ubuntu versions provided by Github.

flip1995 · 2020-10-12T12:35:42Z

@svartalf In actions-rs/core#125 I use a combination of os.platform() + '-' + os.release() + '-' + os.arch(); to determine the runner. Should I change os.release to os.version()? os.platform() is the same as process.platform IIUC.

svartalf · 2020-10-12T15:39:53Z

@flip1995 yeah, os.release() seems to be more precise, let's use it!

flip1995 · 2020-10-14T18:59:14Z

@svartalf Can you take another look at actions-rs/core#125 in combination with 57e52b9 then, please?

coltfred · 2021-01-29T00:40:03Z

Has there been further development on this? With the old tool-cache repo not updating with new binaries it's very painful.

flip1995 · 2021-01-29T09:59:26Z

This is still waiting on review of actions-rs/core#125 which then will require an update of 57e52b9

FlorianFranzen · 2022-02-04T12:28:14Z

So am I getting this right? Even though core supports cached installs for over 2 years, it has not made it into the install action, because we want core also to run os.platform() + '-' + os.release() + '-' + os.arch() for us, instead of just running it in the install action?

flip1995 · 2022-02-04T15:07:33Z

To do this cleanly without backwards breaking changes, yes a the actions-rs/core#125 PR has to be merged.

The alternative would be to merge this with a breaking change, just to revert the breaking change when doing this cleanly, which again would be a breaking change.

thomaseizinger · 2022-02-15T01:13:50Z

I just came across https://github.com/ryankurte/cargo-binstall which might be useful for other people interested in this feature.

flip1995 added 2 commits June 25, 2020 15:01

Use GitHub Actions cache for installed binaries

40cec18

Use resolveVersion from actions-rs/core

981bb2e

This was moved there, because it was also required for caching installed binaries

flip1995 mentioned this pull request Jun 25, 2020

Use GitHub Actions cache in installCached actions-rs/core#92

Merged

flip1995 changed the title ~~Install cached pr~~ Use GitHub Actions cache for installed binaries Jun 25, 2020

flip1995 commented Jun 25, 2020

View reviewed changes

src/download.ts Show resolved Hide resolved

action.yml Show resolved Hide resolved

camshaft mentioned this pull request Jul 15, 2020

ci: random fixes aws/s2n-quic#33

Merged

flip1995 added 2 commits August 7, 2020 12:44

Update package{-lock}.json

0d51c02

Update dist/index.js

9f5d8b9

flip1995 force-pushed the install_cached_pr branch from deff6bb to 9f5d8b9 Compare August 7, 2020 10:45

svartalf suggested changes Aug 16, 2020

View reviewed changes

WIP: Remove key arguments of actions

57e52b9

This depends on a change in @actions-rs/core

flip1995 mentioned this pull request Oct 12, 2020

[build] caching on Ubuntu builders of GH actions does nothing rust-lang/rust-clippy#6155

Open

coltfred mentioned this pull request Feb 26, 2021

Don't require a key to install cached program actions-rs/core#125

Open

Manishearth mentioned this pull request Jun 10, 2021

Refactor Makefile.toml and use it as the source of truth for (almost) all CI unicode-org/icu4x#783

Merged

baptiste0928 mentioned this pull request Sep 6, 2021

Improve CI cache twilight-rs/twilight#1151

Merged

ithinuel mentioned this pull request Aug 25, 2022

remove unused dependencies & add GHA flow rp-rs/rp-hal#429

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use GitHub Actions cache for installed binaries #5

Use GitHub Actions cache for installed binaries #5

flip1995 commented Jun 25, 2020 •

edited

Loading

Blisto91 commented Jul 16, 2020

flip1995 commented Jul 16, 2020

Blisto91 commented Jul 16, 2020

svartalf commented Aug 6, 2020

flip1995 commented Aug 6, 2020 •

edited

Loading

svartalf commented Aug 7, 2020

flip1995 commented Aug 7, 2020

svartalf Aug 16, 2020

flip1995 Aug 16, 2020 •

edited

Loading

flip1995 Sep 10, 2020

flip1995 commented Sep 10, 2020

svartalf commented Sep 28, 2020

flip1995 commented Oct 12, 2020

svartalf commented Oct 12, 2020

flip1995 commented Oct 14, 2020

coltfred commented Jan 29, 2021

flip1995 commented Jan 29, 2021

FlorianFranzen commented Feb 4, 2022 •

edited

Loading

flip1995 commented Feb 4, 2022

thomaseizinger commented Feb 15, 2022

Use GitHub Actions cache for installed binaries #5

Are you sure you want to change the base?

Use GitHub Actions cache for installed binaries #5

Conversation

flip1995 commented Jun 25, 2020 • edited Loading

Drawbacks

Blisto91 commented Jul 16, 2020

flip1995 commented Jul 16, 2020

Blisto91 commented Jul 16, 2020

svartalf commented Aug 6, 2020

flip1995 commented Aug 6, 2020 • edited Loading

svartalf commented Aug 7, 2020

flip1995 commented Aug 7, 2020

svartalf Aug 16, 2020

Choose a reason for hiding this comment

flip1995 Aug 16, 2020 • edited Loading

Choose a reason for hiding this comment

flip1995 Sep 10, 2020

Choose a reason for hiding this comment

flip1995 commented Sep 10, 2020

svartalf commented Sep 28, 2020

flip1995 commented Oct 12, 2020

svartalf commented Oct 12, 2020

flip1995 commented Oct 14, 2020

coltfred commented Jan 29, 2021

flip1995 commented Jan 29, 2021

FlorianFranzen commented Feb 4, 2022 • edited Loading

flip1995 commented Feb 4, 2022

thomaseizinger commented Feb 15, 2022

flip1995 commented Jun 25, 2020 •

edited

Loading

flip1995 commented Aug 6, 2020 •

edited

Loading

flip1995 Aug 16, 2020 •

edited

Loading

FlorianFranzen commented Feb 4, 2022 •

edited

Loading