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

Analyzer FileResult.isPart returns false when it should return true #49031

Closed
fzyzcjy opened this issue May 14, 2022 · 21 comments
Closed

Analyzer FileResult.isPart returns false when it should return true #49031

fzyzcjy opened this issue May 14, 2022 · 21 comments
Assignees
Labels
area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion.

Comments

@fzyzcjy
Copy link
Contributor

fzyzcjy commented May 14, 2022

Thank you for taking the time to file an issue!

In order to route, prioritize, and act on this as soon as possible please include:

Missing some or all of the above might make the issue take longer or be impossible to act on.


Hi thanks for the build runner! However I cannot run it. Although it looks like a freezed problem, but the error stack trace solely lies in source_gen, so I raise issue here.

Steps: Download https://github.com/fzyzcjy/freezed_build_runner_bug. Run flutter pub get. Run flutter pub run build_runner build --delete-conflicting-outputs --verbose.

Output:

...
[        ] Artifact Instance of 'FontSubsetArtifacts' is not required, skipping update.
[        ] Artifact Instance of 'PubDependencies' is not required, skipping update.
[  +82 ms] executing: /Users/tom/fvm/versions/3.0.0/bin/cache/dart-sdk/bin/dart __deprecated_pub run build_runner build
--delete-conflicting-outputs --verbose
[INFO] Entrypoint:Generating build script...
[INFO] Entrypoint:Generating build script completed, took 497ms

[WARNING] ApplyBuilders:
Configuring `mobx_codegen:mobx_generator` in target `freezed_build_runner_bug:freezed_build_runner_bug` but this is not a known Builder
[INFO] BuildDefinition:Initializing inputs
[INFO] BuildDefinition:Reading cached asset graph...
[INFO] BuildDefinition:Reading cached asset graph completed, took 54ms

[INFO] BuildDefinition:Checking for updates since last build...
[INFO] BuildDefinition:Checking for updates since last build completed, took 556ms

[INFO] Build:Running build...
[INFO] Build:Running build completed, took 23ms

[INFO] Build:Caching finalized dependency graph...
[INFO] Build:Caching finalized dependency graph completed, took 29ms

[SEVERE] source_gen:combining_builder on lib/src/model/a.freezed.dart (cached):

Asset [freezed_build_runner_bug|lib/src/model/a.freezed.dart] is not a Dart library. It may be a part file or a file without Dart source code.
package:build_resolvers/src/resolver.dart 222:7   AnalyzerResolver.libraryFor
package:build_resolvers/src/resolver.dart 123:26  PerActionResolver.libraryFor.<fn>
package:source_gen/builder.dart 115:26            CombiningBuilder.build

[SEVERE] Build:
Failed after 89ms
[+2957 ms] "flutter run" took 3,124ms.
[   +5 ms] pub finished with exit code 1
[   +1 ms] 
           #0      throwToolExit (package:flutter_tools/src/base/common.dart:10:3)
           dart-lang/build#1      _DefaultPub.interactively (package:flutter_tools/src/dart/pub.dart:416:7)
           <asynchronous suspension>
           dart-lang/build#2      PackagesForwardCommand.runCommand (package:flutter_tools/src/commands/packages.dart:251:5)
           <asynchronous suspension>
           dart-lang/build#3      FlutterCommand.run.<anonymous closure> (package:flutter_tools/src/runner/flutter_command.dart:1183:27)
           <asynchronous suspension>
           dart-lang/build#4      AppContext.run.<anonymous closure> (package:flutter_tools/src/base/context.dart:150:19)
           <asynchronous suspension>
           dart-lang/build#5      CommandRunner.runCommand (package:args/command_runner.dart:209:13)
           <asynchronous suspension>
           dart-lang/build#6      FlutterCommandRunner.runCommand.<anonymous closure>
(package:flutter_tools/src/runner/flutter_command_runner.dart:281:9)
           <asynchronous suspension>
           dart-lang/build#7      AppContext.run.<anonymous closure> (package:flutter_tools/src/base/context.dart:150:19)
           <asynchronous suspension>
           dart-lang/build#8      FlutterCommandRunner.runCommand (package:flutter_tools/src/runner/flutter_command_runner.dart:229:5)
           <asynchronous suspension>
           dart-lang/build#9      run.<anonymous closure>.<anonymous closure> (package:flutter_tools/runner.dart:62:9)
           <asynchronous suspension>
           dart-lang/build#10     AppContext.run.<anonymous closure> (package:flutter_tools/src/base/context.dart:150:19)
           <asynchronous suspension>
           dart-lang/build#11     main (package:flutter_tools/executable.dart:94:3)
           <asynchronous suspension>
           
           
[ +261 ms] ensureAnalyticsSent: 258ms
[   +1 ms] Running shutdown hooks
[        ] Shutdown hooks complete
[        ] exiting with code 1
@jakemac53
Copy link
Contributor

So this is indeed a part file, which is why the error is appearing, but usually source_gen would just skip part files. My guess is that somehow that detection of part files is failing?

@jakemac53 jakemac53 transferred this issue from dart-lang/build May 16, 2022
@jakemac53
Copy link
Contributor

