Skip to content

Switch to new resolver, remove old resolver. #3962

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

Merged
merged 1 commit into from
Apr 9, 2025

Conversation

davidmorgan
Copy link
Contributor

@davidmorgan davidmorgan commented Apr 7, 2025

For #3811.

Switch to using AnalysisDriverModel always, remove BuildAssetUriResolver and --use-experimental-resolver flag.

Clean up use of a few utility methods that were in the old file, assetPath and packagePath. Move deps parsing now used only by the "transitiveDigestsBuilder" into that file; note that the transitive digests builder is no longer enabled by default, it's not clear if anyone still uses it.

As a result of the switch, AnalysisDriverModel is now used in more tests, make it work :)

  • allow concurrent calls to performResolve, which happen when a read during resolve triggers an earlier phase generated file to be built
  • track file syncing by phase, add code to check whether something might have changed between the current phase and the last phase at which it was synced
  • make readPhased in SingleStepReaderWriter trigger builds of generated assets in earlier phases

Copy link

github-actions bot commented Apr 7, 2025

PR Health

@davidmorgan davidmorgan force-pushed the remove-old-resolver branch 8 times, most recently from d074f81 to 38cf987 Compare April 9, 2025 12:12
@davidmorgan davidmorgan changed the title Remove old resolver. Switch to new resolver, remove old resolver. Apr 9, 2025
@davidmorgan davidmorgan force-pushed the remove-old-resolver branch from 38cf987 to 45bb9be Compare April 9, 2025 12:25
@davidmorgan davidmorgan marked this pull request as ready for review April 9, 2025 12:27
@davidmorgan davidmorgan requested a review from jensjoha April 9, 2025 12:27
if (_runningBuild == null) return false;

final node = _runningBuild.assetGraph.get(id);
if (node == null) return false;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Previously in this CL I saw that we deleted a file if reading it returned null or something.
Why does a null node here indicate a non-change?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Null here means it's not in the asset graph, which means it's not a generated file; if you read it once, you'll get the same value by reading it again. So it only needs syncing once.

The dartdoc for the method now says Returns false for all other types of asset, including unknown assets." :)

Thanks.

@davidmorgan davidmorgan merged commit 7bb331e into dart-lang:master Apr 9, 2025
75 checks passed
@davidmorgan davidmorgan deleted the remove-old-resolver branch April 9, 2025 14:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants