GitHub: Add cache-dependency-path

[muuki88]

I noticed that on my PR no caching is taking place.

🔧 Type of changes

• bugfix

✨ What's the context?

https://github.com/prebid/prebid-server-java/pull/3613/checks shows

Creating settings.xml with server-id: github
Writing to /home/runner/.m2/settings.xml
maven cache is not found

🧠 Rationale behind the change

Make PR tests faster

[muuki88]

Seems to work now:

[Net-burst]

Looking into this one. Sorry for taking so long.

[Net-burst]

So, I did a small investigation and learned that the cache is working as intended. With the intention itself being somewhat weird for our use case... And sometimes outright not working at all...

Here's what I mean. If you look at your example PR, you will notice that cache failed for the first check run (and for some reason for the second run too) but succeeded on the subsequent ones. At least for some tasks. It's just weird by itself. What you're proposing is to limit the change check to only one file instead of **/pom.xml. Which shouldn't really fix anything unless there's even more obscure bug in one of the global GitHub action dependencies which computes the cache key.

Here's the part of the Action code that is impacted by this property: https://github.com/actions/setup-java/blob/7a6d8a8234af8eb26422e24e3006232cccaa061b/src/cache.ts#L88
Please notice this part of the code:

  const pattern = cacheDependencyPath
    ? cacheDependencyPath.trim().split('\n')
    : packageManager.pattern;

Where packageManager.pattern equals to:

  {
    id: 'maven',
    path: [join(os.homedir(), '.m2', 'repository')],
    // https://github.com/actions/cache/blob/0638051e9af2c23d10bb70fa9beffcad6cff9ce3/examples.md#java---maven
    pattern: ['**/pom.xml']
  },

TLDR: caching is borked by itself, we'll need to investigate it a bit more before we try any hacks.

[muuki88]

Hi @Net-burst

No worries 😄 Thanks for taking the time.

Here's what I mean. If you look at your example PR, you will notice that cache failed for the first check run (and for some reason for the second run too) but succeeded on the subsequent ones. At least for some tasks. It's just weird by itself.

If this is the order, than this possible. The cache is populated for the first time and could not be available due to eventually consistency or tasks running in parallel for the some tasks.

What you're proposing is to limit the change check to only one file instead of **/pom.xml. Which shouldn't really fix anything unless there's even more obscure bug in one of the global GitHub action dependencies which computes the cache key.

Using a pattern **/pom.xml doesn't guarantee any order. This is maybe a sensible default for a project that has only a single pom.xml , but for this project it may yield the different pom.xml files in different orders on each run.

I wouldn't say that limiting the cache key to a single pom.xml that encompasses all dependencies is a workaround. It's just very explicitly stating where all dependencies are declared. This ensure a consistent cache key and consistent cache itself.

[Net-burst]

Yeah, looks like you are right. That thing is asynchronous so hashing will happen in any order... And I have no knowledge about how crypto is implemented in Node and whether ordering matters...

Let's merge your workaround and see whether this will solve the caching. Although during the investigation I learned that cache is per-branch. Which means that every first run will be with cold caches... Which is meh, but it's better than nothing.

[Net-burst]

LGTM

[muuki88]

Although during the investigation I learned that cache is per-branch. Which means that every first run will be with cold caches... Which is meh, but it's better than nothing.

I guess this is not entirely correct. This is from the github caching docs

Workflow runs can restore caches created in either the current branch or the default branch (usually main). If a workflow run is triggered for a pull request, it can also restore caches created in the base branch, including base branches of forked repositories

So pull requests should use the cache from the master branch, if available. So it should mostly work 🎉