install icon indicating copy to clipboard operation
install copied to clipboard

Use GitHub Actions cache for installed binaries

Open flip1995 opened this issue 4 years ago • 17 comments

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.

flip1995 avatar Jun 25 '20 14:06 flip1995

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?

Blisto91 avatar Jul 16 '20 17:07 Blisto91

@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:

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

flip1995 avatar Jul 16 '20 17:07 flip1995

@flip1995 Sounds awesome!

Hope it gets merged soon :).

Blisto91 avatar Jul 16 '20 17:07 Blisto91

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

svartalf avatar Aug 06 '20 20:08 svartalf

@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 🤔

flip1995 avatar Aug 06 '20 21:08 flip1995

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

svartalf avatar Aug 07 '20 05:08 svartalf

@svartalf CI looks good 👍

flip1995 avatar Aug 07 '20 10:08 flip1995

@svartalf I removed the key arguments from the action again and introduced the old useCache argument. This requires the changes in https://github.com/actions-rs/core/pull/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.

flip1995 avatar Sep 10 '20 20:09 flip1995

@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.

svartalf avatar Sep 28 '20 13:09 svartalf

@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.

flip1995 avatar Oct 12 '20 12:10 flip1995

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

svartalf avatar Oct 12 '20 15:10 svartalf

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

flip1995 avatar Oct 14 '20 18:10 flip1995

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

coltfred avatar Jan 29 '21 00:01 coltfred

This is still waiting on review of actions-rs/core#125 which then will require an update of https://github.com/actions-rs/install/pull/5/commits/57e52b98fb97b1070e51638bed5ebd3e8e32c0e3

flip1995 avatar Jan 29 '21 09:01 flip1995

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?

FlorianFranzen avatar Feb 04 '22 12:02 FlorianFranzen

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.

flip1995 avatar Feb 04 '22 15:02 flip1995

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

thomaseizinger avatar Feb 15 '22 01:02 thomaseizinger