Skip to content

Commit

Permalink
Revert Dual Web Compile changes (flutter#143175)
Browse files Browse the repository at this point in the history 
Dual Web Compile has had some issues where `flutter test` is not respecting the `--web-renderer` flag for some reason. I haven't gotten entirely to the bottom of the issue, but for now we need to rever these changes while I investigate. This reverts the following PRs:

flutter#143128
flutter#141396

While doing this revert, I had a few merge conflicts with flutter#142760, and I tried to resolve the merge conflicts within the spirit of that PR's change, but @chingjun I might need your input on whether the imports I have modified are okay with regards to the change you were making.
  • Loading branch information
@eyebrowsoffire
eyebrowsoffire authored Feb 8, 2024
1 parent 8d3c7b6 commit 2efeeb4
Show file tree
Hide file tree
Showing 32 changed files with 414 additions and 794 deletions.
6 changes: 1 addition & 5 deletions dev/benchmarks/macrobenchmarks/web/index.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,8 @@
<head>
<meta charset="UTF-8">
<title>Web Benchmarks</title>
<script src="flutter.js"></script>
</head>
<body>
<script>
{{flutter_build_config}}
_flutter.loader.load();
</script>
<script src="main.dart.js" type="application/javascript"></script>
</body>
</html>
4 changes: 2 additions & 2 deletions dev/devicelab/lib/tasks/web_benchmarks.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ Future<TaskResult> runWebBenchmark(WebBenchmarkOptions benchmarkOptions) async {
'--omit-type-checks',
],
'--dart-define=FLUTTER_WEB_ENABLE_PROFILING=true',
if (!benchmarkOptions.useWasm) '--web-renderer=${benchmarkOptions.webRenderer}',
'--web-renderer=${benchmarkOptions.webRenderer}',
'--profile',
'--no-web-resources-cdn',
'-t',
Expand Down Expand Up @@ -125,7 +125,7 @@ Future<TaskResult> runWebBenchmark(WebBenchmarkOptions benchmarkOptions) async {
return Response.internalServerError(body: '$error');
}
}).add(createBuildDirectoryHandler(
path.join(macrobenchmarksDirectory, 'build', 'web'),
path.join(macrobenchmarksDirectory, 'build', benchmarkOptions.useWasm ? 'web_wasm' : 'web'),
));

server = await io.HttpServer.bind('localhost', benchmarkServerPort);
Expand Down
17 changes: 7 additions & 10 deletions dev/integration_tests/web_e2e_tests/web/index.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,17 +5,14 @@
<html>
<head>
<title>Web Integration Tests</title>
<script src="flutter.js"></script>
<script>
// Use the local CanvasKit bundle instead of the CDN to reduce test flakiness.
window.flutterConfiguration = {
canvasKitBaseUrl: "/canvaskit/"
};
</script>
</head>
<body>
<script>
{{flutter_build_config}}
_flutter.loader.load({
config: {
// Use the local CanvasKit bundle instead of the CDN to reduce test flakiness.
canvasKitBaseUrl: "/canvaskit/",
},
});
</script>
<script src="main.dart.js"></script>
</body>
</html>
13 changes: 11 additions & 2 deletions packages/flutter_tools/lib/src/build_info.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import 'base/os.dart';
import 'base/utils.dart';
import 'convert.dart';
import 'globals.dart' as globals;
import 'web/compile.dart';

