-
Notifications
You must be signed in to change notification settings - Fork 3.3k
[ci] Add a legacy Android build-all test #4005
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
Changes from 1 commit
ffa519c
4c37f43
216a43a
99f0f96
79c39e5
8191176
b30a4b4
e077b37
6a2b491
ece81a4
ce2bef3
4d8f614
ae1650d
3c6e658
ed1f5dc
a27dfa9
78455c6
27b2751
eeb2c16
453f909
8bab358
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
- Loading branch information
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,9 +2,8 @@ | |
// Use of this source code is governed by a BSD-style license that can be | ||
// found in the LICENSE file. | ||
|
||
import 'dart:io' as io; | ||
|
||
import 'package:file/file.dart'; | ||
import 'package:meta/meta.dart'; | ||
import 'package:path/path.dart' as p; | ||
import 'package:platform/platform.dart'; | ||
import 'package:pub_semver/pub_semver.dart'; | ||
|
@@ -15,26 +14,27 @@ import 'common/package_command.dart'; | |
import 'common/process_runner.dart'; | ||
import 'common/repository_package.dart'; | ||
|
||
const String _projectName = 'all_packages'; | ||
@visibleForTesting | ||
|
||
/// The name of the build-all-packages project. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can the comment state the intended format? For example There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done. |
||
const String allPackagesProjectName = 'all_packages'; | ||
|
||
const int _exitGenNativeBuildFilesFailed = 3; | ||
const int _exitMissingFile = 4; | ||
const int _exitFlutterCreateFailed = 3; | ||
const int _exitGenNativeBuildFilesFailed = 4; | ||
const int _exitMissingFile = 5; | ||
|
||
/// A command to create an application that builds all in a single application. | ||
class CreateAllPackagesAppCommand extends PackageCommand { | ||
/// Creates an instance of the builder command. | ||
CreateAllPackagesAppCommand( | ||
Directory packagesDir, { | ||
ProcessRunner processRunner = const ProcessRunner(), | ||
Directory? pluginsRoot, | ||
Platform platform = const LocalPlatform(), | ||
}) : super(packagesDir, processRunner: processRunner, platform: platform) { | ||
final Directory defaultDir = | ||
pluginsRoot ?? packagesDir.fileSystem.currentDirectory; | ||
argParser.addOption(_outputDirectoryFlag, | ||
defaultsTo: defaultDir.path, | ||
help: | ||
'The path the directory to create the "$_projectName" project in.\n' | ||
defaultsTo: packagesDir.parent.path, | ||
help: 'The path the directory to create the "$allPackagesProjectName" ' | ||
'project in.\n' | ||
'Defaults to the repository root.'); | ||
argParser.addOption(_agpVersionFlag, | ||
help: 'The AGP version to use in the created app, instead of the ' | ||
|
@@ -50,17 +50,20 @@ class CreateAllPackagesAppCommand extends PackageCommand { | |
help: 'The Gradle version to use in the created app, instead of the ' | ||
'default `flutter create`d version. Will generally need to be used ' | ||
' with $_agpVersionFlag due to compatibility limits.'); | ||
argParser.addMultiOption(_platformsFlag, | ||
help: 'A platforms list to pass to `flutter create`'); | ||
} | ||
|
||
static const String _androidLanguageFlag = 'android-language'; | ||
static const String _agpVersionFlag = 'agp-version'; | ||
static const String _gradleVersionFlag = 'gradle-version'; | ||
static const String _outputDirectoryFlag = 'output-dir'; | ||
static const String _platformsFlag = 'platforms'; | ||
|
||
/// The location to create the synthesized app project. | ||
Directory get _appDirectory => packagesDir.fileSystem | ||
.directory(getStringArg(_outputDirectoryFlag)) | ||
.childDirectory(_projectName); | ||
.childDirectory(allPackagesProjectName); | ||
|
||
/// The synthesized app project. | ||
RepositoryPackage get app => RepositoryPackage(_appDirectory); | ||
|
@@ -76,7 +79,8 @@ class CreateAllPackagesAppCommand extends PackageCommand { | |
Future<void> run() async { | ||
final int exitCode = await _createApp(); | ||
if (exitCode != 0) { | ||
throw ToolExit(exitCode); | ||
printError('Failed to `flutter create`: $exitCode'); | ||
throw ToolExit(_exitFlutterCreateFailed); | ||
} | ||
|
||
final Set<String> excluded = getExcludedPackageNames(); | ||
|
@@ -104,33 +108,44 @@ class CreateAllPackagesAppCommand extends PackageCommand { | |
} | ||
|
||
await Future.wait(<Future<void>>[ | ||
_updateTopLevelGradle(agpVersion: getNullableStringArg(_agpVersionFlag)), | ||
_updateGradleWrapper( | ||
gradleVersion: getNullableStringArg(_gradleVersionFlag)), | ||
_updateAppGradle(), | ||
_updateManifest(), | ||
_updateMacosPbxproj(), | ||
// This step requires the native file generation triggered by | ||
// flutter pub get above, so can't currently be run on Windows. | ||
if (!platform.isWindows) _updateMacosPodfile(), | ||
if (_targetPlatformIncludes(platformAndroid)) ...<Future<void>>[ | ||
_updateTopLevelGradle( | ||
agpVersion: getNullableStringArg(_agpVersionFlag)), | ||
_updateGradleWrapper( | ||
gradleVersion: getNullableStringArg(_gradleVersionFlag)), | ||
_updateAppGradle(), | ||
_updateManifest(), | ||
], | ||
if (_targetPlatformIncludes(platformMacOS)) ...<Future<void>>[ | ||
_updateMacosPbxproj(), | ||
// This step requires the native file generation triggered by | ||
// flutter pub get above, so can't currently be run on Windows. | ||
if (!platform.isWindows) _updateMacosPodfile(), | ||
], | ||
]); | ||
} | ||
|
||
/// True if the created app includes [platform]. | ||
bool _targetPlatformIncludes(String platform) { | ||
final List<String> platforms = getStringListArg(_platformsFlag); | ||
// An empty platform list means the app targets all platforms, since that's | ||
// how `flutter create` works. | ||
return platforms.contains(platform) || platforms.isEmpty; | ||
} | ||
|
||
Future<int> _createApp() async { | ||
final io.ProcessResult result = io.Process.runSync( | ||
final List<String> platforms = getStringListArg(_platformsFlag); | ||
return processRunner.runAndStream( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this modifying the current all packages test to use kotlin? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes; this is mentioned in the PR description. I'm not sure why it wasn't already, given that it's been the default for Flutter developers for a long time. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yeah I read that in the description and was looking for the place where we made the swap. Where in the cl do we test the legacy version with java? |
||
flutterCommand, | ||
<String>[ | ||
'create', | ||
if (platforms.isNotEmpty) '--platforms=${platforms.join(',')}', | ||
'--template=app', | ||
'--project-name=$_projectName', | ||
'--project-name=$allPackagesProjectName', | ||
'--android-language=${getStringArg(_androidLanguageFlag)}', | ||
_appDirectory.path, | ||
], | ||
); | ||
|
||
print(result.stdout); | ||
print(result.stderr); | ||
return result.exitCode; | ||
} | ||
|
||
/// Rewrites [file], replacing any lines contain a key in [replacements] with | ||
|
@@ -234,7 +249,7 @@ class CreateAllPackagesAppCommand extends PackageCommand { | |
_adjustFile( | ||
manifestFile, | ||
additions: <String, List<String>>{ | ||
'package="com.example.$_projectName"': <String>[ | ||
'package="com.example.$allPackagesProjectName"': <String>[ | ||
'xmlns:tools="http://schemas.android.com/tools">', | ||
'', | ||
'<uses-sdk tools:overrideLibrary="io.flutter.plugins.camera"/>', | ||
|
@@ -259,7 +274,7 @@ class CreateAllPackagesAppCommand extends PackageCommand { | |
final Map<String, PathDependency> pluginDeps = | ||
await _getValidPathDependencies(); | ||
final Pubspec pubspec = Pubspec( | ||
_projectName, | ||
allPackagesProjectName, | ||
description: 'Flutter app containing all 1st party plugins.', | ||
version: Version.parse('1.0.0+1'), | ||
environment: <String, VersionConstraint>{ | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Whitespace after visibleForTesting
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed, I forgot the autoformatter wants it after the declaration comment.