New release process #14061

Hixie · 2018-01-12T02:25:59Z

Generate the "version" file from git tags.
Remove the old VERSION file and mentions of versions in pubspec.yaml files.
Replace the old update_versions.dart script with a new roll_dev.dart script.
Update "flutter channel".
Update "flutter upgrade", including making it transition from alpha to dev.
Update "flutter --version" and "flutter doctor".

Hixie · 2018-01-12T02:58:10Z

cc @tvolkert for review

I've already landed a separate change that renames all the tags from the old format ("0.0.20") to the new one ("v0.0.20"). Having the "v" will make it easier to add other kinds of tags later. Also I removed an extremely old and confusing "v0.0.20-alpha" tag.

Hixie · 2018-01-12T02:58:50Z

I've also updated https://github.com/flutter/flutter/wiki/Release-process accordingly already.

Hixie · 2018-01-12T03:05:46Z

I've also created the "beta" and "dev" branches. Right now they're at the old v0.0.20 build.

tvolkert · 2018-01-12T17:33:02Z

dev/tools/dartdoc.dart

+  final StringBuffer buf = new StringBuffer();
+  buf.writeln('name: Flutter');
+  buf.writeln('homepage: https://flutter.io');
+  buf.writeln('version: $version');


The Travis doc script is complaining about "version: unknown"

tvolkert · 2018-01-12T18:22:45Z

