Skip to content

Commit

Permalink
Give channel descriptions in flutter channel, use branch instead of…
Browse files Browse the repository at this point in the history 
… upstream for channel name (#126936)

## How we determine the channel name

Historically, we used the current branch's upstream to figure out the current channel name. I have no idea why. I traced it back to https://github.com/flutter/flutter/pull/446/files where @abarth implement this and I reviewed that PR and left no comment on it at the time.

I think this is confusing. You can be on a branch and it tells you that your channel is different. That seems weird.

This PR changes the logic to uses the current branch as the channel name.

## How we display channels

The main reason this PR exists is to add channel descriptions to the `flutter channel` list:

```
ianh@burmese:~/dev/flutter/packages/flutter_tools$ flutter channel
Flutter channels:
  master (tip of tree, for contributors)
  main (tip of tree, follows master channel)
  beta (updated monthly, recommended for experienced users)
  stable (updated quarterly, for new users and for production app releases)
* foo_bar

Currently not on an official channel.
ianh@burmese:~/dev/flutter/packages/flutter_tools$
```

## Other changes

I made a few other changes while I was at it:

* If you're not on an official channel, we used to imply `--show-all`, but now we don't, we just show the official channels plus yours. This avoids flooding the screen in the case the user is on a weird channel and just wants to know what channel they're on.
* I made the tool more consistent about how it handles unofficial branches. Now it's always `[user branch]`.
* I slightly adjusted how unknown versions are rendered so it's clearer the version is unknown rather than just having the word "Unknown" floating in the output without context.
* Simplified some of the code.
* Made some of the tests more strict (checking all output rather than just some aspects of it).
* Changed the MockFlutterVersion to implement the FlutterVersion API more strictly.
* I made sure we escape the output to `.metadata` to avoid potential injection bugs (previously we just inlined the version and channel name verbatim with no escaping, which is super sketchy).
* Tweaked the help text for the `downgrade` command to be clearer.
* Removed some misleading text in some error messages.
* Made the `.metadata` generator consistent with the template file.
* Removed some obsolete code to do with the `dev` branch.

## Reviewer notes

I'm worried that there are implications to some of these changes that I am not aware of, so please don't assume I know what I'm doing when reviewing this code. :-)
  • Loading branch information
@Hixie
Hixie committed May 23, 2023
1 parent 25d2f90 commit 9c7a9e7
Show file tree
Hide file tree
Showing 20 changed files with 267 additions and 189 deletions.
2 changes: 1 addition & 1 deletion packages/flutter_tools/lib/src/base/user_messages.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ class UserMessages {

// Messages used in FlutterValidator
String flutterStatusInfo(String? channel, String? version, String os, String locale) =>
'Channel ${channel ?? 'unknown'}, ${version ?? 'Unknown'}, on $os, locale $locale';
'Channel ${channel ?? 'unknown'}, ${version ?? 'unknown version'}, on $os, locale $locale';
String flutterVersion(String version, String channel, String flutterRoot) =>
'Flutter version $version on channel $channel at $flutterRoot';
String get flutterUnknownChannel =>
Expand Down
37 changes: 18 additions & 19 deletions packages/flutter_tools/lib/src/commands/channel.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,13 +53,12 @@ class ChannelCommand extends FlutterCommand {

Future<void> _listChannels({ required bool showAll, required bool verbose }) async {
// Beware: currentBranch could contain PII. See getBranchName().
final String currentChannel = globals.flutterVersion.channel;
final String currentChannel = globals.flutterVersion.channel; // limited to known branch names
assert(kOfficialChannels.contains(currentChannel) || kObsoleteBranches.containsKey(currentChannel) || currentChannel == kUserBranch, 'potential PII leak in channel name: "$currentChannel"');
final String currentBranch = globals.flutterVersion.getBranchName();
final Set<String> seenUnofficialChannels = <String>{};
final List<String> rawOutput = <String>[];

showAll = showAll || currentChannel != currentBranch;

globals.printStatus('Flutter channels:');
final int result = await globals.processUtils.stream(
<String>['git', 'branch', '-r'],
Expand All @@ -74,8 +73,7 @@ class ChannelCommand extends FlutterCommand {
throwToolExit('List channels failed: $result$details', exitCode: result);
}

final List<String> officialChannels = kOfficialChannels.toList();
final List<bool> availableChannels = List<bool>.filled(officialChannels.length, false);
final Set<String> availableChannels = <String>{};

for (final String line in rawOutput) {
final List<String> split = line.split('/');
Expand All @@ -84,27 +82,25 @@ class ChannelCommand extends FlutterCommand {
continue;
}
final String branch = split[1];
if (split.length > 1) {
final int index = officialChannels.indexOf(branch);

if (index != -1) { // Mark all available channels official channels from output
availableChannels[index] = true;
} else if (showAll && !seenUnofficialChannels.contains(branch)) {
// add other branches to seenUnofficialChannels if --all flag is given (to print later)
seenUnofficialChannels.add(branch);
}
if (kOfficialChannels.contains(branch)) {
availableChannels.add(branch);
} else if (showAll) {
seenUnofficialChannels.add(branch);
}
}

bool currentChannelIsOfficial = false;

// print all available official channels in sorted manner
for (int i = 0; i < officialChannels.length; i++) {
for (final String channel in kOfficialChannels) {
// only print non-missing channels
if (availableChannels[i]) {
if (availableChannels.contains(channel)) {
String currentIndicator = ' ';
if (officialChannels[i] == currentChannel) {
if (channel == currentChannel) {
currentIndicator = '*';
currentChannelIsOfficial = true;
}
globals.printStatus('$currentIndicator ${officialChannels[i]}');
globals.printStatus('$currentIndicator $channel (${kChannelDescriptions[channel]})');
}
}

Expand All @@ -117,9 +113,12 @@ class ChannelCommand extends FlutterCommand {
globals.printStatus(' $branch');
}
}
} else if (!currentChannelIsOfficial) {
globals.printStatus('* $currentBranch');
}

if (currentChannel == 'unknown') {
if (!currentChannelIsOfficial) {
assert(currentChannel == kUserBranch, 'Current channel is "$currentChannel", which is not an official branch. (Current branch is "$currentBranch".)');
globals.printStatus('');
globals.printStatus('Currently not on an official channel.');
}
Expand Down
9 changes: 5 additions & 4 deletions packages/flutter_tools/lib/src/commands/create_base.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -407,8 +407,8 @@ abstract class CreateBase extends FlutterCommand {
'iosLanguage': iosLanguage,
'hasIosDevelopmentTeam': iosDevelopmentTeam != null && iosDevelopmentTeam.isNotEmpty,
'iosDevelopmentTeam': iosDevelopmentTeam ?? '',
'flutterRevision': globals.flutterVersion.frameworkRevision,
'flutterChannel': globals.flutterVersion.channel,
'flutterRevision': escapeYamlString(globals.flutterVersion.frameworkRevision),
'flutterChannel': escapeYamlString(globals.flutterVersion.getBranchName()), // may contain PII
'ios': ios,
'android': android,
'web': web,
Expand Down Expand Up @@ -571,10 +571,11 @@ abstract class CreateBase extends FlutterCommand {
final FlutterProjectMetadata metadata = FlutterProjectMetadata.explicit(
file: metadataFile,
versionRevision: globals.flutterVersion.frameworkRevision,
versionChannel: globals.flutterVersion.channel,
versionChannel: globals.flutterVersion.getBranchName(), // may contain PII
projectType: projectType,
migrateConfig: MigrateConfig(),
logger: globals.logger);
logger: globals.logger,
);
metadata.populate(
platforms: platformsForMigrateConfig,
projectDirectory: directory,
Expand Down
24 changes: 12 additions & 12 deletions packages/flutter_tools/lib/src/commands/downgrade.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,15 +47,15 @@ class DowngradeCommand extends FlutterCommand {
'working-directory',
hide: !verboseHelp,
help: 'Override the downgrade working directory. '
'This is only intended to enable integration testing of the tool itself.'
'This is only intended to enable integration testing of the tool itself. '
'It allows one to use the flutter tool from one checkout to downgrade a '
'different checkout.'
);
argParser.addFlag(
'prompt',
defaultsTo: true,
hide: !verboseHelp,
help: 'Show the downgrade prompt. '
'The ability to disable this using "--no-prompt" is only provided for '
'integration testing of the tool itself.'
help: 'Show the downgrade prompt.'
);
}

Expand Down Expand Up @@ -99,8 +99,8 @@ class DowngradeCommand extends FlutterCommand {
final Channel? channel = getChannelForName(currentChannel);
if (channel == null) {
throwToolExit(
'Flutter is not currently on a known channel. Use "flutter channel <name>" '
'to switch to an official channel.',
'Flutter is not currently on a known channel. '
'Use "flutter channel" to switch to an official channel. '
);
}
final PersistentToolState persistentToolState = _persistentToolState!;
Expand Down Expand Up @@ -153,23 +153,23 @@ class DowngradeCommand extends FlutterCommand {
} on ProcessException catch (error) {
throwToolExit(
'Unable to downgrade Flutter: The tool could not update to the version '
'$humanReadableVersion. This may be due to git not being installed or an '
'internal error. Please ensure that git is installed on your computer and '
'retry again.\nError: $error.'
'$humanReadableVersion.\n'
'Error: $error'
);
}
try {
await processUtils.run(
// The `--` bit (because it's followed by nothing) means that we don't actually change
// anything in the working tree, which avoids the need to first go into detached HEAD mode.
<String>['git', 'checkout', currentChannel, '--'],
throwOnError: true,
workingDirectory: workingDirectory,
);
} on ProcessException catch (error) {
throwToolExit(
'Unable to downgrade Flutter: The tool could not switch to the channel '
'$currentChannel. This may be due to git not being installed or an '
'internal error. Please ensure that git is installed on your computer '
'and retry again.\nError: $error.'
'$currentChannel.\n'
'Error: $error'
);
}
await FlutterVersion.resetFlutterVersionFreshnessCheck();
Expand Down
12 changes: 6 additions & 6 deletions packages/flutter_tools/lib/src/doctor.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -577,14 +577,14 @@ class FlutterValidator extends DoctorValidator {
ValidationMessage _getFlutterVersionMessage(String frameworkVersion, String versionChannel, String flutterRoot) {
String flutterVersionMessage = _userMessages.flutterVersion(frameworkVersion, versionChannel, flutterRoot);

// The tool sets the channel as "unknown", if the current branch is on a
// "detached HEAD" state or doesn't have an upstream, and sets the
// frameworkVersion as "0.0.0-unknown" if "git describe" on HEAD doesn't
// produce an expected format to be parsed for the frameworkVersion.
if (versionChannel != 'unknown' && frameworkVersion != '0.0.0-unknown') {
// The tool sets the channel as kUserBranch, if the current branch is on a
// "detached HEAD" state, doesn't have an upstream, or is on a user branch,
// and sets the frameworkVersion as "0.0.0-unknown" if "git describe" on
// HEAD doesn't produce an expected format to be parsed for the frameworkVersion.
if (versionChannel != kUserBranch && frameworkVersion != '0.0.0-unknown') {
return ValidationMessage(flutterVersionMessage);
}
if (versionChannel == 'unknown') {
if (versionChannel == kUserBranch) {
flutterVersionMessage = '$flutterVersionMessage\n${_userMessages.flutterUnknownChannel}';
}
if (frameworkVersion == '0.0.0-unknown') {
Expand Down
7 changes: 4 additions & 3 deletions packages/flutter_tools/lib/src/flutter_project_metadata.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import 'base/file_system.dart';
import 'base/logger.dart';
import 'base/utils.dart';
import 'project.dart';
import 'template.dart';
import 'version.dart';

enum FlutterProjectType implements CliEnum {
Expand Down Expand Up @@ -172,11 +173,11 @@ class FlutterProjectMetadata {
# This file tracks properties of this Flutter project.
# Used by Flutter tool to assess capabilities and perform upgrades etc.
#
# This file should be version controlled.
# This file should be version controlled and should not be manually edited.
version:
revision: $_versionRevision
channel: $_versionChannel
revision: ${escapeYamlString(_versionRevision ?? '')}
channel: ${escapeYamlString(_versionChannel ?? kUserBranch)}
project_type: ${projectType == null ? '' : projectType!.cliName}
${migrateConfig.getOutputFileString()}''';
Expand Down
3 changes: 0 additions & 3 deletions packages/flutter_tools/lib/src/globals.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -300,9 +300,6 @@ CustomDevicesConfig get customDevicesConfig => context.get<CustomDevicesConfig>(

PreRunValidator get preRunValidator => context.get<PreRunValidator>() ?? const NoOpPreRunValidator();

// TODO(fujino): Migrate to 'main' https://github.com/flutter/flutter/issues/95041
const String kDefaultFrameworkChannel = 'master';

// Used to build RegExp instances which can detect the VM service message.
final RegExp kVMServiceMessageRegExp = RegExp(r'The Dart VM service is listening on ((http|//)[a-zA-Z0-9:/=_\-\.\[\]]+)');

Expand Down
21 changes: 21 additions & 0 deletions packages/flutter_tools/lib/src/template.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -375,3 +375,24 @@ String _escapeKotlinKeywords(String androidIdentifier) {
).toList();
return correctedSegments.join('.');
}

String escapeYamlString(String value) {
final StringBuffer result = StringBuffer();
result.write('"');
for (final int rune in value.runes) {
result.write(
switch (rune) {
0x00 => r'\0',
0x09 => r'\t',
0x0A => r'\n',
0x0D => r'\r',
0x22 => r'\"',
0x5C => r'\\',
< 0x20 => '\\x${rune.toRadixString(16).padLeft(2, "0")}',
_ => String.fromCharCode(rune),
}
);
}
result.write('"');
return result.toString();
}
58 changes: 32 additions & 26 deletions packages/flutter_tools/lib/src/version.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,9 @@ const String _unknownFrameworkVersion = '0.0.0-unknown';
/// See `man gitrevisions` for more information.
const String kGitTrackingUpstream = '@{upstream}';

/// Replacement name when the branch is user-specific.
const String kUserBranch = '[user-branch]';

/// This maps old branch names to the names of branches that replaced them.
///
/// For example, in 2021 we deprecated the "dev" channel and transitioned "dev"
Expand All @@ -40,12 +43,24 @@ enum Channel {

// Beware: Keep order in accordance with stability
const Set<String> kOfficialChannels = <String>{
globals.kDefaultFrameworkChannel,
'master',
'main',
'beta',
'stable',
};

const Map<String, String> kChannelDescriptions = <String, String>{
'master': 'latest development branch, for contributors',
'main': 'latest development branch, follows master channel',
'beta': 'updated monthly, recommended for experienced users',
'stable': 'updated quarterly, for new users and for production app releases',
};

const Set<String> kDevelopmentChannels = <String>{
'master',
'main',
};

/// Retrieve a human-readable name for a given [channel].
///
/// Requires [kOfficialChannels] to be correctly ordered.
Expand Down Expand Up @@ -101,16 +116,7 @@ class FlutterVersion {

String? _repositoryUrl;
String? get repositoryUrl {
final String _ = channel;
return _repositoryUrl;
}

String? _channel;
/// The channel is the upstream branch.
/// `master`, `dev`, `beta`, `stable`; or old ones, like `alpha`, `hackathon`, ...
String get channel {
String? channel = _channel;
if (channel == null) {
if (_repositoryUrl == null) {
final String gitChannel = _runGit(
'git rev-parse --abbrev-ref --symbolic $kGitTrackingUpstream',
globals.processUtils,
Expand All @@ -124,14 +130,16 @@ class FlutterVersion {
globals.processUtils,
_workingDirectory,
);
channel = gitChannel.substring(slash + 1);
} else if (gitChannel.isEmpty) {
channel = 'unknown';
} else {
channel = gitChannel;
}
_channel = channel;
}
return _repositoryUrl;
}

/// The channel is the current branch if we recognize it, or "[user-branch]" (kUserBranch).
/// `master`, `beta`, `stable`; or old ones, like `alpha`, `hackathon`, `dev`, ...
String get channel {
final String channel = getBranchName(redactUnknownBranches: true);
assert(kOfficialChannels.contains(channel) || kObsoleteBranches.containsKey(channel) || channel == kUserBranch, 'Potential PII leak in channel name: "$channel"');
return channel;
}

Expand Down Expand Up @@ -296,16 +304,16 @@ class FlutterVersion {
/// Return the branch name.
///
/// If [redactUnknownBranches] is true and the branch is unknown,
/// the branch name will be returned as `'[user-branch]'`.
/// the branch name will be returned as `'[user-branch]'` ([kUserBranch]).
String getBranchName({ bool redactUnknownBranches = false }) {
_branch ??= () {
final String branch = _runGit('git rev-parse --abbrev-ref HEAD', globals.processUtils);
return branch == 'HEAD' ? channel : branch;
final String branch = _runGit('git symbolic-ref --short HEAD', globals.processUtils, _workingDirectory);
return branch == 'HEAD' ? '' : branch;
}();
if (redactUnknownBranches || _branch!.isEmpty) {
// Only return the branch names we know about; arbitrary branch names might contain PII.
if (!kOfficialChannels.contains(_branch) && !kObsoleteBranches.containsKey(_branch)) {
return '[user-branch]';
return kUserBranch;
}
}
return _branch!;
Expand Down Expand Up @@ -619,7 +627,7 @@ String _runSync(List<String> command, { bool lenient = true }) {
return '';
}

String _runGit(String command, ProcessUtils processUtils, [String? workingDirectory]) {
String _runGit(String command, ProcessUtils processUtils, String? workingDirectory) {
return processUtils.runSync(
command.split(' '),
workingDirectory: workingDirectory ?? Cache.flutterRoot,
Expand Down Expand Up @@ -709,8 +717,8 @@ class GitTagVersion {
String gitRef = 'HEAD'
}) {
if (fetchTags) {
final String channel = _runGit('git rev-parse --abbrev-ref HEAD', processUtils, workingDirectory);
if (channel == 'dev' || channel == 'beta' || channel == 'stable') {
final String channel = _runGit('git symbolic-ref --short HEAD', processUtils, workingDirectory);
if (!kDevelopmentChannels.contains(channel) && kOfficialChannels.contains(channel)) {
globals.printTrace('Skipping request to fetchTags - on well known channel $channel.');
} else {
final String flutterGit = platform.environment['FLUTTER_GIT_URL'] ?? 'https://github.com/flutter/flutter.git';
Expand Down Expand Up @@ -918,8 +926,6 @@ class VersionFreshnessValidator {
return const Duration(days: 365 ~/ 2); // Six months
case 'beta':
return const Duration(days: 7 * 8); // Eight weeks
case 'dev':
return const Duration(days: 7 * 4); // Four weeks
default:
return const Duration(days: 7 * 3); // Three weeks
}
Expand Down

0 comments on commit 9c7a9e7

Please sign in to comment.