getsentry / craft

The universal Sentry release CLI 🚀
MIT License
133 stars 15 forks source link

Flutter users should run `flutter pub get` instead of `dart pub get`. #452

Closed marandaneto closed 1 year ago

marandaneto commented 1 year ago

Environment

I'm not sure what's configured in the image of the publish action.

Steps to Reproduce

Try to publish the pub.dev target with a Flutter package, example. This is blocking us to release new versions of our SDKs.

Expected Result

Publish with no errors

Actual Result

https://github.com/getsentry/publish/actions/runs/4084948587/jobs/7042272509

[info] dart: Resolving dependencies... [info] dart: Error: Process "dart" errored with code 1 STDOUT: dart: Resolving dependencies... dart: STDERR:dart: Warning: pubspec.yaml has overrides from pubspec_overrides.yaml dart: Because sentry_flutter requires the Flutter SDK, version solving failed. dart: dart: Flutter users should run flutter pub get instead of dart pub get. dart:

STDOUT: dart: Resolving dependencies... dart:

STDERR:dart: Warning: pubspec.yaml has overrides from pubspec_overrides.yaml dart: Because sentry_flutter requires the Flutter SDK, version solving failed. dart: dart: Flutter users should run flutter pub get instead of dart pub get. dart:

I'm not sure what has changed, maybe we need to upgrade the dart version in the image? Maybe we need to access the raw logs and check if there's more info? Maybe there's a breaking change and we cannot use dart pub publish anymore for Flutter packages?

I could not reproduce this locally, I actually got a different error, by trying dart pub publish --dry-run but it's different than the output in the error.

Package validation found the following potential issue:

  • Your dependency on "sentry" should allow more than one version. For example:

    dependencies: sentry: ^6.20.0

    Constraints that are too tight will make it difficult for people to use your package along with other packages that also depend on "sentry".

BYK commented 1 year ago

There is no "publish image". It uses the docker image built here: https://github.com/getsentry/craft/blob/master/Dockerfile

BYK commented 1 year ago

It is defined here: https://github.com/getsentry/craft/blob/0b738d51ca7504a883bd12b40b9ac6cc12434715/src/targets/pubDev.ts#L67

BYK commented 1 year ago

I guess you can override it by simply adding dartCliPath: flutter to your .craft.yml file under the pubdev target. Worth a try?

marandaneto commented 1 year ago

Good point, dart is downloaded here https://github.com/getsentry/craft/blob/0b738d51ca7504a883bd12b40b9ac6cc12434715/Dockerfile#L32 and it uses the latest version, I need to check if there are any breaking changes in the latest version.

I guess you can override it by simply adding dartCliPath: flutter to your .craft.yml file under the pubdev target. Worth a try?

I'd need to first add flutter to the Dockerfile otherwise it won't be found anyway.

BYK commented 1 year ago

Ah, makes sense. Ping me here if you need any help regarding the Dockerfile

marandaneto commented 1 year ago

Looking at https://github.com/dart-lang/pub/blob/bfaf358551f11dffc8706abbead2dd2131b7af38/test/sdk_test.dart#L153-L170 Apparently, all we need to do is to make sure that the docker image installs flutter, the dart pub already calls either dart pub or flutter pub automatically.

marandaneto commented 1 year ago

Ah, makes sense. Ping me here if you need any help regarding the Dockerfile

Sounds good, I will look into https://docs.flutter.dev/get-started/install/linux#install-flutter-manually Likely needs https://docs.flutter.dev/get-started/install/linux#additional-linux-requirements as well.

Unless it'd be fine to do it using snapd, WDYT?/

asottile-sentry commented 1 year ago

snapd isn't a thing in docker