From 8b4d48715bddfdfbaa2bfcdd9950cd63d58d62db Mon Sep 17 00:00:00 2001 From: Jason Simmons Date: Mon, 21 Oct 2024 15:52:29 -0700 Subject: [PATCH] Ensure that the build system checks for updates of locally built flutter_web_sdk artifacts (#156534) The SourceVisitor uses the engineVersion parameter to determine whether it needs to check for changes to artifacts or if it can assume that artifacts are unmodified from a versioned build of the engine. engineVersion is set based on whether the Artifacts instance sets the isLocalEngine property. CachedLocalWebSdkArtifacts (instantiated when the --local-web-sdk flag is used) was only setting isLocalEngine if --local-engine was also used. This caused the build system to ignore changes to the files in the locally built flutter_web_sdk when using --local-web-sdk alone. This PR renames Artifacts.isLocalEngine to usesLocalArtifacts in order to clarify what it means. It also changes CachedLocalWebSdkArtifacts to always enable usesLocalArtifacts. --- packages/flutter_tools/lib/src/artifacts.dart | 17 +++++++++-------- .../flutter_tools/lib/src/bundle_builder.dart | 2 +- .../lib/src/commands/assemble.dart | 2 +- .../lib/src/commands/build_ios_framework.dart | 2 +- .../lib/src/commands/build_macos_framework.dart | 2 +- packages/flutter_tools/lib/src/web/compile.dart | 2 +- .../test/general.shard/artifacts_test.dart | 11 +++++++++++ 7 files changed, 25 insertions(+), 13 deletions(-) diff --git a/packages/flutter_tools/lib/src/artifacts.dart b/packages/flutter_tools/lib/src/artifacts.dart index ec421d4b27f..2b1705058ed 100644 --- a/packages/flutter_tools/lib/src/artifacts.dart +++ b/packages/flutter_tools/lib/src/artifacts.dart @@ -431,8 +431,9 @@ abstract class Artifacts { // and [mode] combination. String getEngineType(TargetPlatform platform, [ BuildMode? mode ]); - /// Whether these artifacts correspond to a non-versioned local engine. - bool get isLocalEngine; + /// Whether these artifacts use any locally built files that are not part of + /// a versioned engine. + bool get usesLocalArtifacts; /// If these artifacts are bound to a local engine build, returns info about /// the location and name of the local engine, otherwise returns null. @@ -906,7 +907,7 @@ class CachedArtifacts implements Artifacts { } @override - bool get isLocalEngine => false; + bool get usesLocalArtifacts => false; } TargetPlatform _currentHostPlatform(Platform platform, OperatingSystemUtils operatingSystemUtils) { @@ -1357,7 +1358,7 @@ class CachedLocalEngineArtifacts implements Artifacts { } @override - bool get isLocalEngine => true; + bool get usesLocalArtifacts => true; } class CachedLocalWebSdkArtifacts implements Artifacts { @@ -1559,7 +1560,7 @@ class CachedLocalWebSdkArtifacts implements Artifacts { } @override - bool get isLocalEngine => _parent.isLocalEngine; + bool get usesLocalArtifacts => true; @override LocalEngineInfo? get localEngineInfo => _parent.localEngineInfo; @@ -1620,7 +1621,7 @@ class OverrideArtifacts implements Artifacts { String getEngineType(TargetPlatform platform, [ BuildMode? mode ]) => parent.getEngineType(platform, mode); @override - bool get isLocalEngine => parent.isLocalEngine; + bool get usesLocalArtifacts => parent.usesLocalArtifacts; @override FileSystemEntity getHostArtifact(HostArtifact artifact) { @@ -1675,7 +1676,7 @@ class _TestArtifacts implements Artifacts { } @override - bool get isLocalEngine => false; + bool get usesLocalArtifacts => false; @override FileSystemEntity getHostArtifact(HostArtifact artifact) { @@ -1694,7 +1695,7 @@ class _TestLocalEngine extends _TestArtifacts { ); @override - bool get isLocalEngine => true; + bool get usesLocalArtifacts => true; @override final LocalEngineInfo localEngineInfo; diff --git a/packages/flutter_tools/lib/src/bundle_builder.dart b/packages/flutter_tools/lib/src/bundle_builder.dart index 83ebbcd7320..effda740a51 100644 --- a/packages/flutter_tools/lib/src/bundle_builder.dart +++ b/packages/flutter_tools/lib/src/bundle_builder.dart @@ -60,7 +60,7 @@ class BundleBuilder { buildDir: project.dartTool.childDirectory('flutter_build'), cacheDir: globals.cache.getRoot(), flutterRootDir: globals.fs.directory(Cache.flutterRoot), - engineVersion: globals.artifacts!.isLocalEngine + engineVersion: globals.artifacts!.usesLocalArtifacts ? null : globals.flutterVersion.engineRevision, defines: { diff --git a/packages/flutter_tools/lib/src/commands/assemble.dart b/packages/flutter_tools/lib/src/commands/assemble.dart index 42252acbee0..1f2de226563 100644 --- a/packages/flutter_tools/lib/src/commands/assemble.dart +++ b/packages/flutter_tools/lib/src/commands/assemble.dart @@ -248,7 +248,7 @@ class AssembleCommand extends FlutterCommand { usage: globals.flutterUsage, analytics: globals.analytics, platform: globals.platform, - engineVersion: artifacts.isLocalEngine + engineVersion: artifacts.usesLocalArtifacts ? null : globals.flutterVersion.engineRevision, generateDartPluginRegistry: true, diff --git a/packages/flutter_tools/lib/src/commands/build_ios_framework.dart b/packages/flutter_tools/lib/src/commands/build_ios_framework.dart index fd6589397ba..1b4ddbb0ed2 100644 --- a/packages/flutter_tools/lib/src/commands/build_ios_framework.dart +++ b/packages/flutter_tools/lib/src/commands/build_ios_framework.dart @@ -456,7 +456,7 @@ end platform: globals.platform, usage: globals.flutterUsage, analytics: globals.analytics, - engineVersion: globals.artifacts!.isLocalEngine + engineVersion: globals.artifacts!.usesLocalArtifacts ? null : globals.flutterVersion.engineRevision, generateDartPluginRegistry: true, diff --git a/packages/flutter_tools/lib/src/commands/build_macos_framework.dart b/packages/flutter_tools/lib/src/commands/build_macos_framework.dart index deebf44e4ba..7794b72860b 100644 --- a/packages/flutter_tools/lib/src/commands/build_macos_framework.dart +++ b/packages/flutter_tools/lib/src/commands/build_macos_framework.dart @@ -237,7 +237,7 @@ end platform: globals.platform, usage: globals.flutterUsage, analytics: globals.analytics, - engineVersion: globals.artifacts!.isLocalEngine ? null : globals.flutterVersion.engineRevision, + engineVersion: globals.artifacts!.usesLocalArtifacts ? null : globals.flutterVersion.engineRevision, generateDartPluginRegistry: true, ); Target target; diff --git a/packages/flutter_tools/lib/src/web/compile.dart b/packages/flutter_tools/lib/src/web/compile.dart index c7082cd66ad..973ed7b9bb4 100644 --- a/packages/flutter_tools/lib/src/web/compile.dart +++ b/packages/flutter_tools/lib/src/web/compile.dart @@ -111,7 +111,7 @@ class WebBuilder { usage: _flutterUsage, analytics: _analytics, cacheDir: globals.cache.getRoot(), - engineVersion: globals.artifacts!.isLocalEngine ? null : _flutterVersion.engineRevision, + engineVersion: globals.artifacts!.usesLocalArtifacts ? null : _flutterVersion.engineRevision, flutterRootDir: _fileSystem.directory(Cache.flutterRoot), // Web uses a different Dart plugin registry. // https://github.com/flutter/flutter/issues/80406 diff --git a/packages/flutter_tools/test/general.shard/artifacts_test.dart b/packages/flutter_tools/test/general.shard/artifacts_test.dart index 89734310ee0..43a00db2b13 100644 --- a/packages/flutter_tools/test/general.shard/artifacts_test.dart +++ b/packages/flutter_tools/test/general.shard/artifacts_test.dart @@ -300,6 +300,17 @@ void main() { 'darwin-x64', ); }); + + testWithoutContext('CachedLocalWebSdkArtifacts wrapping a versioned engine sets usesLocalArtifacts', () { + final CachedLocalWebSdkArtifacts webArtifacts = CachedLocalWebSdkArtifacts( + parent: artifacts, + webSdkPath: fileSystem.path.join(fileSystem.currentDirectory.path, 'out', 'wasm_release'), + fileSystem: fileSystem, + platform: platform, + operatingSystemUtils: FakeOperatingSystemUtils() + ); + expect(webArtifacts.usesLocalArtifacts, true); + }); }); group('LocalEngineArtifacts', () {