search

DeLaGuardo / setup-clojure

GitHub Action to provision clojure's most popular build tools for Linux, Mac OS X and Windows.
MIT License
184 stars 27 forks source link

Add arch to cache key #72

Open cap10morgan opened 2 years ago

cap10morgan commented 2 years ago

I was still seeing the x86-64 version installed in my self-hosted arm64 runner when using version 9.5, even after destroying and recreating it. So it seemed like a caching thing.

cap10morgan commented 2 years ago

Hmm... even telling it to use this commit in my workflow I'm still getting the x86-64 version installed. I'll keep investigating.

cap10morgan commented 2 years ago

Well, I guess it isn't actually using this commit after all. With debug logging turned on, the arch still isn't present in the cache key. So this might be the fix after all, I just need to make sure I'm actually using it in my testing. Maybe if I point it at my fork instead.

cap10morgan commented 2 years ago

It helps if I point it at a commit where I've updated the compiled JS in dist/ :)

cap10morgan commented 2 years ago

It also seems like the invalidate-cache: true param isn't invalidating the cache? It still says it's restoring from the cache even with that present.

cap10morgan commented 2 years ago

OK, I had to manually delete the cache dir _work/_tool/Babashka (even with invalidate-cache: true) but once I did that w/ this code it did finally install the arm64 version of bb. So this PR is ready to merge if you're good w/ it @DeLaGuardo.

Want me to open an issue about invalidate-cache not working?

cap10morgan commented 2 years ago

The invalidate-cache issue is likely specific to self-hosted runners. I imagine a lot of this stuff is ephemeral in GitHub-provided runners whereas self-hosted runners seem to persist the tool cache, for example. So skipping the cache restores when invalidate-cache is true is not sufficient because the cache is usually already there in its restored location on self-hosted runners. I can write up an issue and open a separate PR for that.

cap10morgan commented 2 years ago

@DeLaGuardo Any thoughts on this and #74?