dart-lang / build

A build system for Dart written in Dart
https://pub.dev/packages/build
BSD 3-Clause "New" or "Revised" License
791 stars 211 forks source link

Build runner deletes flutter_gen from package_config.json #3705

Closed derdilla closed 6 months ago

derdilla commented 6 months ago

Build runner currently rewrites .dart_tool/package_config.json when run, breaking packages like flutter_gen without asking the user. From a quick search flutter_gen writing directly to that file seems to be an recognized issue. But from a user perspective, it would be great if build_runner could "just preserve the file" for the time being (until there are stable macros).

To reproduce

  1. Create a flutter app with localization
  2. Add something that uses build_runner like mockito
  3. flutter clean && flutter pub get to build localizations
  4. dart run build_runner build

Build_runner and flutter_gen work together when changing the order of steps 4 and 3, showing that they are not fundamentally incompatible.

Versions

SDK: 3.4.0-282.2.beta build_runner: 2.4.9 flutter: 3.22.0-0.2.pre

jakemac53 commented 6 months ago

Build runner currently rewrites .dart_tool/package_config.json when run

build_runner does not delete .dart_tool/package_config.json 😄. But, it is possible that dart run <anything> does (if it runs a pub get).

I think it might be possible to do flutter pub run build_runner or possibly there is some different command that is less verbose, but I am not super familiar with the best way of running Dart scripts from the flutter tool. @christopherfujino might know more.

christopherfujino commented 6 months ago

Build runner currently rewrites .dart_tool/package_config.json when run

build_runner does not delete .dart_tool/package_config.json 😄. But, it is possible that dart run <anything> does (if it runs a pub get).

I think it might be possible to do flutter pub run build_runner or possibly there is some different command that is less verbose, but I am not super familiar with the best way of running Dart scripts from the flutter tool. @christopherfujino might know more.

Build runner currently rewrites .dart_tool/package_config.json when run

build_runner does not delete .dart_tool/package_config.json 😄. But, it is possible that dart run <anything> does (if it runs a pub get).

I think it might be possible to do flutter pub run build_runner or possibly there is some different command that is less verbose, but I am not super familiar with the best way of running Dart scripts from the flutter tool. @christopherfujino might know more.

flutter pub run <PACKAGE> is the only way from the tool to do this. @sigurdm does dart run <anything> do a pub get first?

derdilla commented 6 months ago

flutter pub run build_runner build is deprecated and refers me to dart run. On a related note dart pub get does indeed cause the same reformat.


flutter pub get (to regenerate localizations) also messes with build_runner:

Found 4 declared outputs which already exist on disk. This is likely because the`.dart_tool/build` folder was deleted, or you are submitting generated files to your source repository.
jakemac53 commented 6 months ago

flutter pub run build_runner build is deprecated and refers me to dart run.

Fwiw, this message is just the wrong advice for build_runner, period. It will not be able to recognize dart:ui types unless you run it with the flutter SDK.

And it sounds like it is the wrong advice for any project using flutter_gen, because they will all have this problem of bashing over the flutter customized version of the package config (which flutter_gen really shouldn't be doing, but that is a separate issue).

jakemac53 commented 6 months ago

@christopherfujino can we remove the deprecation warning for flutter pub run?

sigurdm commented 6 months ago

does dart run do a pub get first?

Yes. It does a fast-path check to see if there is an existing working resolution. If not it runs a new (implicit) pub get.

If you have done flutter pub get first I'm surprised to see it doing a new resolution. That is a bug indeed: https://github.com/dart-lang/pub/pull/4272).

Still - I really think we need to get rid of the flutter_gen package, or revise how we are depending on it. Flutter adding it to the package_config is a big mess.

Alternatively, and perhaps the right way to go short-term we should revive the effort to move the flutter post-processing into a hook called by pub. https://github.com/dart-lang/pub/pull/3745. That would erase the difference between dart pub get and flutter pub get.

Fwiw, this message is just the wrong advice for build_runner, period. It will not be able to recognize dart:ui types unless you run it with the flutter SDK.

Not sure how build_runnner recognizes that it is in the Flutter sdk, but this is about $FLUTTER_ROOT/bin/dart wrapper which sets the FLUTTER_ROOT environment variable.

flutter pub run is not doing anything special, it merely invokes the (deprecated) dart pub run. (https://github.com/flutter/flutter/blob/c53a18f4e49aa28726bb6e0ea0b525db0c37c750/packages/flutter_tools/lib/src/commands/packages.dart#L40).

@jonasfj might be interested.

sigurdm commented 6 months ago

Filed a cherry-pick request to get this fixed in the next stable.

Even with the fix, one would still run into trouble if you eg. modify pubspec.yaml, and run dart run build_runner build without a preceding flutter pub get. But that is harder to fix...

jakemac53 commented 6 months ago

Not sure how build_runnner recognizes that it is in the Flutter sdk, but this is about $FLUTTER_ROOT/bin/dart wrapper which sets the FLUTTER_ROOT environment variable.

See here

So, if the dart you use is actually the one from the flutter SDK it should also work I think? But if it is a separately installed Dart SDK it won't.

sigurdm commented 6 months ago

So, if the dart you use is actually the one from the flutter SDK it should also work I think?

I tried running this program with inside build_resolvers

import 'package:build_resolvers/src/sdk_summary.dart';

main() {
  print('IsFlutter: $isFlutter');
}

And it seems to work:

$~//flutter/bin/dart bin/isFlutter.dart 
IsFlutter: true

But if it is a separately installed Dart SDK it won't.

True - to me that is expected.

jakemac53 commented 6 months ago

Right, so flutter pub run is the more canonical way of ensuring this works for people, as it guarantees we use the dart SDK vendored from flutter. If we tell them to use dart run, it will only work if that happens to be the dart from flutter.

keeyzar commented 6 months ago

Hi!

Is there an estimate for a fix or a workaround?

flutter pub run build_runner watch -d stops running on flutter pub get, therefore on each change to my code I need to first run the slow build_runner command and then the flutter pub get. This is super slow...

I tried adding the flutter_gen entry manually, but that also interrupts the build_runner command.

GeumBinLee commented 6 months ago

I did flutter pub get and then flutter pub run build_runner build --delete-conflicting-outputs but it still makes an error saying that Target of URI doesn't exist: 'package:flutter_gen/gen_l10n/app_localizations.dart'.. I just can't find flutter_gen in package_config.json but I can find it and everything work well after running flutter pub get again.

jakemac53 commented 6 months ago

https://github.com/dart-lang/sdk/issues/55758 This cherry pick request should resolve the issue if I understand it correctly

GeumBinLee commented 6 months ago

oh I upgraded flutter and everything work well thank you

jakemac53 commented 6 months ago

I am going to close this issue as I believe it is resolved (in the main channel). The fix should go out in the next stable hotfix.