search

subosito / flutter-action

Flutter environment for use in GitHub Actions. It works on Linux, Windows, and macOS.
MIT License
2.17k stars 193 forks source link

Update cache-key with `Runner.os` and `Arch` #152

Closed KKimj closed 2 years ago

KKimj commented 2 years ago

Hello.. This PR will be the last.

How about add ${{ runner.os }} on cache-key ?

Thanks. Take Care!!

kuhnroyal commented 2 years ago

This looks important :o Do we also need to include the architecture?

KKimj commented 2 years ago

@kuhnroyal Yeah, I think it would be better that way. In fact, since there is no arm64 environment at actions, it may feel dubious.

yurikoles commented 2 years ago

Yes, there are no GitHub-hosted arm64 runners now, but one may use a self-hosted one, see #147.

KKimj commented 2 years ago

@yurikoles Oh.. Sorry.. I missed it. Thanks you for pointing out it. In that case, it would be reasonable to introduce arch.

KKimj commented 2 years ago

How about like below?

${{ inputs.cache-key }}-${{ runner.os }}-${{ runner.arch }}-${{ inputs.channel }}-${{ inputs.flutter-version }}
subosito commented 2 years ago

@KKimj Yeah, I think that's better

kuhnroyal commented 2 years ago

Use the ${{ inputs.architecture }}? It seems to have a sane default.

KKimj commented 2 years ago

@kuhnroyal I agree with you. I think there is a conner case something like below

Is it reasonable? @subosito

subosito commented 2 years ago

I think it's ok, but it would be better if we could set the default by using the existing value given by GitHub runner, like RUNNER_ARCH or runner.arch if possible. What do you think, guys?

KKimj commented 2 years ago

@subosito It makes sense! Nevertheless, In my opinion, it looks better to maintain.

KKimj commented 2 years ago

As It is an edge case, and without a self-hosting machine, there is no good way to test with github action.

KKimj commented 2 years ago

@subosito here we go!