Skip to content

Commit 8bc9075

Browse files
eseidelbdero
authored andcommitted
fix: honor --flavor for ios-framework builds (#141)
* fix: honor --flavor for ios-framework builds `flutter build ios-framework --flavor X` was silently dropping --flavor before the asset bundling step ran, so flavor-conditional assets and the compiled shorebird.yaml's app_id always fell back to the top-level (non-flavor) value. The same gap exists upstream — tracked at flutter#186220 with proposed fix flutter#186221. Adds `kFlavor: ?flavor` to `BuildInfo.toBuildSystemEnvironment()` with a TODO referencing the upstream PR so a future fork-sync can drop the line cleanly once upstream lands. Tests: - Existing `toBuildSystemEnvironment encoding of standard values` test now exercises a non-null flavor and asserts `Flavor` is included. Added a sibling test asserting the key is omitted when flavor is null. - New shorebird-fork-only tests for compileShorebirdYaml flavor compilation: - CopyAssets path, covering the shared codepath used by every platform. - ReleaseIosApplicationBundle, covering the iOS-framework-specific asset bundle target. End-to-end reproduction (vanilla stable Flutter): https://github.com/eseidel/flavor_bundle_repro * fix: drop unnecessary TODO comment on kFlavor line The comment described a non-action: when flutter#186221 lands, the upstream merge brings in the same kFlavor: ?flavor line and the fork's copy merges cleanly — there's nothing to remove. Carrying the comment just adds a sync-time cleanup task that doesn't otherwise exist.
1 parent 43550bb commit 8bc9075

4 files changed

Lines changed: 133 additions & 286 deletions

File tree

packages/flutter_tools/lib/src/build_info.dart

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -382,6 +382,7 @@ class BuildInfo {
382382
kFileSystemScheme: ?fileSystemScheme,
383383
kBuildName: ?buildName,
384384
kBuildNumber: ?buildNumber,
385+
kFlavor: ?flavor,
385386
if (useLocalCanvasKit) kUseLocalCanvasKitFlag: useLocalCanvasKit.toString(),
386387
};
387388
}

