SlimeVR / SlimeVR-OpenVR-Driver

SlimeVR driver for OpenVR
Apache License 2.0
204 stars 46 forks source link

[CI] Cache tag is incorrect/useless, and should be fixed. #30

Open kitlith opened 1 year ago

kitlith commented 1 year ago

not submitting a PR for this right now because there is only so much CI work I feel like doing at once. CI currently specifies the following cache key for vcpkg: ${{ hashFiles( 'vcpkg_manifest/vcpkg.json' ) }}-${{ hashFiles( '.git/modules/vcpkg/HEAD' )}}-${{ hashFiles( '${{ env.VCPKG_ROOT }}/.git/HEAD' )}}-${{ matrix.triplet }}-invalidate

for each variable used from left to right:

All in all this results in us generating a cache key of ---x64-windows-static-md-invalidate as of the current latest commit. Given that cache entries are immutable, we're not actually tracking most of the variables that should be invalidating the cache. whoops.

How to fix

  1. An easy mitigation is to point to the correct vcpkg.json. That way, it'll update when we add new libraries, and we can always bump something in vcpkg.json if we notice that cache is stale.
  2. We can use a github action like lukka/run-vcpkg to handle the vcpkg cache for us, so that we don't have to embed the requirements into our CI script. One less thing to worry about.
  3. We can use vcpkg as a submodule, as I do in the feeder app. Unfortunately I can't find the documentation that led me to believe that this was a good idea at the moment.
    • This would also result in users not having to explicitly setup vcpkg on their machines, just run cmake and the rest is more or less handled.
    • Currently we only use vcpkg for a single dependency, but we could move our other dependencies into vcpkg, or maybe we could decide on a different way to depend on protobuf.