Skip to content

Commit ebe53d5

Browse files
andrewkolosandrewkolos
authored
Fix asset transformation in the presence of resolution-aware asset variants (#151932)
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 flutter/flutter#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.
1 parent a8d11d2 commit ebe53d5

File tree

7 files changed

+136
-48
lines changed

7 files changed

+136
-48
lines changed

‎packages/flutter_tools/lib/src/build_system/targets/assets.dart‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -154,7 +154,8 @@ Future<Depfile> copyAssets(
154154
);
155155
doCopy = false;
156156
if (failure != null) {
157-
throwToolExit(failure.message);
157+
throwToolExit('User-defined transformation of asset "${entry.key}" failed.\n'
158+
'${failure.message}');
158159
}
159160
}
160161
case AssetKind.font:

‎packages/flutter_tools/lib/src/build_system/tools/asset_transformer.dart‎

Lines changed: 18 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -55,16 +55,21 @@ final class AssetTransformer {
5555
required Logger logger,
5656
}) async {
5757

58-
String getTempFilePath(int transformStep) {
58+
final Directory tempDirectory = _fileSystem.systemTempDirectory.createTempSync();
59+
60+
int transformStep = 0;
61+
File nextTempFile() {
5962
final String basename = _fileSystem.path.basename(asset.path);
6063
final String ext = _fileSystem.path.extension(asset.path);
61-
return '$basename-transformOutput$transformStep$ext';
64+
65+
final File result = tempDirectory.childFile('$basename-transformOutput$transformStep$ext');
66+
transformStep++;
67+
return result;
6268
}
6369

64-
File tempInputFile = _fileSystem.systemTempDirectory.childFile(getTempFilePath(0));
70+
File tempInputFile = nextTempFile();
6571
await asset.copy(tempInputFile.path);
66-
File tempOutputFile = _fileSystem.systemTempDirectory.childFile(getTempFilePath(1));
67-
ErrorHandlingFileSystem.deleteIfExists(tempOutputFile);
72+
File tempOutputFile = nextTempFile();
6873

6974
final Stopwatch stopwatch = Stopwatch()..start();
7075
try {
@@ -78,10 +83,7 @@ final class AssetTransformer {
7883
);
7984

8085
if (transformerFailure != null) {
81-
return AssetTransformationFailure(
82-
'User-defined transformation of asset "${asset.path}" failed.\n'
83-
'${transformerFailure.message}',
84-
);
86+
return AssetTransformationFailure(transformerFailure.message);
8587
}
8688

8789
ErrorHandlingFileSystem.deleteIfExists(tempInputFile);
@@ -90,15 +92,13 @@ final class AssetTransformer {
9092
await tempOutputFile.copy(outputPath);
9193
} else {
9294
tempInputFile = tempOutputFile;
93-
tempOutputFile = _fileSystem.systemTempDirectory.childFile(getTempFilePath(i+2));
94-
ErrorHandlingFileSystem.deleteIfExists(tempOutputFile);
95+
tempOutputFile = nextTempFile();
9596
}
9697
}
9798

9899
logger.printTrace("Finished transforming asset at path '${asset.path}' (${stopwatch.elapsedMilliseconds}ms)");
99100
} finally {
100-
ErrorHandlingFileSystem.deleteIfExists(tempInputFile);
101-
ErrorHandlingFileSystem.deleteIfExists(tempOutputFile);
101+
ErrorHandlingFileSystem.deleteIfExists(tempDirectory, recursive: true);
102102
}
103103

104104
return null;
@@ -124,6 +124,11 @@ final class AssetTransformer {
124124
...transformerArguments,
125125
];
126126

127+
// Delete the output file if it already exists for whatever reason.
128+
// With this, we can check for the existence of the file after transformation
129+
// to make sure the transformer produced an output file.
130+
ErrorHandlingFileSystem.deleteIfExists(output);
131+
127132
logger.printTrace("Transforming asset using command '${command.join(' ')}'");
128133
final ProcessResult result = await _processManager.run(
129134
command,

‎packages/flutter_tools/lib/src/bundle_builder.dart‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -213,7 +213,8 @@ Future<void> writeBundle(
213213
);
214214
doCopy = false;
215215
if (failure != null) {
216-
throwToolExit(failure.message);
216+
throwToolExit('User-defined transformation of asset "${entry.key}" failed.\n'
217+
'${failure.message}');
217218
}
218219
case AssetKind.font:
219220
break;

‎packages/flutter_tools/test/general.shard/build_system/targets/asset_transformer_test.dart‎

Lines changed: 21 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -31,8 +31,8 @@ void main() {
3131
artifacts.getArtifactPath(Artifact.engineDartBinary),
3232
'run',
3333
'my_copy_transformer',
34-
'--input=/.tmp_rand0/asset.txt-transformOutput0.txt',
35-
'--output=/.tmp_rand0/asset.txt-transformOutput1.txt',
34+
'--input=/.tmp_rand0/rand0/asset.txt-transformOutput0.txt',
35+
'--output=/.tmp_rand0/rand0/asset.txt-transformOutput1.txt',
3636
'-f',
3737
'--my_option',
3838
'my_option_value',
@@ -95,8 +95,8 @@ void main() {
9595
dartBinaryPath,
9696
'run',
9797
'my_copy_transformer',
98-
'--input=/.tmp_rand0/asset.txt-transformOutput0.txt',
99-
'--output=/.tmp_rand0/asset.txt-transformOutput1.txt',
98+
'--input=/.tmp_rand0/rand0/asset.txt-transformOutput0.txt',
99+
'--output=/.tmp_rand0/rand0/asset.txt-transformOutput1.txt',
100100
],
101101
onRun: (List<String> args) {
102102
final ArgResults parsedArgs = (ArgParser()
@@ -136,10 +136,9 @@ void main() {
136136
expect(failure, isNotNull);
137137
expect(failure!.message,
138138
'''
139-
User-defined transformation of asset "asset.txt" failed.
140139
Transformer process terminated with non-zero exit code: 1
141140
Transformer package: my_copy_transformer
142-
Full command: $dartBinaryPath run my_copy_transformer --input=/.tmp_rand0/asset.txt-transformOutput0.txt --output=/.tmp_rand0/asset.txt-transformOutput1.txt
141+
Full command: $dartBinaryPath run my_copy_transformer --input=/.tmp_rand0/rand0/asset.txt-transformOutput0.txt --output=/.tmp_rand0/rand0/asset.txt-transformOutput1.txt
143142
stdout:
144143
Beginning transformation
145144
stderr:
@@ -162,8 +161,8 @@ Something went wrong''');
162161
dartBinaryPath,
163162
'run',
164163
'my_transformer',
165-
'--input=/.tmp_rand0/asset.txt-transformOutput0.txt',
166-
'--output=/.tmp_rand0/asset.txt-transformOutput1.txt',
164+
'--input=/.tmp_rand0/rand0/asset.txt-transformOutput0.txt',
165+
'--output=/.tmp_rand0/rand0/asset.txt-transformOutput1.txt',
167166
],
168167
onRun: (_) {
169168
// Do nothing.
@@ -196,11 +195,10 @@ Something went wrong''');
196195
expect(failure, isNotNull);
197196
expect(failure!.message,
198197
'''
199-
User-defined transformation of asset "asset.txt" failed.
200198
Asset transformer my_transformer did not produce an output file.
201-
Input file provided to transformer: "/.tmp_rand0/asset.txt-transformOutput0.txt"
202-
Expected output file at: "/.tmp_rand0/asset.txt-transformOutput1.txt"
203-
Full command: $dartBinaryPath run my_transformer --input=/.tmp_rand0/asset.txt-transformOutput0.txt --output=/.tmp_rand0/asset.txt-transformOutput1.txt
199+
Input file provided to transformer: "/.tmp_rand0/rand0/asset.txt-transformOutput0.txt"
200+
Expected output file at: "/.tmp_rand0/rand0/asset.txt-transformOutput1.txt"
201+
Full command: $dartBinaryPath run my_transformer --input=/.tmp_rand0/rand0/asset.txt-transformOutput0.txt --output=/.tmp_rand0/rand0/asset.txt-transformOutput1.txt
204202
stdout:
205203
206204
stderr:
@@ -225,8 +223,8 @@ Transformation failed, but I forgot to exit with a non-zero code.'''
225223
dartBinaryPath,
226224
'run',
227225
'my_lowercase_transformer',
228-
'--input=/.tmp_rand0/asset.txt-transformOutput0.txt',
229-
'--output=/.tmp_rand0/asset.txt-transformOutput1.txt',
226+
'--input=/.tmp_rand0/rand0/asset.txt-transformOutput0.txt',
227+
'--output=/.tmp_rand0/rand0/asset.txt-transformOutput1.txt',
230228
],
231229
onRun: (List<String> args) {
232230
final ArgResults parsedArgs = (ArgParser()
@@ -245,8 +243,8 @@ Transformation failed, but I forgot to exit with a non-zero code.'''
245243
dartBinaryPath,
246244
'run',
247245
'my_distance_from_ascii_a_transformer',
248-
'--input=/.tmp_rand0/asset.txt-transformOutput1.txt',
249-
'--output=/.tmp_rand0/asset.txt-transformOutput2.txt',
246+
'--input=/.tmp_rand0/rand0/asset.txt-transformOutput1.txt',
247+
'--output=/.tmp_rand0/rand0/asset.txt-transformOutput2.txt',
250248
],
251249
onRun: (List<String> args) {
252250
final ArgResults parsedArgs = (ArgParser()
@@ -314,8 +312,8 @@ Transformation failed, but I forgot to exit with a non-zero code.'''
314312
dartBinaryPath,
315313
'run',
316314
'my_lowercase_transformer',
317-
'--input=/.tmp_rand0/asset.txt-transformOutput0.txt',
318-
'--output=/.tmp_rand0/asset.txt-transformOutput1.txt',
315+
'--input=/.tmp_rand0/rand0/asset.txt-transformOutput0.txt',
316+
'--output=/.tmp_rand0/rand0/asset.txt-transformOutput1.txt',
319317
],
320318
onRun: (List<String> args) {
321319
final ArgResults parsedArgs = (ArgParser()
@@ -334,8 +332,8 @@ Transformation failed, but I forgot to exit with a non-zero code.'''
334332
dartBinaryPath,
335333
'run',
336334
'my_distance_from_ascii_a_transformer',
337-
'--input=/.tmp_rand0/asset.txt-transformOutput1.txt',
338-
'--output=/.tmp_rand0/asset.txt-transformOutput2.txt',
335+
'--input=/.tmp_rand0/rand0/asset.txt-transformOutput1.txt',
336+
'--output=/.tmp_rand0/rand0/asset.txt-transformOutput2.txt',
339337
],
340338
onRun: (List<String> args) {
341339
// Do nothing.
@@ -374,11 +372,10 @@ Transformation failed, but I forgot to exit with a non-zero code.'''
374372
expect(failure, isNotNull);
375373
expect(failure!.message,
376374
'''
377-
User-defined transformation of asset "asset.txt" failed.
378375
Asset transformer my_distance_from_ascii_a_transformer did not produce an output file.
379-
Input file provided to transformer: "/.tmp_rand0/asset.txt-transformOutput1.txt"
380-
Expected output file at: "/.tmp_rand0/asset.txt-transformOutput2.txt"
381-
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
376+
Input file provided to transformer: "/.tmp_rand0/rand0/asset.txt-transformOutput1.txt"
377+
Expected output file at: "/.tmp_rand0/rand0/asset.txt-transformOutput2.txt"
378+
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
382379
stdout:
383380
384381
stderr:

‎packages/flutter_tools/test/general.shard/build_system/targets/assets_test.dart‎

Lines changed: 86 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -252,6 +252,7 @@ flutter:
252252
),
253253
});
254254

255+
255256
testUsingContext('exits tool if an asset transformation fails', () async {
256257
Cache.flutterRoot = Cache.defaultFlutterRoot(
257258
platform: globals.platform,
@@ -289,7 +290,7 @@ flutter:
289290

290291
await expectToolExitLater(
291292
const CopyAssets().build(environment),
292-
startsWith('User-defined transformation of asset "/input.txt" failed.\n'),
293+
startsWith('User-defined transformation of asset "input.txt" failed.\n'),
293294
);
294295
expect(globals.processManager, hasNoRemainingExpectations);
295296
}, overrides: <Type, Generator> {
@@ -316,6 +317,90 @@ flutter:
316317
),
317318
});
318319

320+
testUsingContext('asset transformation, per each asset, uses unique paths for temporary files', () async {
321+
final List<String> inputFilePaths = <String>[];
322+
final List<String> outputFilePaths = <String>[];
323+
324+
final FakeCommand transformerCommand = FakeCommand(
325+
command: <Pattern>[
326+
Artifacts.test().getArtifactPath(Artifact.engineDartBinary),
327+
'run',
328+
'my_capitalizer_transformer',
329+
RegExp('--input=.*'),
330+
RegExp('--output=.*'),
331+
],
332+
onRun: (List<String> args) {
333+
final ArgResults parsedArgs = (ArgParser()
334+
..addOption('input')
335+
..addOption('output'))
336+
.parse(args);
337+
338+
final String input = parsedArgs['input'] as String;
339+
final String output = parsedArgs['output'] as String;
340+
341+
inputFilePaths.add(input);
342+
outputFilePaths.add(output);
343+
344+
fileSystem.file(output)
345+
..createSync()
346+
..writeAsStringSync('foo');
347+
},
348+
);
349+
350+
Cache.flutterRoot = Cache.defaultFlutterRoot(
351+
platform: globals.platform,
352+
fileSystem: fileSystem,
353+
userMessages: UserMessages(),
354+
);
355+
356+
final Environment environment = Environment.test(
357+
fileSystem.currentDirectory,
358+
processManager: FakeProcessManager.list(
359+
<FakeCommand>[
360+
transformerCommand,
361+
transformerCommand,
362+
],
363+
),
364+
artifacts: Artifacts.test(),
365+
fileSystem: fileSystem,
366+
logger: logger,
367+
platform: globals.platform,
368+
defines: <String, String>{
369+
kBuildMode: BuildMode.debug.cliName,
370+
},
371+
);
372+
373+
await fileSystem.file('.packages').create();
374+
375+
fileSystem.file('pubspec.yaml')
376+
..createSync()
377+
..writeAsStringSync('''
378+
name: example
379+
flutter:
380+
assets:
381+
- path: input.txt
382+
transformers:
383+
- package: my_capitalizer_transformer
384+
''');
385+
386+
fileSystem.file('input.txt')
387+
..createSync(recursive: true)
388+
..writeAsStringSync('abc');
389+
390+
fileSystem.directory('2x').childFile('input.txt')
391+
..createSync(recursive: true)
392+
..writeAsStringSync('def');
393+
394+
await const CopyAssets().build(environment);
395+
396+
expect(inputFilePaths.toSet(), hasLength(inputFilePaths.length));
397+
expect(outputFilePaths.toSet(), hasLength(outputFilePaths.length));
398+
}, overrides: <Type, Generator>{
399+
Logger: () => logger,
400+
FileSystem: () => fileSystem,
401+
Platform: () => FakePlatform(),
402+
ProcessManager: () => FakeProcessManager.empty(),
403+
});
319404

320405
testUsingContext('Throws exception if pubspec contains missing files', () async {
321406
fileSystem.file('pubspec.yaml')

‎packages/flutter_tools/test/general.shard/bundle_builder_test.dart‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -71,8 +71,8 @@ void main() {
7171
artifacts.getArtifactPath(Artifact.engineDartBinary),
7272
'run',
7373
'increment',
74-
'--input=/.tmp_rand0/my-asset.txt-transformOutput0.txt',
75-
'--output=/.tmp_rand0/my-asset.txt-transformOutput1.txt'
74+
'--input=/.tmp_rand0/rand0/my-asset.txt-transformOutput0.txt',
75+
'--output=/.tmp_rand0/rand0/my-asset.txt-transformOutput1.txt'
7676
],
7777
onRun: (List<String> command) {
7878
final ArgResults argParseResults = (ArgParser()

‎packages/flutter_tools/test/general.shard/devfs_test.dart‎

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -733,8 +733,8 @@ void main() {
733733
artifacts.getArtifactPath(Artifact.engineDartBinary),
734734
'run',
735735
'increment',
736-
'--input=/.tmp_rand0/retransformerInput-asset.txt-transformOutput0.txt',
737-
'--output=/.tmp_rand0/retransformerInput-asset.txt-transformOutput1.txt',
736+
'--input=/.tmp_rand0/rand0/retransformerInput-asset.txt-transformOutput0.txt',
737+
'--output=/.tmp_rand0/rand0/retransformerInput-asset.txt-transformOutput1.txt',
738738
],
739739
onRun: (List<String> command) {
740740
final ArgResults argParseResults = (ArgParser()
@@ -831,8 +831,8 @@ void main() {
831831
artifacts.getArtifactPath(Artifact.engineDartBinary),
832832
'run',
833833
'increment',
834-
'--input=/.tmp_rand0/retransformerInput-asset.txt-transformOutput0.txt',
835-
'--output=/.tmp_rand0/retransformerInput-asset.txt-transformOutput1.txt',
834+
'--input=/.tmp_rand0/rand0/retransformerInput-asset.txt-transformOutput0.txt',
835+
'--output=/.tmp_rand0/rand0/retransformerInput-asset.txt-transformOutput1.txt',
836836
],
837837
exitCode: 1,
838838
),
@@ -895,10 +895,9 @@ void main() {
895895
expect(devFSWriter.entries, isNull, reason: 'DevFS should not have written anything since the update failed.');
896896
expect(
897897
logger.errorText,
898-
'User-defined transformation of asset "/.tmp_rand0/retransformerInput-asset.txt" failed.\n'
899898
'Transformer process terminated with non-zero exit code: 1\n'
900899
'Transformer package: increment\n'
901-
'Full command: Artifact.engineDartBinary run increment --input=/.tmp_rand0/retransformerInput-asset.txt-transformOutput0.txt --output=/.tmp_rand0/retransformerInput-asset.txt-transformOutput1.txt\n'
900+
'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'
902901
'stdout:\n'
903902
'\n'
904903
'stderr:\n'

0 commit comments

Comments
 (0)