Bazel example (Take #2️⃣)

[vorburger]

@PiotrSikora how about this re. #575 ?

How Has This Been Tested?

I'm using this in a project and as far as I can tell so far it seems to work.

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)
• Documentation (add or update README or docs)

Checklist:

• My code follows the code style of this project.
• My change requires a change to the documentation.
• I have updated the documentation accordingly.
• I have read the CONTRIBUTING document.
• I have added tests to cover my changes.
• All new and existing tests passed.

[vorburger]

@davidB FYI this is a follow-up to your original #575.

[PiotrSikora]

I'd argue that we shouldn't do partial restore with Bazel (at least not that wide), since most of the restored files won't be used anyway, and they are going to be saved in the new cache, wasting cache space and increasing restore times.

[vorburger]

@vsvipul I'm reluctant to make changes to a PR after a LGTM.. before I do, could you comment if you want this to be removed or not? Personally I'm not sure I see the harm in this being there, and in my project it does seem to help quite a bit to speed up things on small dependency changes. It also does not seem consistent, to me, for Bazel to be the only example where there is not restore-keys? Isn't what @PiotrSikora alludes to something that could be said for all examples and that is not specific to Bazel per se?

[PiotrSikora]

Skimming through examples, deno doesn't have restore-keys either.

But if you want to retain it here, I'd probably add more layers of restore-keys, e.g.:

restore-keys: |
    ${{ runner.os }}-bazel-${{ hashFiles('.bazelversion', '.bazelrc', 'WORKSPACE', 'WORKSPACE.bazel') }}
    ${{ runner.os }}-bazel-${{ hashFiles('.bazelversion', '.bazelrc') }}
    ${{ runner.os }}-bazel-${{ hashFiles('.bazelversion') }}
    ${{ runner.os }}-bazel-

[vsvipul]

Its upto you @vorburger . But in most cases we don't include the hashFiles in restore-keys the key with the hashFiles is already accounted for in an exact match. Also, in some cases we don't keep restore-keys where the restored cache might not be very useful. I don't have much clue about bazel so can't comment on that. would leave the decision to you.

[PiotrSikora]

Sorry, the restore-keys above is wrong, since those hashFiles would all evaluate to different hashes, even if the other files didin't change, and there would be no partial match. It should be this instead:

key: ${{ runner.os }}-bazel-${{ hashFiles('.bazelversion') }}-${{ hashFiles('.bazelrc') }}-${{ hashFiles('WORKSPACE', 'WORKSPACE.bazel') }}-${{ hashFiles('MODULE.bazel') }}
restore-keys: |
    ${{ runner.os }}-bazel-${{ hashFiles('.bazelversion') }}-${{ hashFiles('.bazelrc') }}-${{ hashFiles('WORKSPACE', 'WORKSPACE.bazel') }}
    ${{ runner.os }}-bazel-${{ hashFiles('.bazelversion') }}-${{ hashFiles('.bazelrc') }}
    ${{ runner.os }}-bazel-${{ hashFiles('.bazelversion') }}
    ${{ runner.os }}-bazel-

[vorburger]

Piotr, I believe I now finally better understand what you are after. You mean that there is a sort of "hierarchy" involved here, yes? Like this, a .bazelversion change causes the entire cache to be thrown away (which would make sense to me as well, yes), but a .bazrlrc version causes some cache reuse, etc.? That's a nice idea - but I'm personally not familiar enough with the cache action to be sure that this will really work like this. Vipul, thoughts?

[vsvipul]

The thing is that i need to know when each of these files change '.bazelversion', '.bazelrc', 'WORKSPACE', 'WORKSPACE.bazel', 'MODULE.bazel' to allow some kind of reuse for cache and I have never used bazel myself so i'm hesitant to merge something which I'm not sure about. I feel the current state of the PR is good enough for normal users.

[PiotrSikora]

SGTM, I don't mind if this gets merged as-is. Thanks!

[vorburger]

@vsvipul can I do anything else to now see this get merged as-is?

[vsvipul]

no. this lgtm.

[PiotrSikora]

Same here.

[vsvipul]

Merged. Thank you for contributing. 🎊 🎉