[flutter_tools] check first for stable tag, then dev tag (#55342)

packages/flutter_tools/lib/src/version.dart

@@ -752,80 +752,83 @@ class GitTagVersion {
         _runGit('git fetch $_flutterGit --tags', processUtils, workingDirectory);
       }
     }
-    // `--match` glob must match old version tag `v1.2.3` and new `1.2.3-dev.4.5`
-    return parse(_runGit('git describe --match *.*.* --first-parent --long --tags', processUtils, workingDirectory));
-  }
-
-  // TODO(fujino): Deprecate this https://github.com/flutter/flutter/issues/53850
-  /// Check for the release tag format of the form x.y.z-dev.m.n
-  static GitTagVersion parseLegacyVersion(String version) {
-    final RegExp versionPattern = RegExp(
-      r'^([0-9]+)\.([0-9]+)\.([0-9]+)(-dev\.[0-9]+\.[0-9]+)?-([0-9]+)-g([a-f0-9]+)$');
-    final List<String> parts = versionPattern.matchAsPrefix(version)?.groups(<int>[1, 2, 3, 4, 5, 6]);
-    if (parts == null) {
-      return const GitTagVersion.unknown();
+    final List<String> tags = _runGit(
+      'git tag --contains HEAD', processUtils, workingDirectory).split('\n');
+
+    // Check first for a stable tag
+    final RegExp stableTagPattern = RegExp(r'^\d+\.\d+\.\d+$');
+    for (final String tag in tags) {
+      final String trimmedTag = tag.trim();
+      if (stableTagPattern.hasMatch(trimmedTag)) {
+        return parse(trimmedTag);
+      }
     }
-    final List<int> parsedParts = parts.take(5).map<int>(
-      (String source) => source == null ? null : int.tryParse(source)).toList();
-    List<int> devParts = <int>[null, null];
-    if (parts[3] != null) {
-      devParts = RegExp(r'^-dev\.(\d+)\.(\d+)')
-        .matchAsPrefix(parts[3])
-        ?.groups(<int>[1, 2])
-        ?.map<int>(
-          (String source) => source == null ? null : int.tryParse(source)
-        )?.toList() ?? <int>[null, null];
+    // Next check for a dev tag
+    final RegExp devTagPattern = RegExp(r'^\d+\.\d+\.\d+-\d+\.\d+\.pre$');
+    for (final String tag in tags) {
+      final String trimmedTag = tag.trim();
+      if (devTagPattern.hasMatch(trimmedTag)) {
+        return parse(trimmedTag);
+      }
     }
-    return GitTagVersion(
-      x: parsedParts[0],
-      y: parsedParts[1],
-      z: parsedParts[2],
-      devVersion: devParts[0],
-      devPatch: devParts[1],
-      commits: parsedParts[4],
-      hash: parts[5],
-      gitTag: '${parts[0]}.${parts[1]}.${parts[2]}${parts[3] ?? ''}', // x.y.z-dev.m.n
+
+    // If we're not currently on a tag, use git describe to find the most
+    // recent tag and number of commits past.
+    return parse(
+      _runGit(
+        'git describe --match *.*.*-*.*.pre --first-parent --long --tags',
+        processUtils,
+        workingDirectory,
+      )
     );
   }
 
-  /// Check for the release tag format of the form x.y.z-m.n.pre
+  /// Parse a version string.
+  ///
+  /// The version string can either be an exact release tag (e.g. '1.2.3' for
+  /// stable or 1.2.3-4.5.pre for a dev) or the output of `git describe` (e.g.
+  /// for commit abc123 that is 6 commits after tag 1.2.3-4.5.pre, git would
+  /// return '1.2.3-4.5.pre-6-gabc123').
   static GitTagVersion parseVersion(String version) {
     final RegExp versionPattern = RegExp(
-      r'^(\d+)\.(\d+)\.(\d+)(-\d+\.\d+\.pre)?-(\d+)-g([a-f0-9]+)$');
-    final List<String> parts = versionPattern.matchAsPrefix(version)?.groups(<int>[1, 2, 3, 4, 5, 6]);
-    if (parts == null) {
+      r'^(\d+)\.(\d+)\.(\d+)(-\d+\.\d+\.pre)?(?:-(\d+)-g([a-f0-9]+))?$');
+    final Match match = versionPattern.firstMatch(version.trim());
+    if (match == null) {
       return const GitTagVersion.unknown();
     }
-    final List<int> parsedParts = parts.take(5).map<int>(
-      (String source) => source == null ? null : int.tryParse(source)).toList();
-    List<int> devParts = <int>[null, null];
-    if (parts[3] != null) {
-      devParts = RegExp(r'^-(\d+)\.(\d+)\.pre')
-        .matchAsPrefix(parts[3])
-        ?.groups(<int>[1, 2])
-        ?.map<int>(
-          (String source) => source == null ? null : int.tryParse(source)
-        )?.toList() ?? <int>[null, null];
+
+    final List<String> matchGroups = match.groups(<int>[1, 2, 3, 4, 5, 6]);
+    final int x = matchGroups[0] == null ? null : int.tryParse(matchGroups[0]);
+    final int y = matchGroups[1] == null ? null : int.tryParse(matchGroups[1]);
+    final int z = matchGroups[2] == null ? null : int.tryParse(matchGroups[2]);
+    final String devString = matchGroups[3];
+    int devVersion, devPatch;
+    if (devString != null) {
+      final Match devMatch = RegExp(r'^-(\d+)\.(\d+)\.pre$')
+        .firstMatch(devString);
+      final List<String> devGroups = devMatch.groups(<int>[1, 2]);
+      devVersion = devGroups[0] == null ? null : int.tryParse(devGroups[0]);
+      devPatch = devGroups[1] == null ? null : int.tryParse(devGroups[1]);
     }
+    // count of commits past last tagged version
+    final int commits = matchGroups[4] == null ? 0 : int.tryParse(matchGroups[4]);
+    final String hash = matchGroups[5] ?? '';
+
     return GitTagVersion(
-      x: parsedParts[0],
-      y: parsedParts[1],
-      z: parsedParts[2],
-      devVersion: devParts[0],
-      devPatch: devParts[1],
-      commits: parsedParts[4],
-      hash: parts[5],
-      gitTag: '${parts[0]}.${parts[1]}.${parts[2]}${parts[3] ?? ''}', // x.y.z-m.n.pre
+      x: x,
+      y: y,
+      z: z,
+      devVersion: devVersion,
+      devPatch: devPatch,
+      commits: commits,
+      hash: hash,
+      gitTag: '$x.$y.$z${devString ?? ''}', // e.g. 1.2.3-4.5.pre
     );
   }
 
   static GitTagVersion parse(String version) {
     GitTagVersion gitTagVersion;
 
-    gitTagVersion = parseLegacyVersion(version);
-    if (gitTagVersion != const GitTagVersion.unknown()) {
-      return gitTagVersion;
-    }
     gitTagVersion = parseVersion(version);
     if (gitTagVersion != const GitTagVersion.unknown()) {
       return gitTagVersion;

packages/flutter_tools/test/commands.shard/hermetic/version_test.dart

@@ -259,7 +259,7 @@ class MockProcessManager extends Mock implements ProcessManager {
       return ProcessResult(0, 0, '000000000000000000000', '');
     }
     if (commandStr ==
-        'git describe --match *.*.* --first-parent --long --tags') {
+        'git describe --match *.*.*-*.*.pre --first-parent --long --tags') {
       if (version.isNotEmpty) {
         return ProcessResult(0, 0, '$version-0-g00000000', '');
       }

packages/flutter_tools/test/commands.shard/permeable/upgrade_test.dart

@@ -238,7 +238,13 @@ void main() {
         fakeProcessManager = FakeProcessManager.list(<FakeCommand>[
           const FakeCommand(
             command: <String>[
-              'git', 'describe', '--match', '*.*.*', '--first-parent', '--long', '--tags',
+              'git', 'tag', '--contains', 'HEAD',
+            ],
+            stdout: '',
+          ),
+          const FakeCommand(
+            command: <String>[
+              'git', 'describe', '--match', '*.*.*-*.*.pre', '--first-parent', '--long', '--tags',
             ],
             stdout: 'v1.12.16-19-gb45b676af',
           ),

packages/flutter_tools/test/general.shard/runner/flutter_command_runner_test.dart

@@ -185,7 +185,9 @@ void main() {
           workingDirectory: Cache.flutterRoot)).thenReturn(result);
         when(processManager.runSync('git fetch https://github.com/flutter/flutter.git --tags'.split(' '),
           workingDirectory: Cache.flutterRoot)).thenReturn(result);
-        when(processManager.runSync('git describe --match *.*.* --first-parent --long --tags'.split(' '),
+        when(processManager.runSync('git tag --contains HEAD'.split(' '),
+          workingDirectory: Cache.flutterRoot)).thenReturn(result);
+        when(processManager.runSync('git describe --match *.*.*-*.*.pre --first-parent --long --tags'.split(' '),
           workingDirectory: Cache.flutterRoot)).thenReturn(result);
         when(processManager.runSync(FlutterVersion.gitLog('-n 1 --pretty=format:%ad --date=iso'.split(' ')),
           workingDirectory: Cache.flutterRoot)).thenReturn(result);

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

@@ -394,16 +394,25 @@ void main() {
     const String hash = 'abcdef';
     GitTagVersion gitTagVersion;
 
-    // legacy tag format (x.y.z-dev.m.n), master channel
-    gitTagVersion = GitTagVersion.parse('1.2.3-dev.4.5-4-g$hash');
-    expect(gitTagVersion.frameworkVersionFor(hash), '1.2.3-5.0.pre.4');
-    expect(gitTagVersion.gitTag, '1.2.3-dev.4.5');
+    // Master channel
+    gitTagVersion = GitTagVersion.parse('1.2.3-4.5.pre-13-g$hash');
+    expect(gitTagVersion.frameworkVersionFor(hash), '1.2.3-5.0.pre.13');
+    expect(gitTagVersion.gitTag, '1.2.3-4.5.pre');
     expect(gitTagVersion.devVersion, 4);
     expect(gitTagVersion.devPatch, 5);
 
-    // new tag release format, master channel
-    gitTagVersion = GitTagVersion.parse('1.2.3-4.5.pre-13-g$hash');
-    expect(gitTagVersion.frameworkVersionFor(hash), '1.2.3-5.0.pre.13');
+    // Stable channel
+    gitTagVersion = GitTagVersion.parse('1.2.3');
+    expect(gitTagVersion.frameworkVersionFor(hash), '1.2.3');
+    expect(gitTagVersion.x, 1);
+    expect(gitTagVersion.y, 2);
+    expect(gitTagVersion.z, 3);
+    expect(gitTagVersion.devVersion, null);
+    expect(gitTagVersion.devPatch, null);
+
+    // Dev channel
+    gitTagVersion = GitTagVersion.parse('1.2.3-4.5.pre');
+    expect(gitTagVersion.frameworkVersionFor(hash), '1.2.3-4.5.pre');
     expect(gitTagVersion.gitTag, '1.2.3-4.5.pre');
     expect(gitTagVersion.devVersion, 4);
     expect(gitTagVersion.devPatch, 5);
@@ -448,6 +457,29 @@ void main() {
     );
   });
 
+  testUsingContext('determine favors stable tags over dev tags', () {
+    final MockProcessUtils mockProcessUtils = MockProcessUtils();
+    when(mockProcessUtils.runSync(
+      <String>['git', 'tag', '--contains', 'HEAD'],
+      workingDirectory: anyNamed('workingDirectory'),
+      environment: anyNamed('environment'),
+    )).thenReturn(RunResult(
+      ProcessResult(1, 0, '1.2.3-0.0.pre\n1.2.3\n1.2.3-0.1.pre', ''),
+      <String>['git', 'tag', '--contains', 'HEAD'],
+    ));
+    final GitTagVersion version = GitTagVersion.determine(mockProcessUtils, workingDirectory: '.');
+    expect(version.gitTag, '1.2.3');
+    expect(version.devPatch, null);
+    expect(version.devVersion, null);
+    // We shouldn't have to fallback to git describe, because we are exactly
+    // on a release tag.
+    verifyNever(mockProcessUtils.runSync(
+      <String>['git', 'describe', '--match', '*.*.*-*.*.pre', '--first-parent', '--long', '--tags'],
+      workingDirectory: anyNamed('workingDirectory'),
+      environment: anyNamed('environment'),
+    ));
+  });
+
   testUsingContext('determine does not call fetch --tags', () {
     final MockProcessUtils processUtils = MockProcessUtils();
     when(processUtils.runSync(
@@ -456,10 +488,18 @@ void main() {
       environment: anyNamed('environment'),
     )).thenReturn(RunResult(ProcessResult(105, 0, '', ''), <String>['git', 'fetch']));
     when(processUtils.runSync(
-      <String>['git', 'describe', '--match', '*.*.*', '--first-parent', '--long', '--tags'],
+      <String>['git', 'describe', '--match', '*.*.*-*.*.pre', '--first-parent', '--long', '--tags'],
       workingDirectory: anyNamed('workingDirectory'),
       environment: anyNamed('environment'),
     )).thenReturn(RunResult(ProcessResult(106, 0, 'v0.1.2-3-1234abcd', ''), <String>['git', 'describe']));
+    when(processUtils.runSync(
+      <String>['git', 'tag', '--contains', 'HEAD'],
+      workingDirectory: anyNamed('workingDirectory'),
+      environment: anyNamed('environment'),
+    )).thenReturn(
+      RunResult(ProcessResult(110, 0, '', ''),
+      <String>['git', 'tag', '--contains', 'HEAD'],
+    ));
 
     GitTagVersion.determine(processUtils, workingDirectory: '.');
 
@@ -474,7 +514,7 @@ void main() {
       environment: anyNamed('environment'),
     ));
     verify(processUtils.runSync(
-      <String>['git', 'describe', '--match', '*.*.*', '--first-parent', '--long', '--tags'],
+      <String>['git', 'describe', '--match', '*.*.*-*.*.pre', '--first-parent', '--long', '--tags'],
       workingDirectory: anyNamed('workingDirectory'),
       environment: anyNamed('environment'),
     )).called(1);
@@ -493,10 +533,18 @@ void main() {
       environment: anyNamed('environment'),
     )).thenReturn(RunResult(ProcessResult(106, 0, '', ''), <String>['git', 'fetch']));
     when(processUtils.runSync(
-      <String>['git', 'describe', '--match', '*.*.*', '--first-parent', '--long', '--tags'],
+      <String>['git', 'describe', '--match', '*.*.*-*.*.pre', '--first-parent', '--long', '--tags'],
       workingDirectory: anyNamed('workingDirectory'),
       environment: anyNamed('environment'),
     )).thenReturn(RunResult(ProcessResult(107, 0, 'v0.1.2-3-1234abcd', ''), <String>['git', 'describe']));
+    when(processUtils.runSync(
+      <String>['git', 'tag', '--contains', 'HEAD'],
+      workingDirectory: anyNamed('workingDirectory'),
+      environment: anyNamed('environment'),
+    )).thenReturn(
+      RunResult(ProcessResult(108, 0, '', ''),
+      <String>['git', 'tag', '--contains', 'HEAD'],
+    ));
 
     GitTagVersion.determine(processUtils, workingDirectory: '.', fetchTags: true);
 
@@ -511,7 +559,7 @@ void main() {
       environment: anyNamed('environment'),
     ));
     verify(processUtils.runSync(
-      <String>['git', 'describe', '--match', '*.*.*', '--first-parent', '--long', '--tags'],
+      <String>['git', 'describe', '--match', '*.*.*-*.*.pre', '--first-parent', '--long', '--tags'],
       workingDirectory: anyNamed('workingDirectory'),
       environment: anyNamed('environment'),
     )).called(1);
@@ -530,10 +578,18 @@ void main() {
       environment: anyNamed('environment'),
     )).thenReturn(RunResult(ProcessResult(109, 0, '', ''), <String>['git', 'fetch']));
     when(processUtils.runSync(
-      <String>['git', 'describe', '--match', '*.*.*', '--first-parent', '--long', '--tags'],
+      <String>['git', 'tag', '--contains', 'HEAD'],
+      workingDirectory: anyNamed('workingDirectory'),
+      environment: anyNamed('environment'),
+    )).thenReturn(
+      RunResult(ProcessResult(110, 0, '', ''),
+      <String>['git', 'tag', '--contains', 'HEAD'],
+    ));
+    when(processUtils.runSync(
+      <String>['git', 'describe', '--match', '*.*.*-*.*.pre', '--first-parent', '--long', '--tags'],
       workingDirectory: anyNamed('workingDirectory'),
       environment: anyNamed('environment'),
-    )).thenReturn(RunResult(ProcessResult(110, 0, 'v0.1.2-3-1234abcd', ''), <String>['git', 'describe']));
+    )).thenReturn(RunResult(ProcessResult(111, 0, 'v0.1.2-3-1234abcd', ''), <String>['git', 'describe']));
 
     GitTagVersion.determine(processUtils, workingDirectory: '.', fetchTags: true);
 
@@ -548,7 +604,7 @@ void main() {
       environment: anyNamed('environment'),
     )).called(1);
     verify(processUtils.runSync(
-      <String>['git', 'describe', '--match', '*.*.*', '--first-parent', '--long', '--tags'],
+      <String>['git', 'describe', '--match', '*.*.*-*.*.pre', '--first-parent', '--long', '--tags'],
       workingDirectory: anyNamed('workingDirectory'),
       environment: anyNamed('environment'),
     )).called(1);
@@ -669,10 +725,15 @@ void fakeData(
     environment: anyNamed('environment'),
   )).thenReturn(ProcessResult(105, 0, '', ''));
   when(pm.runSync(
-    <String>['git', 'describe', '--match', '*.*.*', '--first-parent', '--long', '--tags'],
+    <String>['git', 'tag', '--contains', 'HEAD'],
+    workingDirectory: anyNamed('workingDirectory'),
+    environment: anyNamed('environment'),
+  )).thenReturn(ProcessResult(106, 0, '', ''));
+  when(pm.runSync(
+    <String>['git', 'describe', '--match', '*.*.*-*.*.pre', '--first-parent', '--long', '--tags'],
     workingDirectory: anyNamed('workingDirectory'),
     environment: anyNamed('environment'),
-  )).thenReturn(ProcessResult(106, 0, 'v0.1.2-3-1234abcd', ''));
+  )).thenReturn(ProcessResult(107, 0, 'v0.1.2-3-1234abcd', ''));
 }
 
 class MockProcessManager extends Mock implements ProcessManager {}