packages/flutter_tools/lib/src/commands/channel.dart


 class ChannelCommand extends FlutterCommand {
+  ChannelCommand({ bool verboseHelp: false }) {
+    argParser.addFlag(
+      'all',


abbreviation of a?

tvolkert · 2018-01-12T18:24:52Z

packages/flutter_tools/lib/src/commands/channel.dart

-    final String currentBranch = runSync(
-        <String>['git', 'rev-parse', '--abbrev-ref', 'HEAD'],
-        workingDirectory: Cache.flutterRoot);
+  static Future<Null> listChannels({ bool showAll }) async {


Why public? And if it were still private, it could be an instance method that just refers to argResults directly.

Made it private again. I had thought I was going to use this from other files, but only upgradeChannel ended up being used.

I left it using an argument rather than argResults.

tvolkert · 2018-01-12T18:28:42Z

packages/flutter_tools/lib/src/commands/channel.dart

      },
    );
    if (result != 0)
      throwToolExit('List channels failed: $result', exitCode: result);
  }

-  Future<Null> _switchChannel(String branchName) async {
-    printStatus('Switching to flutter channel named $branchName');
+  static Future<Null> switchChannel(String branchName) {


why public?

tvolkert · 2018-01-12T18:29:39Z

packages/flutter_tools/lib/src/version.dart

+    'release',
+  ]);
+
+  static Map<String, String> obsoleteBranches = <String, String>{


Add docs explaining what this maps to and from

tvolkert · 2018-01-12T18:36:11Z

packages/flutter_tools/lib/src/version.dart

@@ -42,18 +35,47 @@ class FlutterVersion {

    _frameworkRevision = _runGit('git log -n 1 --pretty=format:%H');
    _frameworkAge = _runGit('git log -n 1 --pretty=format:%ar');
+
+    final String version = _runGit('git describe --match v*.*.* --exclude *-* --first-parent --long --tags');


That's some serious voodoo right there

tvolkert · 2018-01-12T18:48:53Z

packages/flutter_tools/lib/src/version.dart

@@ -42,18 +35,47 @@ class FlutterVersion {

    _frameworkRevision = _runGit('git log -n 1 --pretty=format:%H');
    _frameworkAge = _runGit('git log -n 1 --pretty=format:%ar');
+
+    final String version = _runGit('git describe --match v*.*.* --exclude *-* --first-parent --long --tags');
+    final RegExp versionPattern = new RegExp('^v([0-9]+)\.([0-9]+)\.([0-9]+\)-([0-9]+)-g([a-f0-9]+)\$');


It's hard to read this and know which fields represent what pieces. Can you encapsulate this in a class that takes the output of the git command in its constructor and exposes meaningfully-named fields for each of the pieces it extracts from the regexp?

What about using https://pub.dartlang.org/packages/pub_semver instead? (used in pub AFAIK)

@zoechi Instead of what?

Parsing the version, pumping major/minor/patch/dev, comparing versions
Just wanted to bring it to your attention in case you were not aware of it

The version that is being parsed here isn't a semver version, it's a git tag description long format.

As far as adding 1 is concerned, I don't think we need to use a library for that. :-) We're trying to minimize our dependencies, so a library would have to do something a lot more complicated than that to justify using it.

@tvolkert done

tvolkert · 2018-01-12T18:53:13Z

packages/flutter_tools/lib/src/version.dart

@@ -73,14 +98,18 @@ class FlutterVersion {

  String _runGit(String command) => runSync(command.split(' '), workingDirectory: Cache.flutterRoot);

+  void ensureVersionFile() {


Should this just be written out during construction?

If it does make sense to keep this as a separate method called by the command runner, then lat's make it async.

i'd rather not have a constructor that mutates the world (notwithstanding that it already runs subprocesses and stuff, i just don't want to make it worse...). But I've made it async.

tvolkert · 2018-01-12T19:27:31Z

packages/flutter_tools/lib/src/version.dart

@@ -158,12 +187,12 @@ class FlutterVersion {
  /// Return the branch name.
  ///
  /// If [whitelistBranchName] is true and the branch is unknown,
-  /// the branch name will be returned as 'dev'.
+  /// the branch name will be returned as '[user-branch]'.


Nit: surround '[user-branch]' in backticks so Dartdoc knows not to try to resolve it as a reference

tvolkert · 2018-01-12T19:29:24Z

packages/flutter_tools/lib/src/version.dart

  @override
  String toString() {
-    final String flutterText = 'Flutter • channel $channel • ${repositoryUrl == null ? 'unknown source' : repositoryUrl}';
+    final String flutterText = 'Flutter $_frameworkVersion • channel $channel • ${repositoryUrl == null ? 'unknown source' : repositoryUrl}';


Create a separate • version $_frameworkVersion bullet. Otherwise they might see "Flutter unknown", which would be confusing.

i've special-cased the unknown version case to not display the string, so it just says "Flutter" in that case.

tvolkert · 2018-01-12T19:29:42Z

packages/flutter_tools/lib/src/version.dart

    final String frameworkText = 'Framework • revision $frameworkRevisionShort ($frameworkAge) • $frameworkCommitDate';
    final String engineText = 'Engine • revision $engineRevisionShort';
    final String toolsText = 'Tools • Dart $dartSdkVersion';

-    // Flutter • channel master • https://github.com/flutter/flutter.git
+    // Flutter v1.3.922-pre.2 • channel master • https://github.com/flutter/flutter.git


And then update this to match

tvolkert · 2018-01-12T19:41:40Z

packages/flutter_tools/lib/src/commands/channel.dart

-        <String>['git', 'rev-parse', '--abbrev-ref', 'HEAD'],
-        workingDirectory: Cache.flutterRoot);
+  static Future<Null> listChannels({ bool showAll }) async {
+    // Beware: currentBranch could contain PII. See getBranchName().


But getBranchName() redacts it if it's not a known branch name, so we should be safe here

Ah, scratch that - we don't set whitelistBranchName to true here

tvolkert · 2018-01-12T19:46:58Z

packages/flutter_tools/lib/src/version.dart

@@ -158,12 +187,12 @@ class FlutterVersion {
  /// Return the branch name.
  ///
  /// If [whitelistBranchName] is true and the branch is unknown,
-  /// the branch name will be returned as 'dev'.
+  /// the branch name will be returned as '[user-branch]'.
  String getBranchName({ bool whitelistBranchName: false }) {


whitelistBranchName seems like a weird name for this argument... it's more like redactUnknownBranches

Yeah. Fixed.

tvolkert · 2018-01-12T19:56:49Z

dev/tools/roll_dev.dart

@@ -0,0 +1,150 @@
+// Copyright 2017 The Chromium Authors. All rights reserved.


wdyt about creating dev/tools/lib and putting this there? It'd allow things like Google's roll script to import this and run it inline rather than having to spawn a new Dart process.

tvolkert · 2018-01-12T19:59:02Z

dev/tools/roll_dev.dart

+      print('stdout from git:\n${result.stdout}\n');
+    if (result.stderr.isNotEmpty)
+      print('stderr from git:\n${result.stderr}\n');
+    exit(1);


result.exitCode instead of 1?

In general I'm not a fan of propagating exit codes that way, because it's essentially merging two namespaces (theirs and ours and any other processes we might do this to). In practice what matters is 0 vs 1.

tvolkert · 2018-01-12T19:59:17Z

dev/tools/roll_dev.dart

+      print('stderr from git:\n${result.stderr}\n');
+    exit(1);
+  }
+}


Newline at end of file

tvolkert · 2018-01-12T20:00:02Z

dev/tools/roll_dev.dart

+  return null; // for the analyzer's sake
+}
+
+void runGit(String command, String explanation) {


Seems like this could be unified with getGitOutput()

factored out common stuff

tvolkert · 2018-01-12T20:02:43Z

dev/tools/roll_dev.dart

+    case kZ:
+      parts[2] += 1;
+      break;
+  }


Default case exits with a failure

goderbauer · 2018-01-16T17:55:21Z

bin/flutter

@@ -73,6 +73,7 @@ function upgrade_flutter () {

  local revision=`(cd "$FLUTTER_ROOT"; git rev-parse HEAD)`
  if [ ! -f "$SNAPSHOT_PATH" ] || [ ! -s "$STAMP_PATH" ] || [ `cat "$STAMP_PATH"` != "$revision" ] || [ "$FLUTTER_TOOLS_DIR/pubspec.yaml" -nt "$FLUTTER_TOOLS_DIR/pubspec.lock" ]; then
+    rm -f "$FLUTTER_ROOT/version"


Does this need to be mirrored in the Windows version of the script (flutter.bat)?

Added:

IF EXIST "%FLUTTER_ROOT%\version" DEL %FLUTTER_ROOT%\version

...just after :do_snapshot.

You probably want the argument to DEL in quotes as well, just in case there are spaces in FLUTTER_ROOT.

goderbauer · 2018-01-16T21:00:42Z

bin/flutter.bat

@@ -93,6 +93,7 @@ GOTO :after_subroutine
    )

  :do_snapshot
+    IF EXIST "%FLUTTER_ROOT%\version" DEL %FLUTTER_ROOT%\version


Quotes around the argument to DEL, just in case there a spaces in the FLUTTER_ROOT path?

Hixie · 2018-01-17T17:05:49Z

@tvolkert PTAL

goderbauer

LGTM modulo one comment

goderbauer · 2018-01-18T00:54:29Z

dev/tools/lib/roll_dev.dart

+
+  switch (level) {
+    case kX:
+      parts[0] += 1;


Shouldn't this also set parts[1] and parts[2] to 0? You want to go from 1.3.6 to 2.0.0 and not 2.3.6, right?

goderbauer · 2018-01-18T00:56:03Z

dev/tools/lib/roll_dev.dart

+      parts[0] += 1;
+      break;
+    case kY:
+      parts[1] += 1;


same, but only for parts[2].

Generate the "version" file from git tags. Remove the old VERSION file and mentions of versions in pubspec.yaml files. Replace the old update_versions.dart script with a new roll_dev.dart script. Update "flutter channel". Update "flutter upgrade", including making it transition from alpha to dev. Update "flutter --version" and "flutter doctor".

Hixie · 2018-01-18T01:43:40Z

Updated as suggested. Good catch!

Thanks. Will land on green and roll dev shortly thereafter.

tvolkert · 2018-01-18T17:15:36Z

dev/tools/lib/roll_dev.dart

+
+  print('Your tree is ready to publish Flutter $version to the "dev" channel.');
+  stdout.write('Are you? [yes/no] ');
+  if (stdin.readLineSync() != 'yes') {


We may want to respect 'y' as well.

Two extra characters per person every few months is probably a cost worth paying for the extra level of confidence in the confirmation. :-P

tvolkert

LGTM

Generate the "version" file from git tags. Remove the old VERSION file and mentions of versions in pubspec.yaml files. Replace the old update_versions.dart script with a new roll_dev.dart script. Update "flutter channel". Update "flutter upgrade", including making it transition from alpha to dev. Update "flutter --version" and "flutter doctor".

git@github.com:flutter/engine.git/compare/6c605f8a9624...8672e79 git log 6c605f8..8672e79 --first-parent --oneline 2019-11-28 skia-flutter-autoroll@skia.org Roll src/third_party/skia c96f5108df28..73beaaa48fcc (2 commits) (#14065) 2019-11-28 hterkelsen@users.noreply.github.com Fallback to Roboto if no suitable font is found (#14061) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll Please CC cbracken@google.com on the revert to ensure that a human is aware of the problem. To report a problem with the AutoRoller itself, please file a bug: https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+/master/autoroll/README.md

googlebot added the cla: yes label Jan 12, 2018

Hixie force-pushed the version branch from 899d114 to 3716127 Compare January 12, 2018 03:15

tvolkert reviewed Jan 12, 2018

View reviewed changes

goderbauer reviewed Jan 16, 2018

View reviewed changes

Hixie force-pushed the version branch from 3716127 to bdda65f Compare January 16, 2018 20:59

goderbauer reviewed Jan 16, 2018

View reviewed changes

Hixie force-pushed the version branch 2 times, most recently from edb0c6e to 24bc52b Compare January 16, 2018 21:31

goderbauer approved these changes Jan 18, 2018

View reviewed changes

Hixie force-pushed the version branch from c7eaf7d to a49c3fe Compare January 18, 2018 01:43

Hixie merged commit 9e42e4b into flutter:master Jan 18, 2018

Hixie deleted the version branch January 18, 2018 15:59

tvolkert reviewed Jan 18, 2018

View reviewed changes

devoncarew mentioned this pull request Jan 20, 2018

Intellij reported that flutter plugin crashed. flutter/flutter-intellij#1646

Closed

jcollins-g mentioned this pull request Jan 25, 2018

dartdoc's flutter travis shard is broken dart-lang/dartdoc#1596

Closed

aam mentioned this pull request Feb 6, 2018

version.dart fails to parse git tag version with 'alpha' in it #14496

Closed

This was referenced Nov 29, 2019

Roll engine 6c605f8a9624..8672e79af873 (2 commits) #45817

Merged

Roll engine 6c605f8a9624..8672e79af873 (2 commits) #45918

Merged

github-actions bot locked as resolved and limited conversation to collaborators Aug 12, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

New release process #14061

New release process #14061

		@@ -73,14 +98,18 @@ class FlutterVersion {

		String _runGit(String command) => runSync(command.split(' '), workingDirectory: Cache.flutterRoot);

		void ensureVersionFile() {

		@@ -0,0 +1,150 @@
		// Copyright 2017 The Chromium Authors. All rights reserved.

New release process #14061

New release process #14061

Conversation

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment