github/flutter
7
0
Fork 0
mirror of https://github.com/flutter/flutter.git synced 2025-06-03 00:51:18 +00:00
Code Issues Packages Projects Releases Wiki Activity

Fix asset transformation in the presence of resolution-aware asset variants (#151932)

Browse Source 
For the necessary background knowledge, see the flutter.dev content on [Resolution-aware image assets](https://docs.flutter.dev/ui/assets/assets-and-images#resolution-aware) and [Conditional bundling of assets based on app flavor](https://docs.flutter.dev/ui/assets/assets-and-images#conditional-bundling-of-assets-based-on-app-flavor) if you don't have a basic understanding of these features.

Fixes https://github.com/flutter/flutter/issues/151813 by using unique temporary directories, per asset file, for transformations. Currently, only a single directory is used and the name of the temporary files was based only on the basename of files. This means that `assets/image.png` and `assets/2x/image.png` would share an output path (`<temp dir path>/image.png`), causing a race. If this quick and rough explanation is a bit confusing, the original issue—#151813—provides a full repro and correct identification of the exact cause of the failure that can occur in the asset transformation process.
This commit is contained in:
Andrew Kolos 2024-07-22 16:46:19 -07:00 committed by GitHub
parent a8d11d2134
commit ebe53d570a
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
7 changed files with 136 additions and 48 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
packages/flutter_tools/lib/src/build_system/targets/assets.dart
View File

@ -154,7 +154,8 @@ Future<Depfile> copyAssets(
); );
doCopy = false; doCopy = false;
if (failure != null) { if (failure != null) {
throwToolExit(failure.message); throwToolExit('User-defined transformation of asset "${entry.key}" failed.\n'
'${failure.message}');
} }
} }
case AssetKind.font: case AssetKind.font:

31
packages/flutter_tools/lib/src/build_system/tools/asset_transformer.dart
View File

@ -55,16 +55,21 @@ final class AssetTransformer {
required Logger logger, required Logger logger,
}) async { }) async {
String getTempFilePath(int transformStep) { final Directory tempDirectory = _fileSystem.systemTempDirectory.createTempSync();
int transformStep = 0;
File nextTempFile() {
final String basename = _fileSystem.path.basename(asset.path); final String basename = _fileSystem.path.basename(asset.path);
final String ext = _fileSystem.path.extension(asset.path); final String ext = _fileSystem.path.extension(asset.path);
return '$basename-transformOutput$transformStep$ext';
final File result = tempDirectory.childFile('$basename-transformOutput$transformStep$ext');
transformStep++;
return result;
} }
File tempInputFile = _fileSystem.systemTempDirectory.childFile(getTempFilePath(0)); File tempInputFile = nextTempFile();
await asset.copy(tempInputFile.path); await asset.copy(tempInputFile.path);
File tempOutputFile = _fileSystem.systemTempDirectory.childFile(getTempFilePath(1)); File tempOutputFile = nextTempFile();
ErrorHandlingFileSystem.deleteIfExists(tempOutputFile);
final Stopwatch stopwatch = Stopwatch()..start(); final Stopwatch stopwatch = Stopwatch()..start();
try { try {
@ -78,10 +83,7 @@ final class AssetTransformer {
); );
if (transformerFailure != null) { if (transformerFailure != null) {
return AssetTransformationFailure( return AssetTransformationFailure(transformerFailure.message);
'User-defined transformation of asset "${asset.path}" failed.\n'
'${transformerFailure.message}',
);
} }
ErrorHandlingFileSystem.deleteIfExists(tempInputFile); ErrorHandlingFileSystem.deleteIfExists(tempInputFile);
@ -90,15 +92,13 @@ final class AssetTransformer {
await tempOutputFile.copy(outputPath); await tempOutputFile.copy(outputPath);
} else { } else {
tempInputFile = tempOutputFile; tempInputFile = tempOutputFile;
tempOutputFile = _fileSystem.systemTempDirectory.childFile(getTempFilePath(i+2)); tempOutputFile = nextTempFile();
ErrorHandlingFileSystem.deleteIfExists(tempOutputFile);
} }
} }
logger.printTrace("Finished transforming asset at path '${asset.path}' (${stopwatch.elapsedMilliseconds}ms)"); logger.printTrace("Finished transforming asset at path '${asset.path}' (${stopwatch.elapsedMilliseconds}ms)");
} finally { } finally {
ErrorHandlingFileSystem.deleteIfExists(tempInputFile); ErrorHandlingFileSystem.deleteIfExists(tempDirectory, recursive: true);
ErrorHandlingFileSystem.deleteIfExists(tempOutputFile);
} }
return null; return null;
@ -124,6 +124,11 @@ final class AssetTransformer {
...transformerArguments, ...transformerArguments,
]; ];
// Delete the output file if it already exists for whatever reason.
// With this, we can check for the existence of the file after transformation
// to make sure the transformer produced an output file.
ErrorHandlingFileSystem.deleteIfExists(output);
logger.printTrace("Transforming asset using command '${command.join(' ')}'"); logger.printTrace("Transforming asset using command '${command.join(' ')}'");
final ProcessResult result = await _processManager.run( final ProcessResult result = await _processManager.run(
command, command,

3
packages/flutter_tools/lib/src/bundle_builder.dart
View File

@ -213,7 +213,8 @@ Future<void> writeBundle(
); );
doCopy = false; doCopy = false;
if (failure != null) { if (failure != null) {
throwToolExit(failure.message); throwToolExit('User-defined transformation of asset "${entry.key}" failed.\n'
'${failure.message}');
} }
case AssetKind.font: case AssetKind.font:
break; break;

45
packages/flutter_tools/test/general.shard/build_system/targets/asset_transformer_test.dart
View File

@ -31,8 +31,8 @@ void main() {
artifacts.getArtifactPath(Artifact.engineDartBinary), artifacts.getArtifactPath(Artifact.engineDartBinary),
'run', 'run',
'my_copy_transformer', 'my_copy_transformer',
'--input=/.tmp_rand0/asset.txt-transformOutput0.txt', '--input=/.tmp_rand0/rand0/asset.txt-transformOutput0.txt',
'--output=/.tmp_rand0/asset.txt-transformOutput1.txt', '--output=/.tmp_rand0/rand0/asset.txt-transformOutput1.txt',
'-f', '-f',
'--my_option', '--my_option',
'my_option_value', 'my_option_value',
@ -95,8 +95,8 @@ void main() {
dartBinaryPath, dartBinaryPath,
'run', 'run',
'my_copy_transformer', 'my_copy_transformer',
'--input=/.tmp_rand0/asset.txt-transformOutput0.txt', '--input=/.tmp_rand0/rand0/asset.txt-transformOutput0.txt',
'--output=/.tmp_rand0/asset.txt-transformOutput1.txt', '--output=/.tmp_rand0/rand0/asset.txt-transformOutput1.txt',
], ],
onRun: (List<String> args) { onRun: (List<String> args) {
final ArgResults parsedArgs = (ArgParser() final ArgResults parsedArgs = (ArgParser()
@ -136,10 +136,9 @@ void main() {
expect(failure, isNotNull); expect(failure, isNotNull);
expect(failure!.message, expect(failure!.message,
''' '''
User-defined transformation of asset "asset.txt" failed.
Transformer process terminated with non-zero exit code: 1 Transformer process terminated with non-zero exit code: 1
Transformer package: my_copy_transformer Transformer package: my_copy_transformer
Full command: $dartBinaryPath run my_copy_transformer --input=/.tmp_rand0/asset.txt-transformOutput0.txt --output=/.tmp_rand0/asset.txt-transformOutput1.txt Full command: $dartBinaryPath run my_copy_transformer --input=/.tmp_rand0/rand0/asset.txt-transformOutput0.txt --output=/.tmp_rand0/rand0/asset.txt-transformOutput1.txt
stdout: stdout:
Beginning transformation Beginning transformation
stderr: stderr:
@ -162,8 +161,8 @@ Something went wrong''');
dartBinaryPath, dartBinaryPath,
'run', 'run',
'my_transformer', 'my_transformer',
'--input=/.tmp_rand0/asset.txt-transformOutput0.txt', '--input=/.tmp_rand0/rand0/asset.txt-transformOutput0.txt',
'--output=/.tmp_rand0/asset.txt-transformOutput1.txt', '--output=/.tmp_rand0/rand0/asset.txt-transformOutput1.txt',
], ],
onRun: (_) { onRun: (_) {
// Do nothing. // Do nothing.
@ -196,11 +195,10 @@ Something went wrong''');
expect(failure, isNotNull); expect(failure, isNotNull);
expect(failure!.message, expect(failure!.message,
''' '''
User-defined transformation of asset "asset.txt" failed.
Asset transformer my_transformer did not produce an output file. Asset transformer my_transformer did not produce an output file.
Input file provided to transformer: "/.tmp_rand0/asset.txt-transformOutput0.txt" Input file provided to transformer: "/.tmp_rand0/rand0/asset.txt-transformOutput0.txt"
Expected output file at: "/.tmp_rand0/asset.txt-transformOutput1.txt" Expected output file at: "/.tmp_rand0/rand0/asset.txt-transformOutput1.txt"
Full command: $dartBinaryPath run my_transformer --input=/.tmp_rand0/asset.txt-transformOutput0.txt --output=/.tmp_rand0/asset.txt-transformOutput1.txt Full command: $dartBinaryPath run my_transformer --input=/.tmp_rand0/rand0/asset.txt-transformOutput0.txt --output=/.tmp_rand0/rand0/asset.txt-transformOutput1.txt
stdout: stdout:
stderr: stderr:
@ -225,8 +223,8 @@ Transformation failed, but I forgot to exit with a non-zero code.'''
dartBinaryPath, dartBinaryPath,
'run', 'run',
'my_lowercase_transformer', 'my_lowercase_transformer',
'--input=/.tmp_rand0/asset.txt-transformOutput0.txt', '--input=/.tmp_rand0/rand0/asset.txt-transformOutput0.txt',
'--output=/.tmp_rand0/asset.txt-transformOutput1.txt', '--output=/.tmp_rand0/rand0/asset.txt-transformOutput1.txt',
], ],
onRun: (List<String> args) { onRun: (List<String> args) {
final ArgResults parsedArgs = (ArgParser() final ArgResults parsedArgs = (ArgParser()
@ -245,8 +243,8 @@ Transformation failed, but I forgot to exit with a non-zero code.'''
dartBinaryPath, dartBinaryPath,
'run', 'run',
'my_distance_from_ascii_a_transformer', 'my_distance_from_ascii_a_transformer',
'--input=/.tmp_rand0/asset.txt-transformOutput1.txt', '--input=/.tmp_rand0/rand0/asset.txt-transformOutput1.txt',
'--output=/.tmp_rand0/asset.txt-transformOutput2.txt', '--output=/.tmp_rand0/rand0/asset.txt-transformOutput2.txt',
], ],
onRun: (List<String> args) { onRun: (List<String> args) {
final ArgResults parsedArgs = (ArgParser() final ArgResults parsedArgs = (ArgParser()
@ -314,8 +312,8 @@ Transformation failed, but I forgot to exit with a non-zero code.'''
dartBinaryPath, dartBinaryPath,
'run', 'run',
'my_lowercase_transformer', 'my_lowercase_transformer',
'--input=/.tmp_rand0/asset.txt-transformOutput0.txt', '--input=/.tmp_rand0/rand0/asset.txt-transformOutput0.txt',
'--output=/.tmp_rand0/asset.txt-transformOutput1.txt', '--output=/.tmp_rand0/rand0/asset.txt-transformOutput1.txt',
], ],
onRun: (List<String> args) { onRun: (List<String> args) {
final ArgResults parsedArgs = (ArgParser() final ArgResults parsedArgs = (ArgParser()
@ -334,8 +332,8 @@ Transformation failed, but I forgot to exit with a non-zero code.'''
dartBinaryPath, dartBinaryPath,
'run', 'run',
'my_distance_from_ascii_a_transformer', 'my_distance_from_ascii_a_transformer',
'--input=/.tmp_rand0/asset.txt-transformOutput1.txt', '--input=/.tmp_rand0/rand0/asset.txt-transformOutput1.txt',
'--output=/.tmp_rand0/asset.txt-transformOutput2.txt', '--output=/.tmp_rand0/rand0/asset.txt-transformOutput2.txt',
], ],
onRun: (List<String> args) { onRun: (List<String> args) {
// Do nothing. // Do nothing.
@ -374,11 +372,10 @@ Transformation failed, but I forgot to exit with a non-zero code.'''
expect(failure, isNotNull); expect(failure, isNotNull);
expect(failure!.message, expect(failure!.message,
''' '''
User-defined transformation of asset "asset.txt" failed.
Asset transformer my_distance_from_ascii_a_transformer did not produce an output file. Asset transformer my_distance_from_ascii_a_transformer did not produce an output file.
Input file provided to transformer: "/.tmp_rand0/asset.txt-transformOutput1.txt" Input file provided to transformer: "/.tmp_rand0/rand0/asset.txt-transformOutput1.txt"
Expected output file at: "/.tmp_rand0/asset.txt-transformOutput2.txt" Expected output file at: "/.tmp_rand0/rand0/asset.txt-transformOutput2.txt"
Full command: Artifact.engineDartBinary run my_distance_from_ascii_a_transformer --input=/.tmp_rand0/asset.txt-transformOutput1.txt --output=/.tmp_rand0/asset.txt-transformOutput2.txt Full command: Artifact.engineDartBinary run my_distance_from_ascii_a_transformer --input=/.tmp_rand0/rand0/asset.txt-transformOutput1.txt --output=/.tmp_rand0/rand0/asset.txt-transformOutput2.txt
stdout: stdout:
stderr: stderr:

87
packages/flutter_tools/test/general.shard/build_system/targets/assets_test.dart
View File

@ -252,6 +252,7 @@ flutter:
), ),
}); });
testUsingContext('exits tool if an asset transformation fails', () async { testUsingContext('exits tool if an asset transformation fails', () async {
Cache.flutterRoot = Cache.defaultFlutterRoot( Cache.flutterRoot = Cache.defaultFlutterRoot(
platform: globals.platform, platform: globals.platform,
@ -289,7 +290,7 @@ flutter:
await expectToolExitLater( await expectToolExitLater(
const CopyAssets().build(environment), const CopyAssets().build(environment),
startsWith('User-defined transformation of asset "/input.txt" failed.\n'), startsWith('User-defined transformation of asset "input.txt" failed.\n'),
); );
expect(globals.processManager, hasNoRemainingExpectations); expect(globals.processManager, hasNoRemainingExpectations);
}, overrides: <Type, Generator> { }, overrides: <Type, Generator> {
@ -316,6 +317,90 @@ flutter:
), ),
}); });
testUsingContext('asset transformation, per each asset, uses unique paths for temporary files', () async {
final List<String> inputFilePaths = <String>[];
final List<String> outputFilePaths = <String>[];
final FakeCommand transformerCommand = FakeCommand(
command: <Pattern>[
Artifacts.test().getArtifactPath(Artifact.engineDartBinary),
'run',
'my_capitalizer_transformer',
RegExp('--input=.*'),
RegExp('--output=.*'),
],
onRun: (List<String> args) {
final ArgResults parsedArgs = (ArgParser()
..addOption('input')
..addOption('output'))
.parse(args);
final String input = parsedArgs['input'] as String;
final String output = parsedArgs['output'] as String;
inputFilePaths.add(input);
outputFilePaths.add(output);
fileSystem.file(output)
..createSync()
..writeAsStringSync('foo');
},
);
Cache.flutterRoot = Cache.defaultFlutterRoot(
platform: globals.platform,
fileSystem: fileSystem,
userMessages: UserMessages(),
);
final Environment environment = Environment.test(
fileSystem.currentDirectory,
processManager: FakeProcessManager.list(
<FakeCommand>[
transformerCommand,
transformerCommand,
],
),
artifacts: Artifacts.test(),
fileSystem: fileSystem,
logger: logger,
platform: globals.platform,
defines: <String, String>{
kBuildMode: BuildMode.debug.cliName,
},
);
await fileSystem.file('.packages').create();
fileSystem.file('pubspec.yaml')
..createSync()
..writeAsStringSync('''
name: example
flutter:
assets:
- path: input.txt
transformers:
- package: my_capitalizer_transformer
''');
fileSystem.file('input.txt')
..createSync(recursive: true)
..writeAsStringSync('abc');
fileSystem.directory('2x').childFile('input.txt')
..createSync(recursive: true)
..writeAsStringSync('def');
await const CopyAssets().build(environment);
expect(inputFilePaths.toSet(), hasLength(inputFilePaths.length));
expect(outputFilePaths.toSet(), hasLength(outputFilePaths.length));
}, overrides: <Type, Generator>{
Logger: () => logger,
FileSystem: () => fileSystem,
Platform: () => FakePlatform(),
ProcessManager: () => FakeProcessManager.empty(),
});
testUsingContext('Throws exception if pubspec contains missing files', () async { testUsingContext('Throws exception if pubspec contains missing files', () async {
fileSystem.file('pubspec.yaml') fileSystem.file('pubspec.yaml')

4
packages/flutter_tools/test/general.shard/bundle_builder_test.dart
View File

@ -71,8 +71,8 @@ void main() {
artifacts.getArtifactPath(Artifact.engineDartBinary), artifacts.getArtifactPath(Artifact.engineDartBinary),
'run', 'run',
'increment', 'increment',
'--input=/.tmp_rand0/my-asset.txt-transformOutput0.txt', '--input=/.tmp_rand0/rand0/my-asset.txt-transformOutput0.txt',
'--output=/.tmp_rand0/my-asset.txt-transformOutput1.txt' '--output=/.tmp_rand0/rand0/my-asset.txt-transformOutput1.txt'
], ],
onRun: (List<String> command) { onRun: (List<String> command) {
final ArgResults argParseResults = (ArgParser() final ArgResults argParseResults = (ArgParser()

11
packages/flutter_tools/test/general.shard/devfs_test.dart
View File

@ -733,8 +733,8 @@ void main() {
artifacts.getArtifactPath(Artifact.engineDartBinary), artifacts.getArtifactPath(Artifact.engineDartBinary),
'run', 'run',
'increment', 'increment',
'--input=/.tmp_rand0/retransformerInput-asset.txt-transformOutput0.txt', '--input=/.tmp_rand0/rand0/retransformerInput-asset.txt-transformOutput0.txt',
'--output=/.tmp_rand0/retransformerInput-asset.txt-transformOutput1.txt', '--output=/.tmp_rand0/rand0/retransformerInput-asset.txt-transformOutput1.txt',
], ],
onRun: (List<String> command) { onRun: (List<String> command) {
final ArgResults argParseResults = (ArgParser() final ArgResults argParseResults = (ArgParser()
@ -831,8 +831,8 @@ void main() {
artifacts.getArtifactPath(Artifact.engineDartBinary), artifacts.getArtifactPath(Artifact.engineDartBinary),
'run', 'run',
'increment', 'increment',
'--input=/.tmp_rand0/retransformerInput-asset.txt-transformOutput0.txt', '--input=/.tmp_rand0/rand0/retransformerInput-asset.txt-transformOutput0.txt',
'--output=/.tmp_rand0/retransformerInput-asset.txt-transformOutput1.txt', '--output=/.tmp_rand0/rand0/retransformerInput-asset.txt-transformOutput1.txt',
], ],
exitCode: 1, exitCode: 1,
), ),
@ -895,10 +895,9 @@ void main() {
expect(devFSWriter.entries, isNull, reason: 'DevFS should not have written anything since the update failed.'); expect(devFSWriter.entries, isNull, reason: 'DevFS should not have written anything since the update failed.');
expect( expect(
logger.errorText, logger.errorText,
'User-defined transformation of asset "/.tmp_rand0/retransformerInput-asset.txt" failed.\n'
'Transformer process terminated with non-zero exit code: 1\n' 'Transformer process terminated with non-zero exit code: 1\n'
'Transformer package: increment\n' 'Transformer package: increment\n'
'Full command: Artifact.engineDartBinary run increment --input=/.tmp_rand0/retransformerInput-asset.txt-transformOutput0.txt --output=/.tmp_rand0/retransformerInput-asset.txt-transformOutput1.txt\n' 'Full command: Artifact.engineDartBinary run increment --input=/.tmp_rand0/rand0/retransformerInput-asset.txt-transformOutput0.txt --output=/.tmp_rand0/rand0/retransformerInput-asset.txt-transformOutput1.txt\n'
'stdout:\n' 'stdout:\n'
'\n' '\n'
'stderr:\n' 'stderr:\n'