search

nienbo / cache-buildkite-plugin

Tarball, Rsync & S3 Cache Kit for Buildkite. Supports Linux, macOS and Windows
https://buildkite.com/plugins
MIT License
67 stars 39 forks source link

Improve templating support #9

Closed djmarcin closed 4 years ago

djmarcin commented 4 years ago

The existing logic assumes that a checksum template will occur last in the cache key and ignores everything after the first template block. This isn't really mentioned as a limitation in the documentation, and means that a key specified like v1-cache-{{ checksum 'foo.lock' }}-{{ checksum 'bar.lock' }}-some-more-stuff actually translates to just the result of v1-cache-{{ checksum 'foo.lock' }}.

This PR changes the logic to evaluate all template blocks and preserves anything after the last template block. It also makes unsupported template blocks an error. Previously, these were passed through untouched, but that could hide bugs like this typo {{ cheksum 'foo.lock' }} which would silently fail to compute a checksum. Therefore, this PR is conceivably a breaking change for some folks relying on unsupported template blocks to be passed through literally, but this seems unlikely to be the desired behavior.

This PR is built on top of https://github.com/gencer/cache-buildkite-plugin/pull/8 since it touches the same lines.

gencer commented 4 years ago

Awesome! Thanks for the PR 🎉

I assume you've tested this on both macOS and Linux.

P.S.: Did you tested this template against Directory? v1-cache-{{ checksum 'app/dir' }}-some-more-stuff

djmarcin commented 4 years ago

I was just working on some tests, will send another PR.