Repository navigation
feat: don't overwrite flutter if correct version already present - #359
RyanCarrier wants to merge 7 commits into
Conversation
|
Hi, thanks for the PR. If I understand correctly, this adds a new |
|
Nope the |
| if [ -d "${{ steps.flutter-action.outputs.CACHE-PATH }}" ]; then | ||
| echo "Cache path already exists, skipping cache step." | ||
| echo "flutter-action-cache-exists=true" >> "$GITHUB_OUTPUT" | ||
| else | ||
| echo "flutter-action-cache-exists=false" >> "$GITHUB_OUTPUT" | ||
| fi |
There was a problem hiding this comment.
Instead of setting an env var, let's add a new value to outputs of this action (starting at line 54 in this file).
Then please also update the README so that others can learn about this.
|
Can we get this moving forward somehow? This would help a lot on self-hosted runners. |
|
The original author seems to have lost interest; feel free to create a new PR @kuhnroyal |
|
I guess we can just set |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@action.yaml`:
- Around line 79-81: The CACHE-EXISTS output can be empty when the check-cache
step is skipped; update the action outputs so CACHE-EXISTS always yields a
boolean-like value by providing a fallback when inputs.cache != 'true' or the
step didn't run (e.g., set the outputs.CACHE-EXISTS value expression to use
inputs.cache and/or a logical fallback of 'false' referencing the check-cache
step), or alternatively document that CACHE-EXISTS is only meaningful when
inputs.cache == 'true'; adjust the outputs entry for CACHE-EXISTS (and refer to
the check-cache step and inputs.cache) accordingly.
- Around line 113-124: The current check in step id "check-cache" only tests -d
on "${{ steps.flutter-action.outputs.CACHE-PATH }}", which can misidentify stale
or corrupted SDK directories as valid; change the test to verify a known SDK
artifact exists (for example check for "${{
steps.flutter-action.outputs.CACHE-PATH }}/bin/flutter" and that it's a regular
file or executable) and write CACHE-EXISTS=true only when that file passes the
existence/test, otherwise set CACHE-EXISTS=false to force a restore; keep the
same step id and GITHUB_OUTPUT usage so downstream logic unchanged.
In `@README.md`:
- Line 341: The examples print steps.flutter-action.outputs.CACHE-EXISTS but
never enable caching, so CACHE-EXISTS will always be empty; update the shown
workflow examples to include cache: true in the with: block of the
flutter-action (and ensure the check-cache step remains defined) so the
check-cache step runs and populates outputs, or alternatively add a short note
next to the echo lines clarifying that CACHE-EXISTS is only populated when
cache: true is set.
🧹 Nitpick comments (2)
action.yaml (2)
126-131: Skippingactions/cachealso skips cache saving.
actions/cacheboth restores the cache at the start and saves it in a post-job hook. WhenCACHE-EXISTS == 'true', the step is entirely skipped, which means a locally-present SDK will never be uploaded to GitHub's cache store. This is fine for self-hosted runners (the intended use case), but on a hosted runner if the directory somehow exists without a corresponding GitHub cache entry, it will never get cached.Worth documenting this trade-off, at minimum. Alternatively, you could consider using
actions/cache/restorehere and adding a separateactions/cache/savestep (or relying on the post-action hook) to decouple restore and save behavior.
113-113: Nit: misplaced comment.The comment
# only run if cache path doesn't already existon Line 113 appears to be a leftover inline comment that's slightly misplaced—it sits after the previous step'srunblock. It would read more cleanly as part of the step'snamefield or moved directly above the- name:line without the extra blank line.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@action.yaml`:
- Around line 127-132: The cache step skips both restore and save when
steps.check-cache.outputs.LOCAL-CACHE-HIT == 'true', which can allow a stale SDK
if a custom cache path (steps.flutter-action.outputs.CACHE-PATH) doesn’t include
the Flutter version; add a lightweight validation after restoring/skipping
restore that compares the resolved version
(steps.flutter-action.outputs.VERSION) to the actual SDK version (e.g., run
$FLUTTER_BIN --version or read the SDK root "version" file) and if they differ,
force a cache miss/save path or fail fast so the workflow restores the correct
SDK; update the action flow around the actions/cache@v4 step to invoke this
check and conditionally trigger a restore/save when the versions disagree.
|
Sorry guys, I have too many subscribed issues and your messages just got lost. I've also just been using my branch and so haven't been worried about the issue Let me know if there are other changes needed |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@action.yaml`:
- Around line 134-140: The cache step (id: cache-flutter) is currently skipped
when steps.check-cache.outputs.LOCAL-CACHE-HIT == 'true', which prevents
actions/cache@v4's post-job save from running; change the step condition to only
check inputs.cache == 'true' (remove the LOCAL-CACHE-HIT check) so the cache
action always runs when caching is enabled and its post-job save can run, and
add a short comment documenting that self-hosted runners may have a local SDK
that needs the post-job save to populate the GitHub cache.
| - name: Cache Flutter | ||
| id: cache-flutter | ||
| uses: actions/cache@v4 | ||
| if: ${{ inputs.cache == 'true' }} | ||
| if: ${{ inputs.cache == 'true' && steps.check-cache.outputs.LOCAL-CACHE-HIT == 'false' }} | ||
| with: | ||
| path: ${{ steps.flutter-action.outputs.CACHE-PATH }} | ||
| key: ${{ steps.flutter-action.outputs.CACHE-KEY }} |
There was a problem hiding this comment.
Skipping actions/cache also skips the post-job save.
When LOCAL-CACHE-HIT is true, the entire actions/cache@v4 step is skipped — including the post-job save phase. This means that if the local SDK exists but hasn't been saved to the GitHub Actions cache yet (e.g., first run on a self-hosted runner), it will never be saved. This is likely acceptable for self-hosted runners (the primary use case), but worth documenting so users understand the behavior.
🤖 Prompt for AI Agents
In `@action.yaml` around lines 134 - 140, The cache step (id: cache-flutter) is
currently skipped when steps.check-cache.outputs.LOCAL-CACHE-HIT == 'true',
which prevents actions/cache@v4's post-job save from running; change the step
condition to only check inputs.cache == 'true' (remove the LOCAL-CACHE-HIT
check) so the cache action always runs when caching is enabled and its post-job
save can run, and add a short comment documenting that self-hosted runners may
have a local SDK that needs the post-job save to populate the GitHub cache.
|
@bartekpacia The failing workflows are due to macos-13 - I fixed them in my other PR. |
|
Thanks for getting back to us @RyanCarrier. I'll close this PR, feel free to create a new one if you want and have more time in the future :) |
Just a simple one to make my small runs on self hosted runners go faster.
CACHE_PATH includes the version so we only need to check that. The only concern would be if they set their own cache path
Summary by CodeRabbit
New Features
Documentation