packages/flutter_tools/test/general.shard/build_info_test.dart

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -224,7 +224,7 @@ void main() {
224224
testWithoutContext('toBuildSystemEnvironment encoding of standard values', () {
225225
const buildInfo = BuildInfo(
226226
BuildMode.debug,
227-
'',
227+
'strawberry',
228228
treeShakeIcons: true,
229229
trackWidgetCreation: true,
230230
dartDefines: <String>['foo=2', 'bar=2'],
@@ -256,9 +256,21 @@ void main() {
256256
'FileSystemScheme': 'scheme',
257257
'BuildName': '122',
258258
'BuildNumber': '22',
259+
'Flavor': 'strawberry',
259260
});
260261
});
261262

263+
testWithoutContext('toBuildSystemEnvironment omits Flavor when flavor is null', () {
264+
const buildInfo = BuildInfo(
265+
BuildMode.release,
266+
null,
267+
treeShakeIcons: false,
268+
packageConfigPath: '.dart_tool/package_config.json',
269+
);
270+
271+
expect(buildInfo.toBuildSystemEnvironment().containsKey('Flavor'), isFalse);
272+
});
273+
262274
testWithoutContext('toEnvironmentConfig encoding of standard values', () {
263275
const buildInfo = BuildInfo(
264276
BuildMode.debug,

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

Lines changed: 50 additions & 285 deletions
Original file line numberDiff line numberDiff line change
@@ -224,183 +224,66 @@ flutter:
224224
},
225225
);
226226

227-
group(
228-
"Only compiles shaders with a flavor if the shaders' flavor matches the flavor in the environment",
229-
() {
230-
testUsingContext(
231-
'When the environment does not have a flavor defined',
232-
() async {
233-
// Re-create the environment with the correct process manager for this test.
234-
environment = Environment.test(
235-
fileSystem.currentDirectory,
236-
processManager: globals.processManager,
237-
artifacts: Artifacts.test(),
238-
fileSystem: fileSystem,
239-
logger: BufferLogger.test(),
240-
platform: FakePlatform(),
241-
defines: <String, String>{kBuildMode: BuildMode.debug.cliName},
242-
);
243-
244-
final artifacts = Artifacts.test();
245-
final String impellercPath = artifacts.getHostArtifact(HostArtifact.impellerc).path;
246-
fileSystem.file(impellercPath).createSync(recursive: true);
247-
248-
fileSystem.file('pubspec.yaml')
249-
..createSync()
250-
..writeAsStringSync('''
251-
name: example
252-
flutter:
253-
shaders:
254-
- shaders/common.frag
255-
- path: shaders/premium.frag
256-
flavors:
257-
- premium
258-
''');
259-
writePackageConfigFiles(directory: globals.fs.currentDirectory, mainLibName: 'example');
260-
261-
fileSystem.file('shaders/common.frag').createSync(recursive: true);
262-
fileSystem.file('shaders/premium.frag').createSync(recursive: true);
263-
264-
// Manually create expected output files since FakeProcessManager.any() won't create them
265-
fileSystem
266-
.file('${environment.buildDir.path}/flutter_assets/shaders/common.frag')
267-
.createSync(recursive: true);
268-
269-
await const CopyAssets().build(environment);
270-
271-
expect(
272-
fileSystem.file('${environment.buildDir.path}/flutter_assets/shaders/common.frag'),
273-
exists,
274-
);
275-
expect(
276-
fileSystem.file('${environment.buildDir.path}/flutter_assets/shaders/premium.frag'),
277-
isNot(exists),
278-
);
279-
},
280-
overrides: <Type, Generator>{
281-
FileSystem: () => fileSystem,
282-
ProcessManager: () => FakeProcessManager.any(),
283-
},
284-
);
285-
286-
testUsingContext(
287-
'When the environment has a matching flavor defined',
288-
() async {
289-
// Re-create the environment with the correct process manager for this test.
290-
environment = Environment.test(
291-
fileSystem.currentDirectory,
292-
processManager: globals.processManager,
293-
artifacts: Artifacts.test(),
294-
fileSystem: fileSystem,
295-
logger: BufferLogger.test(),
296-
platform: FakePlatform(),
297-
defines: <String, String>{kBuildMode: BuildMode.debug.cliName},
298-
);
299-
300-
final artifacts = Artifacts.test();
301-
final String impellercPath = artifacts.getHostArtifact(HostArtifact.impellerc).path;
302-
fileSystem.file(impellercPath).createSync(recursive: true);
227+
group('CopyAssets compiles shorebird.yaml using the environment flavor', () {
228+
const shorebirdYamlContents = '''
229+
app_id: base-app-id
230+
flavors:
231+
internal: internal-app-id
232+
stable: stable-app-id
233+
''';
303234

304-
environment.defines[kFlavor] = 'premium';
305-
fileSystem.file('pubspec.yaml')
306-
..createSync()
307-
..writeAsStringSync('''
235+
void writeProjectWithShorebirdYaml() {
236+
fileSystem.file('pubspec.yaml')
237+
..createSync()
238+
..writeAsStringSync('''
308239
name: example
309240
flutter:
310-
shaders:
311-
- shaders/common.frag
312-
- path: shaders/premium.frag
313-
flavors:
314-
- premium
241+
assets:
242+
- shorebird.yaml
315243
''');
316-
writePackageConfigFiles(directory: globals.fs.currentDirectory, mainLibName: 'example');
317-
318-
fileSystem.file('shaders/common.frag').createSync(recursive: true);
319-
fileSystem.file('shaders/premium.frag').createSync(recursive: true);
320-
321-
// Manually create expected output files since FakeProcessManager.any() won't create them
322-
fileSystem
323-
.file('${environment.buildDir.path}/flutter_assets/shaders/common.frag')
324-
.createSync(recursive: true);
325-
fileSystem
326-
.file('${environment.buildDir.path}/flutter_assets/shaders/premium.frag')
327-
.createSync(recursive: true);
328-
329-
await const CopyAssets().build(environment);
330-
331-
expect(
332-
fileSystem.file('${environment.buildDir.path}/flutter_assets/shaders/common.frag'),
333-
exists,
334-
);
335-
expect(
336-
fileSystem.file('${environment.buildDir.path}/flutter_assets/shaders/premium.frag'),
337-
exists,
338-
);
339-
},
340-
overrides: <Type, Generator>{
341-
FileSystem: () => fileSystem,
342-
ProcessManager: () => FakeProcessManager.any(),
343-
},
344-
);
345-
346-
testUsingContext(
347-
'When the environment has a mismatching flavor defined',
348-
() async {
349-
// Re-create the environment with the correct process manager for this test.
350-
environment = Environment.test(
351-
fileSystem.currentDirectory,
352-
processManager: globals.processManager,
353-
artifacts: Artifacts.test(),
354-
fileSystem: fileSystem,
355-
logger: BufferLogger.test(),
356-
platform: FakePlatform(),
357-
defines: <String, String>{kBuildMode: BuildMode.debug.cliName},
358-
);
244+
fileSystem.file('shorebird.yaml')
245+
..createSync()
246+
..writeAsStringSync(shorebirdYamlContents);
247+
writePackageConfigFiles(directory: globals.fs.currentDirectory, mainLibName: 'example');
248+
}
359249

360-
final artifacts = Artifacts.test();
361-
final String impellercPath = artifacts.getHostArtifact(HostArtifact.impellerc).path;
362-
fileSystem.file(impellercPath).createSync(recursive: true);
250+
testUsingContext(
251+
'falls back to top-level app_id when no flavor is set',
252+
() async {
253+
writeProjectWithShorebirdYaml();
363254

364-
environment.defines[kFlavor] = 'free'; // Mismatch
365-
fileSystem.file('pubspec.yaml')
366-
..createSync()
367-
..writeAsStringSync('''
368-
name: example
369-
flutter:
370-
shaders:
371-
- shaders/common.frag
372-
- path: shaders/premium.frag
373-
flavors:
374-
- premium
375-
''');
376-
writePackageConfigFiles(directory: globals.fs.currentDirectory, mainLibName: 'example');
255+
await const CopyAssets().build(environment);
377256

378-
fileSystem.file('shaders/common.frag').createSync(recursive: true);
379-
fileSystem.file('shaders/premium.frag').createSync(recursive: true);
257+
final File compiled = fileSystem.file(
258+
'${environment.buildDir.path}/flutter_assets/shorebird.yaml',
259+
);
260+
expect(compiled.readAsStringSync(), 'app_id: base-app-id');
261+
},
262+
overrides: <Type, Generator>{
263+
FileSystem: () => fileSystem,
264+
ProcessManager: () => FakeProcessManager.any(),
265+
},
266+
);
380267

381-
// Manually create expected output files for the ones that SHOULD exist
382-
fileSystem
383-
.file('${environment.buildDir.path}/flutter_assets/shaders/common.frag')
384-
.createSync(recursive: true);
268+
testUsingContext(
269+
'uses the flavor app_id when flavor is set',
270+
() async {
271+
environment.defines[kFlavor] = 'internal';
272+
writeProjectWithShorebirdYaml();
385273

386-
await const CopyAssets().build(environment);
274+
await const CopyAssets().build(environment);
387275

388-
expect(
389-
fileSystem.file('${environment.buildDir.path}/flutter_assets/shaders/common.frag'),
390-
exists,
391-
);
392-
expect(
393-
fileSystem.file('${environment.buildDir.path}/flutter_assets/shaders/premium.frag'),
394-
isNot(exists),
395-
);
396-
},
397-
overrides: <Type, Generator>{
398-
FileSystem: () => fileSystem,
399-
ProcessManager: () => FakeProcessManager.any(),
400-
},
401-
);
402-
},
403-
);
276+
final File compiled = fileSystem.file(
277+
'${environment.buildDir.path}/flutter_assets/shorebird.yaml',
278+
);
279+
expect(compiled.readAsStringSync(), 'app_id: internal-app-id');
280+
},
281+
overrides: <Type, Generator>{
282+
FileSystem: () => fileSystem,
283+
ProcessManager: () => FakeProcessManager.any(),
284+
},
285+
);
286+
});
404287

405288
testUsingContext(
406289
'transforms assets declared with transformers',
@@ -489,124 +372,6 @@ flutter:
489372
},
490373
);
491374

492-
testUsingContext(
493-
'transforms shaders declared with transformers before compilation',
494-
() async {
495-
Cache.flutterRoot = Cache.defaultFlutterRoot(
496-
platform: globals.platform,
497-
fileSystem: fileSystem,
498-
userMessages: UserMessages(),
499-
);
500-
501-
final environment = Environment.test(
502-
fileSystem.currentDirectory,
503-
processManager: globals.processManager,
504-
artifacts: Artifacts.test(),
505-
fileSystem: fileSystem,
506-
logger: logger,
507-
platform: globals.platform,
508-
defines: <String, String>{kBuildMode: BuildMode.debug.cliName},
509-
);
510-
511-
final artifacts = Artifacts.test();
512-
final String impellercPath = artifacts.getHostArtifact(HostArtifact.impellerc).path;
513-
fileSystem.file(impellercPath).createSync(recursive: true);
514-
515-
fileSystem.file('pubspec.yaml')
516-
..createSync()
517-
..writeAsStringSync('''
518-
name: example
519-
flutter:
520-
shaders:
521-
- path: shaders/test.frag
522-
transformers:
523-
- package: my_capitalizer_transformer
524-
''');
525-
526-
writePackageConfigFiles(directory: globals.fs.currentDirectory, mainLibName: 'example');
527-
528-
fileSystem.file('shaders/test.frag')
529-
..createSync(recursive: true)
530-
..writeAsStringSync('abc');
531-
532-
await const CopyAssets().build(environment);
533-
534-
expect(logger.errorText, isEmpty);
535-
expect(globals.processManager, hasNoRemainingExpectations);
536-
expect(
537-
fileSystem.file('${environment.buildDir.path}/flutter_assets/shaders/test.frag'),
538-
exists,
539-
);
540-
},
541-
overrides: <Type, Generator>{
542-
Logger: () => logger,
543-
FileSystem: () => fileSystem,
544-
Platform: () => FakePlatform(),
545-
ProcessManager: () {
546-
final artifacts = Artifacts.test();
547-
548-
return FakeProcessManager.list(<FakeCommand>[
549-
FakeCommand(
550-
command: <Pattern>[
551-
artifacts.getArtifactPath(Artifact.engineDartBinary),
552-
'run',
553-
'my_capitalizer_transformer',
554-
RegExp('--input=.*'),
555-
RegExp('--output=.*'),
556-
],
557-
onRun: (List<String> args) {
558-
final ArgResults parsedArgs =
559-
(ArgParser()
560-
..addOption('input')
561-
..addOption('output'))
562-
.parse(args);
563-
564-
final File input = fileSystem.file(parsedArgs['input'] as String);
565-
expect(input, exists);
566-
expect(input.readAsStringSync(), 'abc');
567-
568-
fileSystem.file(parsedArgs['output'])
569-
..createSync(recursive: true)
570-
..writeAsStringSync('ABC');
571-
},
572-
),
573-
FakeCommand(
574-
command: <Pattern>[
575-
RegExp(r'.*impellerc.*'),
576-
// Match the 11 arguments generated by ShaderCompiler.compileShader.
577-
...List<Pattern>.filled(11, RegExp(r'.*')),
578-
],
579-
onRun: (List<String> args) {
580-
String? inputPath;
581-
String? outputPath;
582-
String? outputSpirvPath;
583-
for (final arg in args) {
584-
if (arg.startsWith('--input=')) {
585-
inputPath = arg.substring('--input='.length);
586-
} else if (arg.startsWith('--sl=')) {
587-
outputPath = arg.substring('--sl='.length);
588-
} else if (arg.startsWith('--spirv=')) {
589-
outputSpirvPath = arg.substring('--spirv='.length);
590-
}
591-
}
592-
593-
expect(inputPath, isNotNull);
594-
expect(outputPath, isNotNull);
595-
expect(outputSpirvPath, isNotNull);
596-
597-
final File input = fileSystem.file(inputPath);
598-
expect(input, exists);
599-
expect(input.readAsStringSync(), 'ABC');
600-
601-
fileSystem.file(outputPath).createSync(recursive: true);
602-
fileSystem.file(outputSpirvPath).createSync(recursive: true);
603-
},
604-
),
605-
]);
606-
},
607-
},
608-
);
609-
610375
testUsingContext(
611376
'exits tool if an asset transformation fails',
612377
() async {

0 commit comments

Comments
 (0)