Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Generalize publishing GitHub Action to support flutter #68

Closed
guidezpl opened this issue Feb 2, 2023 · 23 comments · Fixed by widgetbook/widgetbook#713 or #141
Closed

Generalize publishing GitHub Action to support flutter #68

guidezpl opened this issue Feb 2, 2023 · 23 comments · Fixed by widgetbook/widgetbook#713 or #141
Assignees

Comments

@guidezpl
Copy link

guidezpl commented Feb 2, 2023

The action to publish packages doesn't work for Flutter packages due to the following error.

Run dart pub get
  dart pub get
  shell: /usr/bin/bash -e {0}
  env:
    PUB_CACHE: /home/runner/.pub-cache
    DART_HOME: /opt/hostedtoolcache/dart-sdk
    PUB_TOKEN: ***
Resolving dependencies...
Because google_fonts requires the Flutter SDK, version solving failed.

https://github.com/material-foundation/flutter-packages/actions/runs/4079639516/jobs/7031216899

Can this action be generalized or another one published for Flutter?

@guidezpl
Copy link
Author

guidezpl commented Feb 3, 2023

@devoncarew is dart pub get required?

@bartekpacia
Copy link

bartekpacia commented Feb 9, 2023

I had the same/similar problem after updating to Flutter 3.7.

Something has changed and while previously dart pub publish could be used to publish Flutter packages, it doesn't work anymore. I migrated my package to use flutter pub publish to fix the problem.

@jonasfj
Copy link
Member

jonasfj commented Feb 10, 2023

@sigurdm, I think the fact that we patched dart pub publish to do dart analyze and implicit dart pub get, cause the regression that @bartekpacia mentioned.

On topic, I think it would be great if the reusable-workflow could be used to publish flutter packages with:
https://github.com/dart-lang/setup-dart/blob/main/.github/workflows/publish.yml

I really would love to recommend that people use the reusable workflow as it'll let us patch the workflow after they've adopted it. So in theory we could potentially add SLSA support in the future.


@sigurdm, any ideas? should we extend the reusable-workflow to install Flutter, when there is an SDK dependency on Flutter?

I'm not keep on patching the community maintained flutter-action repository to have reusable-workflow, because we would be unable to trust such a workflow on pub.dev -- should ever want to do thing that requires trust -- like inspecting github APIs to check if branch protection is enabled, etc, stuff that SLSA does.

@sigurdm
Copy link
Contributor

sigurdm commented Feb 13, 2023

should we extend the reusable-workflow to install Flutter, when there is an SDK dependency on Flutter?

I don't currently see other good ways to go around this. The only alternative would be to disable the resolution in the publish-step (as we cannot resolve against sdk dependencies without the sdk).

@jonasfj
Copy link
Member

jonasfj commented Feb 13, 2023

@sigurdm, we could consider having a dart pub publish --skip-validation thing.

In practice, if you're publishing with automation, you really should put all of your unit tests ahead of the actual publishing step..

Or we should make the custom workflow install Flutter, I'm not sure if that'd be a huge issue.
It's just downloading and extracting the Flutter SDK. We don't have to make setup-dart do that in general, just .github/workflows/publish.yml

@devoncarew
Copy link
Member

I think it's a bit non-intuitive to have the setup-dart action install a flutter sdk. Is there some way to compose this with the existing actions which install a flutter sdk?

@devoncarew
Copy link
Member

We don't have to make setup-dart do that in general, just .github/workflows/publish.yml

Perhaps having a separate .github/workflows/publish-flutter.yml script might be enough of an indication that this could install flutter specific bits.

@sigurdm
Copy link
Contributor

sigurdm commented Feb 14, 2023

Why doesn't setup-dart know how to setup flutter?

@sigurdm
Copy link
Contributor

sigurdm commented Feb 14, 2023

We discussed this a bit on the weekly pub-meeting.
I think from the pub side of things:

  • we would prefer a single workflow for publishing (to avoid confusion and differences sneaking in)
  • we don't want to implement a pub publish --ignore-warnings (that would lead to broken publications) -> we actually need to install flutter
  • we don't want to run community code (not clear that flutter-actions will always do the right thing)

So that looks like having a dash-team script that can download and install Flutter is needed.

  • We think it would be enough to default to latest stable, with a manual parameter for the channel and version.
  • This can probably be done with a curl and tar invocation.
  • If we install Flutter, we might not need to install the Dart SDK, but can use the one embedded in a flutter-sdk (this might seem like a premature optimization, but otherwise we get the question of choosing the right sdk for publishing a package).

@jonasfj
Copy link
Member

jonasfj commented Feb 14, 2023

we don't want to run community code (not clear that flutter-actions will always do the right thing)

More importantly, it'll be hard to do SLSA in the future.

we would prefer a single workflow for publishing (to avoid confusion and differences sneaking in)

If we decided to do a github.com/flutter/setup-flutter repository, then perhaps it wouldn't be bad to put the logic in there..

Otherwise, perhaps it's find if we just extend publish.yml to support Flutter, whether we always download Flutter or not won't matter much -- in practice this only runs with publishing, which shouldn't be frequent.

@guidezpl
Copy link
Author

Otherwise, perhaps it's find if we just extend publish.yml to support Flutter, whether we always download Flutter or not won't matter much -- in practice this only runs with publishing, which shouldn't be frequent.

+1. If there ever is a flutter/setup-flutter repo and publishing workflow, its workflow could simply depend on publish.yml.

@bartekpacia
Copy link

bartekpacia commented Feb 17, 2023

I'd like to add that flutter pub publish doesn't play well with Automated Publishing with GitHub Actions on pub.dev.