/// Whether icon font subsetting is enabled by default.
const bool kIconTreeShakerEnabledDefault = true;
Expand All @@ -35,6 +36,7 @@ class BuildInfo {
List<String>? dartDefines,
this.bundleSkSLPath,
List<String>? dartExperiments,
this.webRenderer = WebRendererMode.auto,
required this.treeShakeIcons,
this.performanceMeasurementFile,
this.packagesPath = '.dart_tool/package_config.json', // TODO(zanderso): make this required and remove the default.
Expand Down Expand Up @@ -128,6 +130,9 @@ class BuildInfo {
/// A list of Dart experiments.
final List<String> dartExperiments;

/// When compiling to web, which web renderer mode we are using (html, canvaskit, auto)
final WebRendererMode webRenderer;

/// The name of a file where flutter assemble will output performance
/// information in a JSON format.
///
Expand Down Expand Up @@ -793,6 +798,10 @@ HostPlatform getCurrentHostPlatform() {
return HostPlatform.linux_x64;
}

FileSystemEntity getWebPlatformBinariesDirectory(Artifacts artifacts, WebRendererMode webRenderer) {
return artifacts.getHostArtifact(HostArtifact.webPlatformKernelFolder);
}

/// Returns the top-level build output directory.
String getBuildDirectory([Config? config, FileSystem? fileSystem]) {
// TODO(johnmccutchan): Stop calling this function as part of setting
Expand Down Expand Up @@ -835,8 +844,8 @@ String getMacOSBuildDirectory() {
}

/// Returns the web build output directory.
String getWebBuildDirectory() {
return globals.fs.path.join(getBuildDirectory(), 'web');
String getWebBuildDirectory([bool isWasm = false]) {
return globals.fs.path.join(getBuildDirectory(), isWasm ? 'web_wasm' : 'web');
}

/// Returns the Linux build output directory.
Expand Down
142 changes: 37 additions & 105 deletions packages/flutter_tools/lib/src/build_system/build_system.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -136,15 +136,6 @@ abstract class Target {
/// A list of zero or more depfiles, located directly under {BUILD_DIR}.
List<String> get depfiles => const <String>[];

/// A string that differentiates different build variants from each other
/// with regards to build flags or settings on the target. This string should
/// represent each build variant as a different unique value. If this value
/// changes between builds, the target will be invalidated and rebuilt.
///
/// By default, this returns null, which indicates there is only one build
/// variant, and the target won't invalidate or rebuild due to this property.
String? get buildKey => null;

/// Whether this target can be executed with the given [environment].
///
/// Returning `true` will cause [build] to be skipped. This is equivalent
Expand All @@ -165,7 +156,6 @@ abstract class Target {
<Node>[
for (final Target target in dependencies) target._toNode(environment),
],
buildKey,
environment,
inputsFiles.containsNewDepfile,
);
Expand All @@ -191,11 +181,9 @@ abstract class Target {
for (final File output in outputs) {
outputPaths.add(output.path);
}
final String? key = buildKey;
final Map<String, Object> result = <String, Object>{
'inputs': inputPaths,
'outputs': outputPaths,
if (key != null) 'buildKey': key,
};
if (!stamp.existsSync()) {
stamp.createSync();
Expand Down Expand Up @@ -230,7 +218,6 @@ abstract class Target {
/// This requires constants from the [Environment] to resolve the paths of
/// inputs and the output stamp.
Map<String, Object> toJson(Environment environment) {
final String? key = buildKey;
return <String, Object>{
'name': name,
'dependencies': <String>[
Expand All @@ -242,7 +229,6 @@ abstract class Target {
'outputs': <String>[
for (final File file in resolveOutputs(environment).sources) file.path,
],
if (key != null) 'buildKey': key,
'stamp': _findStampFile(environment).absolute.path,
};
}
Expand Down Expand Up @@ -994,85 +980,49 @@ void verifyOutputDirectories(List<File> outputs, Environment environment, Target

/// A node in the build graph.
class Node {
factory Node(
Target target,
List<File> inputs,
List<File> outputs,
List<Node> dependencies,
String? buildKey,
Node(
this.target,
this.inputs,
this.outputs,
this.dependencies,
Environment environment,
bool missingDepfile,
this.missingDepfile,
) {
final File stamp = target._findStampFile(environment);
Map<String, Object?>? stampValues;

// If the stamp file doesn't exist, we haven't run this step before and
// all inputs were added.
if (stamp.existsSync()) {
final String content = stamp.readAsStringSync();
if (content.isEmpty) {
stamp.deleteSync();
} else {
try {
stampValues = castStringKeyedMap(json.decode(content));
} on FormatException {
// The json is malformed in some way.
}
}
if (!stamp.existsSync()) {
// No stamp file, not safe to skip.
_dirty = true;
return;
}
if (stampValues != null) {
final String? previousBuildKey = stampValues['buildKey'] as String?;
final Object? stampInputs = stampValues['inputs'];
final Object? stampOutputs = stampValues['outputs'];
if (stampInputs is List<Object?> && stampOutputs is List<Object?>) {
final Set<String> previousInputs = stampInputs.whereType<String>().toSet();
final Set<String> previousOutputs = stampOutputs.whereType<String>().toSet();
return Node.withStamp(
target,
inputs,
previousInputs,
outputs,
previousOutputs,
dependencies,
buildKey,
previousBuildKey,
missingDepfile,
);
}
final String content = stamp.readAsStringSync();
// Something went wrong writing the stamp file.
if (content.isEmpty) {
stamp.deleteSync();
// Malformed stamp file, not safe to skip.
_dirty = true;
return;
}
return Node.withNoStamp(
target,
inputs,
outputs,
dependencies,
buildKey,
missingDepfile,
);
}

Node.withNoStamp(
this.target,
this.inputs,
this.outputs,
this.dependencies,
this.buildKey,
this.missingDepfile,
) : previousInputs = <String>{},
previousOutputs = <String>{},
previousBuildKey = null,
Map<String, Object?>? values;
try {
values = castStringKeyedMap(json.decode(content));
} on FormatException {
// The json is malformed in some way.
_dirty = true;

Node.withStamp(
this.target,
this.inputs,
this.previousInputs,
this.outputs,
this.previousOutputs,
this.dependencies,
this.buildKey,
this.previousBuildKey,
this.missingDepfile,
) : _dirty = false;
return;
}
final Object? inputs = values?['inputs'];
final Object? outputs = values?['outputs'];
if (inputs is List<Object?> && outputs is List<Object?>) {
inputs.cast<String?>().whereType<String>().forEach(previousInputs.add);
outputs.cast<String?>().whereType<String>().forEach(previousOutputs.add);
} else {
// The json is malformed in some way.
_dirty = true;
}
}

/// The resolved input files.
///
Expand All @@ -1084,11 +1034,6 @@ class Node {
/// These files may not yet exist if the target hasn't run yet.
final List<File> outputs;

/// The current build key of the target
///
/// See `buildKey` in the `Target` class for more information.
final String? buildKey;

/// Whether this node is missing a depfile.
///
/// This requires an additional pass of source resolution after the target
Expand All @@ -1102,15 +1047,10 @@ class Node {
final List<Node> dependencies;

/// Output file paths from the previous invocation of this build node.
final Set<String> previousOutputs;
final Set<String> previousOutputs = <String>{};

/// Input file paths from the previous invocation of this build node.
final Set<String> previousInputs;

/// The buildKey from the previous invocation of this build node.
///
/// See `buildKey` in the `Target` class for more information.
final String? previousBuildKey;
final Set<String> previousInputs = <String>{};

/// One or more reasons why a task was invalidated.
///
Expand All @@ -1134,10 +1074,6 @@ class Node {
FileSystem fileSystem,
Logger logger,
) {
if (buildKey != previousBuildKey) {
_invalidate(InvalidatedReasonKind.buildKeyChanged);
_dirty = true;
}
final Set<String> currentOutputPaths = <String>{
for (final File file in outputs) file.path,
};
Expand Down Expand Up @@ -1237,8 +1173,7 @@ class InvalidatedReason {
InvalidatedReasonKind.inputChanged => 'The following inputs have updated contents: ${data.join(',')}',
InvalidatedReasonKind.outputChanged => 'The following outputs have updated contents: ${data.join(',')}',
InvalidatedReasonKind.outputMissing => 'The following outputs were missing: ${data.join(',')}',
InvalidatedReasonKind.outputSetChanged => 'The following outputs were removed from the output set: ${data.join(',')}',
InvalidatedReasonKind.buildKeyChanged => 'The target build key changed.',
InvalidatedReasonKind.outputSetChanged => 'The following outputs were removed from the output set: ${data.join(',')}'
};
}
}
Expand All @@ -1260,7 +1195,4 @@ enum InvalidatedReasonKind {

/// The set of expected output files changed.
outputSetChanged,

/// The build key changed
buildKeyChanged,
}
14 changes: 11 additions & 3 deletions packages/flutter_tools/lib/src/build_system/build_targets.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
// found in the LICENSE file.

import '../base/file_system.dart';
import '../web/compiler_config.dart';
import '../web/compile.dart';
import './build_system.dart';

/// Commonly used build [Target]s.
Expand All @@ -14,7 +14,11 @@ abstract class BuildTargets {
Target get releaseCopyFlutterBundle;
Target get generateLocalizationsTarget;
Target get dartPluginRegistrantTarget;
Target webServiceWorker(FileSystem fileSystem, List<WebCompilerConfig> compileConfigs);
Target webServiceWorker(
FileSystem fileSystem, {
required WebRendererMode webRenderer,
required bool isWasm
});
}

/// BuildTargets that return NoOpTarget for every action.
Expand All @@ -34,7 +38,11 @@ class NoOpBuildTargets extends BuildTargets {
Target get dartPluginRegistrantTarget => const _NoOpTarget();

@override
Target webServiceWorker(FileSystem fileSystem, List<WebCompilerConfig> compileConfigs) => const _NoOpTarget();
Target webServiceWorker(
FileSystem fileSystem, {
required WebRendererMode webRenderer,
required bool isWasm,
}) => const _NoOpTarget();
}

/// A [Target] that does nothing.
Expand Down
Loading

0 comments on commit 2efeeb4

Please sign in to comment.