Be more strict about finding version number attached to a revision. (#28527)

When we package Flutter, we used to find the "current" tag (which is the version number) by starting at the revision we are building on and looking backwards in time to find the most recent tag. This causes problems on release builds when we failed to tag properly.

This PR makes the packaging script be more strict by requiring the given revision to itself have a tag, but only when we're publishing the result. When we're not publishing the result, it's more lenient, since otherwise we couldn't test packaging on non-release commits.

I also renamed ProcessRunnerException to PreparePackageException, since we were using that exception more generally than just for processes.

dev/bots/prepare_package.dart

@@ -26,8 +26,8 @@ const String baseUrl = 'https://storage.googleapis.com/flutter_infra';
 
 /// Exception class for when a process fails to run, so we can catch
 /// it and provide something more readable than a stack trace.
-class ProcessRunnerException implements Exception {
-  ProcessRunnerException(this.message, [this.result]);
+class PreparePackageException implements Exception {
+  PreparePackageException(this.message, [this.result]);
 
   final String message;
   final ProcessResult result;
@@ -158,17 +158,17 @@ class ProcessRunner {
     } on ProcessException catch (e) {
       final String message = 'Running "${commandLine.join(' ')}" in ${workingDirectory.path} '
           'failed with:\n${e.toString()}';
-      throw ProcessRunnerException(message);
+      throw PreparePackageException(message);
     } on ArgumentError catch (e) {
       final String message = 'Running "${commandLine.join(' ')}" in ${workingDirectory.path} '
           'failed with:\n${e.toString()}';
-      throw ProcessRunnerException(message);
+      throw PreparePackageException(message);
     }
 
     final int exitCode = await allComplete();
     if (exitCode != 0 && !failOk) {
       final String message = 'Running "${commandLine.join(' ')}" in ${workingDirectory.path} failed';
-      throw ProcessRunnerException(
+      throw PreparePackageException(
         message,
         ProcessResult(0, exitCode, null, 'returned $exitCode'),
       );
@@ -194,6 +194,7 @@ class ArchiveCreator {
     this.outputDir,
     this.revision,
     this.branch, {
+    this.strict = true,
     ProcessManager processManager,
     bool subprocessOutput = true,
     this.platform = const LocalPlatform(),
@@ -236,6 +237,11 @@ class ArchiveCreator {
   /// The directory to write the output file to.
   final Directory outputDir;
 
+  /// True if the creator should be strict about checking requirements or not.
+  ///
+  /// In strict mode, will insist that the [revision] be a tagged revision.
+  final bool strict;
+
   final Uri _minGitUri = Uri.parse(mingitForWindowsUrl);
   final ProcessRunner _processRunner;
 
@@ -285,10 +291,29 @@ class ArchiveCreator {
     return _outputFile;
   }
 
-  /// Returns the version number of this release, according the to tags in
-  /// the repo.
+  /// Returns the version number of this release, according the to tags in the
+  /// repo.
+  ///
+  /// This looks for the tag attached to [revision] and, if it doesn't find one,
+  /// git will give an error.
+  ///
+  /// If [strict] is true, the exact [revision] must be tagged to return the
+  /// version.  If [strict] is not true, will look backwards in time starting at
+  /// [revision] to find the most recent version tag.
   Future<String> _getVersion() async {
-    return _runGit(<String>['describe', '--tags', '--abbrev=0']);
+    if (strict) {
+      try {
+        return _runGit(<String>['describe', '--tags', '--exact-match', revision]);
+      } on PreparePackageException catch (exception) {
+        throw PreparePackageException(
+          'Git error when checking for a version tag attached to revision $revision.\n'
+          'Perhaps there is no tag at that revision?:\n'
+          '$exception'
+        );
+      }
+    } else {
+      return _runGit(<String>['describe', '--tags', '--abbrev=0', revision]);
+    }
   }
 
   /// Clone the Flutter repo and make sure that the git environment is sane
@@ -525,14 +550,14 @@ class ArchivePublisher {
     await _runGsUtil(<String>['cp', metadataGsPath, metadataFile.absolute.path]);
     final String currentMetadata = metadataFile.readAsStringSync();
     if (currentMetadata.isEmpty) {
-      throw ProcessRunnerException('Empty metadata received from server');
+      throw PreparePackageException('Empty metadata received from server');
     }
 
     Map<String, dynamic> jsonData;
     try {
       jsonData = json.decode(currentMetadata);
     } on FormatException catch (e) {
-      throw ProcessRunnerException('Unable to parse JSON metadata received from cloud: $e');
+      throw PreparePackageException('Unable to parse JSON metadata received from cloud: $e');
     }
 
     jsonData = await _addRelease(jsonData);
@@ -685,7 +710,7 @@ Future<void> main(List<String> rawArguments) async {
   }
 
   final Branch branch = fromBranchName(parsedArguments['branch']);
-  final ArchiveCreator creator = ArchiveCreator(tempDir, outputDir, revision, branch);
+  final ArchiveCreator creator = ArchiveCreator(tempDir, outputDir, revision, branch, strict: parsedArguments['publish']);
   int exitCode = 0;
   String message;
   try {
@@ -701,7 +726,7 @@ Future<void> main(List<String> rawArguments) async {
       );
       await publisher.publishArchive();
     }
-  } on ProcessRunnerException catch (e) {
+  } on PreparePackageException catch (e) {
     exitCode = e.exitCode;
     message = e.message;
   } catch (e) {

dev/bots/test/prepare_package_test.dart

@@ -25,10 +25,10 @@ void main() {
         expectAsync1((List<String> commandLine) async {
           return processRunner.runProcess(commandLine);
         })(<String>['this_executable_better_not_exist_2857632534321']),
-        throwsA(isInstanceOf<ProcessRunnerException>()));
+        throwsA(isInstanceOf<PreparePackageException>()));
     try {
       await processRunner.runProcess(<String>['this_executable_better_not_exist_2857632534321']);
-    } on ProcessRunnerException catch (e) {
+    } on PreparePackageException catch (e) {
       expect(
         e.message,
         contains('Invalid argument(s): Cannot find executable for this_executable_better_not_exist_2857632534321.'),
@@ -64,7 +64,7 @@ void main() {
             expectAsync1((List<String> commandLine) async {
               return processRunner.runProcess(commandLine);
             })(<String>['echo', 'test']),
-            throwsA(isInstanceOf<ProcessRunnerException>()));
+            throwsA(isInstanceOf<PreparePackageException>()));
       });
     });
     group('ArchiveCreator for $platformName', () {
@@ -110,7 +110,7 @@ void main() {
           'git clone -b dev https://chromium.googlesource.com/external/github.com/flutter/flutter': null,
           'git reset --hard $testRef': null,
           'git remote set-url origin https://github.com/flutter/flutter.git': null,
-          'git describe --tags --abbrev=0': <ProcessResult>[ProcessResult(0, 0, 'v1.2.3', '')],
+          'git describe --tags --exact-match $testRef': <ProcessResult>[ProcessResult(0, 0, 'v1.2.3', '')],
         };
         if (platform.isWindows) {
           calls['7za x ${path.join(tempDir.path, 'mingit.zip')}'] = null;
@@ -153,7 +153,7 @@ void main() {
           'git clone -b dev https://chromium.googlesource.com/external/github.com/flutter/flutter': null,
           'git reset --hard $testRef': null,
           'git remote set-url origin https://github.com/flutter/flutter.git': null,
-          'git describe --tags --abbrev=0': <ProcessResult>[ProcessResult(0, 0, 'v1.2.3', '')],
+          'git describe --tags --exact-match $testRef': <ProcessResult>[ProcessResult(0, 0, 'v1.2.3', '')],
         };
         if (platform.isWindows) {
           calls['7za x ${path.join(tempDir.path, 'mingit.zip')}'] = null;
@@ -201,7 +201,54 @@ void main() {
         };
         processManager.fakeResults = calls;
         expect(expectAsync0(creator.initializeRepo),
-            throwsA(isInstanceOf<ProcessRunnerException>()));
+            throwsA(isInstanceOf<PreparePackageException>()));
+      });
+
+      test('non-strict mode calls the right commands', () async {
+        final String createBase = path.join(tempDir.absolute.path, 'create_');
+        final Map<String, List<ProcessResult>> calls = <String, List<ProcessResult>>{
+          'git clone -b dev https://chromium.googlesource.com/external/github.com/flutter/flutter': null,
+          'git reset --hard $testRef': null,
+          'git remote set-url origin https://github.com/flutter/flutter.git': null,
+          'git describe --tags --abbrev=0 $testRef': <ProcessResult>[ProcessResult(0, 0, 'v1.2.3', '')],
+        };
+        if (platform.isWindows) {
+          calls['7za x ${path.join(tempDir.path, 'mingit.zip')}'] = null;
+        }
+        calls.addAll(<String, List<ProcessResult>>{
+          '$flutter doctor': null,
+          '$flutter update-packages': null,
+          '$flutter precache': null,
+          '$flutter ide-config': null,
+          '$flutter create --template=app ${createBase}app': null,
+          '$flutter create --template=package ${createBase}package': null,
+          '$flutter create --template=plugin ${createBase}plugin': null,
+          'git clean -f -X **/.packages': null,
+        });
+        final String archiveName = path.join(tempDir.absolute.path,
+            'flutter_${platformName}_v1.2.3-dev${platform.isLinux ? '.tar.xz' : '.zip'}');
+        if (platform.isWindows) {
+          calls['7za a -tzip -mx=9 $archiveName flutter'] = null;
+        } else if (platform.isMacOS) {
+          calls['zip -r -9 $archiveName flutter'] = null;
+        } else if (platform.isLinux) {
+          calls['tar cJf $archiveName flutter'] = null;
+        }
+        processManager.fakeResults = calls;
+        creator = ArchiveCreator(
+          tempDir,
+          tempDir,
+          testRef,
+          Branch.dev,
+          strict: false,
+          processManager: processManager,
+          subprocessOutput: false,
+          platform: platform,
+          httpReader: fakeHttpReader,
+        );
+        await creator.initializeRepo();
+        await creator.createArchive();
+        processManager.verifyCalls(calls.keys.toList());
       });
     });