Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add usage event when iOS app is archived #108643

Merged
merged 1 commit into from
Jul 29, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion dev/devicelab/bin/tasks/ios_content_validation_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,16 @@ Future<void> main() async {
section('Archive');

await inDirectory(flutterProject.rootPath, () async {
await flutter('build', options: <String>[
final String output = await evalFlutter('build', options: <String>[
'xcarchive',
'-v',
]);

// Note this isBot so usage won't actually be sent,
// this log line is printed whenever the app is archived.
if (!output.contains('Sending archive event if usage enabled')) {
throw TaskResult.failure('Usage archive event not sent');
}
});

final String archivePath = path.join(
Expand Down
1 change: 1 addition & 0 deletions packages/flutter_tools/bin/xcode_backend.dart
Original file line number Diff line number Diff line change
Expand Up @@ -366,6 +366,7 @@ class Context {
'-dTrackWidgetCreation=${environment['TRACK_WIDGET_CREATION'] ?? ''}',
'-dDartObfuscation=${environment['DART_OBFUSCATION'] ?? ''}',
'-dEnableBitcode=$bitcodeFlag',
'-dAction=${environment['ACTION'] ?? ''}',
'--ExtraGenSnapshotOptions=${environment['EXTRA_GEN_SNAPSHOT_OPTIONS'] ?? ''}',
'--DartDefines=${environment['DART_DEFINES'] ?? ''}',
'--ExtraFrontEndOptions=${environment['EXTRA_FRONT_END_OPTIONS'] ?? ''}',
Expand Down
5 changes: 5 additions & 0 deletions packages/flutter_tools/lib/src/build_info.dart
Original file line number Diff line number Diff line change
Expand Up @@ -950,6 +950,11 @@ const String kBuildName = 'BuildName';
/// The define to pass build number
const String kBuildNumber = 'BuildNumber';

/// The action Xcode is taking.
///
/// Will be "build" when building and "install" when archiving.
const String kXcodeAction = 'Action';

final Converter<String, String> _defineEncoder = utf8.encoder.fuse(base64.encoder);
final Converter<String, String> _defineDecoder = base64.decoder.fuse(utf8.decoder);

Expand Down
8 changes: 8 additions & 0 deletions packages/flutter_tools/lib/src/build_system/build_system.dart
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import '../base/platform.dart';
import '../base/utils.dart';
import '../cache.dart';
import '../convert.dart';
import '../reporting/reporting.dart';
import 'exceptions.dart';
import 'file_store.dart';
import 'source.dart';
Expand Down Expand Up @@ -332,6 +333,7 @@ class Environment {
required Artifacts artifacts,
required ProcessManager processManager,
required Platform platform,
required Usage usage,
String? engineVersion,
required bool generateDartPluginRegistry,
Directory? buildDir,
Expand Down Expand Up @@ -372,6 +374,7 @@ class Environment {
artifacts: artifacts,
processManager: processManager,
platform: platform,
usage: usage,
engineVersion: engineVersion,
inputs: inputs,
generateDartPluginRegistry: generateDartPluginRegistry,
Expand All @@ -392,6 +395,7 @@ class Environment {
Map<String, String> inputs = const <String, String>{},
String? engineVersion,
Platform? platform,
Usage? usage,
bool generateDartPluginRegistry = false,
required FileSystem fileSystem,
required Logger logger,
Expand All @@ -411,6 +415,7 @@ class Environment {
artifacts: artifacts,
processManager: processManager,
platform: platform ?? FakePlatform(),
usage: usage ?? TestUsage(),
engineVersion: engineVersion,
generateDartPluginRegistry: generateDartPluginRegistry,
);
Expand All @@ -429,6 +434,7 @@ class Environment {
required this.logger,
required this.fileSystem,
required this.artifacts,
required this.usage,
this.engineVersion,
required this.inputs,
required this.generateDartPluginRegistry,
Expand Down Expand Up @@ -509,6 +515,8 @@ class Environment {

final FileSystem fileSystem;

final Usage usage;

/// The version of the current engine, or `null` if built with a local engine.
final String? engineVersion;

Expand Down
25 changes: 25 additions & 0 deletions packages/flutter_tools/lib/src/build_system/targets/ios.dart
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import '../../build_info.dart';
import '../../globals.dart' as globals show xcode;
import '../../macos/xcode.dart';
import '../../project.dart';
import '../../reporting/reporting.dart';
import '../build_system.dart';
import '../depfile.dart';
import '../exceptions.dart';
Expand Down Expand Up @@ -570,6 +571,30 @@ class ReleaseIosApplicationBundle extends IosAssetBundle {
List<Target> get dependencies => const <Target>[
AotAssemblyRelease(),
];

@override
Future<void> build(Environment environment) async {
bool buildSuccess = true;
try {
await super.build(environment);
} catch (_) { // ignore: avoid_catches_without_on_clauses
buildSuccess = false;
rethrow;
} finally {
// Send a usage event when the app is being archived.
// Since assemble is run during a `flutter build`/`run` as well as an out-of-band
// archive command from Xcode, this is a more accurate count than `flutter build ipa` alone.
if (environment.defines[kXcodeAction]?.toLowerCase() == 'install') {
environment.logger.printTrace('Sending archive event if usage enabled.');
UsageEvent(
'assemble',
'ios-archive',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we have an analogous event on the Android (, desktop, web, ...) side?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does flutter build apk already fully cover Android? Or does it have the same issue where it can be bundled and released without flutter? Web seems to say "upload a release version" which means the tool may not know if the app is being "published" or just being built in release mode. Windows and Linux both have publication instructions outside of the tool, so that may benefit from a similar pattern to this, if assemble is called, and there's some way to know it's being packaged. Each platform would need individual investigation.

label: buildSuccess ? 'success' : 'fail',
flutterUsage: environment.usage,
).send();
}
}
}
}

/// Create an App.framework for debug iOS targets.
Expand Down
1 change: 1 addition & 0 deletions packages/flutter_tools/lib/src/bundle_builder.dart
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ class BundleBuilder {
fileSystem: globals.fs,
logger: globals.logger,
processManager: globals.processManager,
usage: globals.flutterUsage,
platform: globals.platform,
generateDartPluginRegistry: true,
);
Expand Down
1 change: 1 addition & 0 deletions packages/flutter_tools/lib/src/commands/assemble.dart
Original file line number Diff line number Diff line change
Expand Up @@ -238,6 +238,7 @@ class AssembleCommand extends FlutterCommand {
fileSystem: globals.fs,
logger: globals.logger,
processManager: globals.processManager,
usage: globals.flutterUsage,
platform: globals.platform,
engineVersion: artifacts.isLocalEngine
? null
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -433,6 +433,7 @@ end
logger: globals.logger,
processManager: globals.processManager,
platform: globals.platform,
usage: globals.flutterUsage,
engineVersion: globals.artifacts!.isLocalEngine
? null
: globals.flutterVersion.engineRevision,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,7 @@ end
logger: globals.logger,
processManager: globals.processManager,
platform: globals.platform,
usage: globals.flutterUsage,
engineVersion: globals.artifacts!.isLocalEngine ? null : globals.flutterVersion.engineRevision,
generateDartPluginRegistry: true,
);
Expand Down
1 change: 1 addition & 0 deletions packages/flutter_tools/lib/src/commands/create_base.dart
Original file line number Diff line number Diff line change
Expand Up @@ -538,6 +538,7 @@ abstract class CreateBase extends FlutterCommand {
outputDir: globals.fs.directory(getBuildDirectory()),
processManager: globals.processManager,
platform: globals.platform,
usage: globals.flutterUsage,
projectDir: project.directory,
generateDartPluginRegistry: true,
);
Expand Down
2 changes: 2 additions & 0 deletions packages/flutter_tools/lib/src/commands/packages.dart
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,7 @@ class PackagesGetCommand extends FlutterCommand {
outputDir: globals.fs.directory(getBuildDirectory()),
processManager: globals.processManager,
platform: globals.platform,
usage: globals.flutterUsage,
projectDir: flutterProject.directory,
generateDartPluginRegistry: true,
);
Expand Down Expand Up @@ -332,6 +333,7 @@ class PackagesInteractiveGetCommand extends FlutterCommand {
outputDir: globals.fs.directory(getBuildDirectory()),
processManager: globals.processManager,
platform: globals.platform,
usage: globals.flutterUsage,
projectDir: flutterProject.directory,
generateDartPluginRegistry: true,
);
Expand Down
1 change: 1 addition & 0 deletions packages/flutter_tools/lib/src/resident_runner.dart
Original file line number Diff line number Diff line change
Expand Up @@ -1217,6 +1217,7 @@ abstract class ResidentRunner extends ResidentHandlers {
outputDir: globals.fs.directory(getBuildDirectory()),
processManager: globals.processManager,
platform: globals.platform,
usage: globals.flutterUsage,
projectDir: globals.fs.currentDirectory,
generateDartPluginRegistry: generateDartPluginRegistry,
defines: <String, String>{
Expand Down
1 change: 1 addition & 0 deletions packages/flutter_tools/lib/src/runner/flutter_command.dart
Original file line number Diff line number Diff line change
Expand Up @@ -1343,6 +1343,7 @@ abstract class FlutterCommand extends Command<void> {
outputDir: globals.fs.directory(getBuildDirectory()),
processManager: globals.processManager,
platform: globals.platform,
usage: globals.flutterUsage,
projectDir: project.directory,
generateDartPluginRegistry: true,
);
Expand Down
1 change: 1 addition & 0 deletions packages/flutter_tools/lib/src/web/compile.dart
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ Future<void> buildWeb(
logger: globals.logger,
processManager: globals.processManager,
platform: globals.platform,
usage: globals.flutterUsage,
cacheDir: globals.cache.getRoot(),
engineVersion: globals.artifacts!.isLocalEngine
? null
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import 'package:flutter_tools/src/artifacts.dart';
import 'package:flutter_tools/src/base/deferred_component.dart';
import 'package:flutter_tools/src/base/file_system.dart';
import 'package:flutter_tools/src/base/logger.dart';
import 'package:flutter_tools/src/base/platform.dart';
import 'package:flutter_tools/src/build_info.dart';
import 'package:flutter_tools/src/build_system/build_system.dart';

Expand All @@ -23,21 +22,13 @@ void main() {

Environment createEnvironment() {
final Map<String, String> defines = <String, String>{ kDeferredComponents: 'true' };
final Environment result = Environment(
outputDir: fileSystem.directory('/output'),
buildDir: fileSystem.directory('/build'),
projectDir: fileSystem.directory('/project'),
final Environment result = Environment.test(
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Switched to Environment.test to pick up the defaults, including usage.

fileSystem.directory('/project'),
defines: defines,
inputs: <String, String>{},
cacheDir: fileSystem.directory('/cache'),
flutterRootDir: fileSystem.directory('/flutter_root'),
artifacts: Artifacts.test(),
fileSystem: fileSystem,
logger: logger,
processManager: FakeProcessManager.any(),
platform: FakePlatform(),
engineVersion: 'invalidEngineVersion',
generateDartPluginRegistry: false,
);
return result;
}
Expand Down
Loading