Actually moving this back to build, the source_gen package is using Resolver.isLibrary which is a build API.

@jakemac53 jakemac53 transferred this issue from dart-lang/source_gen May 16, 2022
@jakemac53 jakemac53 transferred this issue from dart-lang/build May 16, 2022
@jakemac53
Copy link
Contributor

Nevermind, this is a source_gen issue. The combining builder assumes all inputs are libraries https://github.com/dart-lang/source_gen/blob/master/source_gen/lib/builder.dart#L115. But it should probably skip all part files.

@jakemac53
Copy link
Contributor

Interestingly though, it seems to have found some .part files that it wanted to merge in... it was likely relying on that being empty previously https://github.com/dart-lang/source_gen/blob/master/source_gen/lib/builder.dart#L113.

@jakemac53
Copy link
Contributor

cc @natebosch this could be related to the change to the regex?

@jakemac53
Copy link
Contributor

Hmmm, that change has not been released so maybe not.

@natebosch
Copy link
Member

The regex change should only make the glob more strict, whereas this looks like it's picking up assets that it shouldn't and thinking they are shared part content?

Somehow it looks like there is a a.freezed.something.part asset in your build.

@natebosch
Copy link
Member

Looks like a bug in json_serializable

@kevmoo - did you change something where it is unconditionally outputting some private content for inputs that have no serializable classes, including part files?

// **************************************************************************
// JsonSerializableGenerator
// **************************************************************************

_$_A _$$_AFromJson(Map<String, dynamic> json) => _$_A(
      map: (json['map'] as Map<String, dynamic>).map(
        (k, e) => MapEntry(int.parse(k), B.fromJson(e as Map<String, dynamic>)),
      ),
    );

Map<String, dynamic> _$$_AToJson(_$_A instance) => <String, dynamic>{
      'map': instance.map.map((k, e) => MapEntry(k.toString(), e)),
    };

@jakemac53
Copy link
Contributor

Yeah I am seeing files like b.freezed.json_serializable.g.part with those contents

@kevmoo
Copy link
Member

kevmoo commented May 16, 2022

@natebosch – I don't think so. Maybe a bug in freezed? Those names look pretty weird...

@jakemac53
Copy link
Contributor

jakemac53 commented May 16, 2022

It definitely looks like a bug in isLibrary (likely related to analyzer, but still debugging). I have confirmed that returns true for a freezed generated part file.

@jakemac53
Copy link
Contributor

jakemac53 commented May 16, 2022

Yeah, this looks like an analyzer issue cc @scheglov.

I can set a breakpoint here with the condition assetId.path.endsWith('.freezed.dart') && !result.isPart and it does get tripped.

Note that it isn't 100% consistent, there seems to be some stale data leaking in here.

@jakemac53 jakemac53 changed the title (With reproducible repository) Cannot run build runner: Asset [....freezed.dart] is not a Dart library. It may be a part file or a file without Dart source code. Analyzer FileResult.isPart returns false when it should return true May 16, 2022
@scheglov
Copy link
Contributor

scheglov commented May 16, 2022

Yes, it looks that the implementation of synchronous methods in AnalysisDriver was broken. We fail to re-read files marked as changeFile() even after await applyPendingFileChanges(). After this fixed locally, the reproduction project can be built with the provided command (it failed similarly before the fix). I will write a test, and when it lands publish analyzer 4.1.0 (there are new unrelated APIs).

@jakemac53 jakemac53 added the area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. label May 16, 2022
@jakemac53 jakemac53 transferred this issue from dart-lang/source_gen May 16, 2022
@jakemac53 jakemac53 added the bug label May 16, 2022
@scheglov
Copy link
Contributor

copybara-service bot pushed a commit that referenced this issue May 16, 2022
…) uses new content.

Bug: #49031
Change-Id: I4c959c0fd0461339623268102c879163ac9f3d3f
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/244961
Reviewed-by: Brian Wilkerson <brianwilkerson@google.com>
Commit-Queue: Konstantin Shcheglov <scheglov@google.com>
@fzyzcjy
Copy link
Contributor Author

fzyzcjy commented May 16, 2022

🎉 You guys are great!

@bwilkerson
Copy link
Member

Does this need to be cherry picked?

@jakemac53
Copy link
Contributor

As far as build_runner is concerned we can just pick up the next pub release. Not sure if it causes other general issues with dart analyze though.

@scheglov
Copy link
Contributor

I'm not sure. We use AnalysisSession.getParsedUnit() in DAS, so interactive DAS users could experience similar race conditions.

@fzyzcjy
Copy link
Contributor Author

fzyzcjy commented May 19, 2022

Hi, is it released to pub.dev now? https://pub.dev/packages/build_runner/changelog does have a new release but seems changelog does not mention this issue. Looking forward to the new release!

@natebosch
Copy link
Member

The fix and changelog entry are in the analyzer package.

https://pub.dev/packages/analyzer/changelog

@fzyzcjy
Copy link
Contributor Author

fzyzcjy commented May 20, 2022

I see. Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion.
Projects
None yet
Development

No branches or pull requests

6 participants