The most widely used subosito/flutter-action doesn't support pulling the pub.dev token, so doing flutter pub publish fails on CI.

The result is that I have to use dart-lang/setup-dart, and then subosito/flutter-action to be able to use flutter pub publish.

See this commit for an example of how I fixed it.

Not a good experience, if you ask me.

@devoncarew
Copy link
Member

we might not need to install the Dart SDK, but can use the one embedded in a flutter-sdk

Keep in mind that the dart: APIs supported by the regular Dart SDK, and the Dart SDK vended into the Flutter SDK, are not the same. The Flutter version of the Dart SDK adds dart:ui and removes dart:mirrors (and several web core libraries). You can use the one to do publishing for the other, as long as you don't want to do something like run static analysis.

The main issue is that dart pub publish now wants to run static analysis as part of publishing validation? It may be worth considering making that portion of the checks opt-in (e.g., dart pub publish --run-analysis-checks).

@IchordeDionysos
Copy link

Can't the flow detect whether the package to be published is a Flutter package and if yes download Flutter and do everything with flutter ... otherwise use the current workflow and with dart ...?

@sigurdm
Copy link
Contributor

sigurdm commented Jun 6, 2023

It may be worth considering making that portion of the checks opt-in (e.g., dart pub publish --run-analysis-checks).

We landed a new option for opting out of all validations --skip-validations: dart-lang/pub#3935

TimBaumgart added a commit to TimBaumgart/flutter_map_polywidget that referenced this issue Aug 31, 2023
m0nac0 added a commit to maplibre/flutter-maplibre-gl that referenced this issue Dec 21, 2023
The first attempt failed because of
dart-lang/setup-dart#68. This uses the
--skip-validation option recommended in that issue.

Apparently the workflow recommended by pub.dev actually doesn't work
with Flutter plugins because it tries to do some client side validations
that fail because the workflow does not setup Flutter...
m0nac0 added a commit to maplibre/flutter-maplibre-gl that referenced this issue Dec 21, 2023
The first attempt failed because of
dart-lang/setup-dart#68. This uses the
--skip-validation option recommended in that issue.

Apparently the workflow recommended by pub.dev actually doesn't work
with Flutter plugins because it tries to do some client side validations
that fail because the workflow does not setup Flutter...
@Gustl22
Copy link

Gustl22 commented Dec 22, 2023

We landed a new option for opting out of all validations --skip-validations

Can we add this option to the publish workflow?

@clragon
Copy link

clragon commented Jan 11, 2024

This is also an issue I am having right now.
I cannot publish my flutter package with the reusable workflow, as it doesnt use that flag.

@eseidel
Copy link

eseidel commented Jul 25, 2024

@eseidel
Copy link

eseidel commented Jul 25, 2024

I have a working setup now:

https://github.com/shorebirdtech/updater/blob/f37fee86a9fb1849d8a15da267a2de062d9e17d8/.github/actions/publish_flutter_package/action.yaml

The crux is:

    - name: 📚 Git Checkout
      uses: actions/checkout@v4
    - name: 🐦 Setup Flutter
      uses: subosito/flutter-action@v2

    - name: 🪪 Get Id Token
      uses: actions/github-script@v6
      with:
        script: |
          let pub_token = await core.getIDToken('https://pub.dev')
          core.exportVariable('PUB_TOKEN', pub_token)

    - name: 📢 Authenticate
      shell: ${{ inputs.shell }}
      run: flutter pub pub token add https://pub.dev --env-var PUB_TOKEN

Careful, what tripped me up for a while was that https://pub.dev/ != https://pub.dev, I had a trailing slash in my token audience request by accident which was getting an invalid token.

rpelavin added a commit to pelavin/square-web-payments that referenced this issue Aug 30, 2024
@git-elliot
Copy link

git-elliot commented Nov 17, 2024

I am a new contributor who attempts to add a flutter_publish.yml workflow in my fork - #139

But when I run this workflow in my github actions it doesn't pass authentication.

Validating package...
Pub needs your authorization to upload packages on your behalf.
In a web browser, go to https://accounts.google.com/o/oauth2/auth?access_type=offline&approval_prompt=force&response_type=code&client_id=818368855108-8grd2eg9tj9f38os6f1urbcvsq399u8n.apps.googleusercontent.com&redirect_uri=http%3A%2F%2Flocalhost%3A33935&code_challenge=nuG0mkGHs1kpqcsxkZdt0qQaX6zdhbInIiFrgAC9ILw&code_challenge_method=S256&scope=openid+https%3A%2F%2Fwww.googleapis.com%2Fauth%2Fuserinfo.email
Then click "Allow access".
Waiting for your authorization...

@sigurdm
Copy link
Contributor

sigurdm commented Nov 18, 2024

Seems like we can have conditional steps: https://docs.github.com/en/actions/writing-workflows/choosing-when-your-workflow-runs/using-conditions-to-control-job-execution

We could make an input "publish-with-flutter"

with:
  publish-with-flutter: true

And if that is true we invoke some setup-flutter action instead of setup-dart...

That would be slightly manual, but perhaps good enough

@sigurdm
Copy link
Contributor

sigurdm commented Nov 18, 2024

Discussed with @jonasfj.

We will update the publish workflow to also invoke a setup-flutter, and then publish using the dart executable from flutter/bin which sets up FLUTTER_ROOT.

@sigurdm sigurdm self-assigned this Nov 18, 2024
@sigurdm
Copy link
Contributor

sigurdm commented Nov 18, 2024

Our reasoning, is that we want to enable publishing flutter packages first.
Later we can polish if we feel the need.

Is subosito/flutter-action still the prefered action for installing flutter?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment