subosito / flutter-action

Flutter environment for use in GitHub Actions. It works on Linux, Windows, and macOS.
MIT License
2.17k stars 193 forks source link

commit 9d48f4e no longer resolves environment variables passed in #184

Closed jorgenpt closed 1 year ago

jorgenpt commented 1 year ago

Running the flutter-action after commit 9d48f4e fails (tested on a ubuntu-latest runner):

tar: This does not look like a tar archive
tar: Exiting with failure status due to previous errors
stable:$FLUTTER_VERSION:x64|null:null:x64

It looks like it's no longer resolving the $FLUTTER_VERSION variable I pass in.

Also worth noting that this tar failure did not fail the action, so the actual failure was the next step trying to run flutter pub get and getting /home/runner/work/_temp/10150038-ae3e-4950-be3c-f4528f6fab2e.sh: line 1: flutter: command not found

Here's the full log output

Here's the the relevant workflow snippet:

      - name: Setup flutter
        uses: subosito/flutter-action@v2
        with:
          cache: true
          flutter-version: $FLUTTER_VERSION

If I lock the action version to the prior commit with the following snippet, it works as expected:

      - name: Setup flutter
        uses: subosito/flutter-action@6c2e035f2692eeac890d854df95630c72673f130
        with:
          cache: true
          flutter-version: $FLUTTER_VERSION
jorgenpt commented 1 year ago

Further investigation reveals that changing the syntax to flutter-version: ${{ env.FLUTTER_VERSION }} seems to address this. Presumably the old behavior would expand environment variables due to how it invoked setup.sh (It now uses single quotes around the arguments.)

I think that constitutes a breaking change and therefore a major version bump, or having the old behavior restored (though I originally wrote my code thinking GH Workflows would expand them before forwarding them to the action, so I understand why you would change your behavior).

subosito commented 1 year ago

Oops, sorry, @jorgenpt, I am not aware of this use case, but it seems better to use ${{ env.FLUTTER_VERSION }} rather than passing the environment variable name directly. What do you think? Is it OK if we stick to this behavior?

jorgenpt commented 1 year ago

It depends on how important breakage is to you -- I've solved my problem by using the (more correct) syntax, but this could theoretically affect other projects (Like simplio-app above). :)

subosito commented 1 year ago

Okay, let's close this issue for now. Hopefully, the solution you posted is sufficient enough to handle similar cases. Thanks, @jorgenpt!