openrewrite / rewrite-github-actions

OpenRewrite recipes for performing GitHub action hygiene and migration tasks.
Apache License 2.0
9 stars 9 forks source link

Prefer `actions/setup` caching over `actions/cache` #47

Open yeikel opened 1 year ago

yeikel commented 1 year ago
  1. Remove actions/cache
  2. Add the cache key to the corresponding actions/setup

See : https://github.com/actions/setup-java#caching-packages-dependencies https://github.com/actions/setup-node#caching-packages-dependencies https://github.com/actions/setup-python https://github.com/actions/setup-go#caching-dependency-files-and-build-outputs

Examples:


diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml
index 4555776..785bc33 100644
--- a/.github/workflows/build.yml
+++ b/.github/workflows/build.yml
@@ -17,16 +17,11 @@ jobs:
       checks: write
     steps:
       - uses: actions/checkout@v3
-      - uses: actions/cache@v3
-        with:
-          path: ~/.m2/repository
-          key: ${{ runner.os }}-main-maven-${{ hashFiles('**/pom.xml') }}
-          restore-keys: |
-            ${{ runner.os }}-main-maven-
       - uses: actions/setup-java@v3
         with:
           java-version: 8
          distribution: 'zulu'
+          cache: 'maven'
       - run: cd tests && mvn clean test --batch-mode -Dmaven.test.failure.ignore=true
       - uses: ./
         if: github.ref != 'refs/heads/master'

diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml
index 785bc33..e2d8a91 100644
--- a/.github/workflows/build.yml
+++ b/.github/workflows/build.yml
@@ -52,12 +52,7 @@ jobs:
       - uses: actions/setup-node@v3
         with:
           node-version: 16
-      - uses: actions/cache@v3
-        with:
-          path: ~/.npm
-          key: ${{ runner.os }}-npm-cache-${{ hashFiles('**/package-lock.json') }}
-          restore-keys: |
-            ${{ runner.os }}-npm-cache-
+          cache: npm
       - run: npm install
       - run: npm run eslint
       - run: npm run test

I think that the main challenge here is to determine what type to use.

For example, for actions/setup-java we have maven, gradle and sbt as options. The recipe should be able to detect the package manager and make the correct selection

yeikel commented 1 year ago

@timtebeek If you think it makes more sense to split it per setup type, we can try creating separate issues for each

timtebeek commented 1 year ago

I'm trying to think through how a change like this would align with our "do no harm" convention. While there's definitely an improvement and simplification to switch to setup type caching, it's hard to reliably cover all cases where users have made subtle changes to cache keys and paths, and how they would compare the the setup type cache keyword. What are your thoughts on that @yeikel ?

yeikel commented 1 year ago

I'm trying to think through how a change like this would align with our "do no harm" convention. While there's definitely an improvement and simplification to switch to setup type caching, it's hard to reliably cover all cases where users have made subtle changes to cache keys and paths, and how they would compare the the setup type cache keyword. What are your thoughts on that @yeikel ?

I think that's a fair point. We definitely shouldn't make this replacement blindly as there are custom usages

Perhaps, we can start with simple base examples?

For example :

For maven, a common pattern is :


path: ~/.m2/repository
key: ${{ runner.os }}-main-maven-${{ hashFiles('**/pom.xml') }}
restore-keys: |
            ${{ runner.os }}-main-maven-

There is nothing special about this example and the default caching feature can cover it

timtebeek commented 1 year ago

Just thinking out loud here; we can possibly use the new data tables feature to explore common patterns in a sizable set of projects, and then target the top x for transformation. It limits the applicability of this recipe, but ensures we don't make any unexpected changes. Would work best in a larger organization with some standardization I suppose, as opposed to in the wild on projects that do not share a common base.