From 9e59a68c98c44e49c216d307c66d5251978657ab Mon Sep 17 00:00:00 2001
From: Christopher Fujino <christopherfujino@gmail.com>
Date: Wed, 23 Aug 2023 11:31:06 -0700
Subject: [PATCH] [flutter_tools] Fix legacy version file not being ensured
 (#133097)

Fixes https://github.com/flutter/flutter/issues/133093

When I introduced the new, more robust version file `//flutter/bin/cache/version.json` in https://github.com/flutter/flutter/pull/124558, I changed `class FlutterVersion` into an abstract interface, implemented by `_FlutterVersionFromGit` (which is essentially the previous behavior) and `_FlutterVersionFromFile`, which merely reads the data it would have computed via git from `//flutter/bin/cache/version.json`.

While doing this, I made `_FlutterVersionFromGit.ensureVersionFile()` to be a no-op, since I assumed this would not be necessary since we already had a version file in the cache. However, this method was what was previously responsible for ensuring `//flutter/version` existed on disk. This means that if, for whatever reason, the user had `//flutter/bin/cache/flutter.version.json` present but NOT `//flutter/version`, the tool would have never created that file, and they would hit the tool crash seen in https://github.com/flutter/flutter/issues/133093.

This fixes the tool by ensuring `//flutter/version` exists regardless of if we're hydrating `FlutterVersion` from `//flutter/bin/cache/flutter.version.json` or not.
---
 packages/flutter_tools/lib/src/version.dart   | 28 +++++++++++---
 .../test/general.shard/version_test.dart      | 37 +++++++++++++++++++
 2 files changed, 60 insertions(+), 5 deletions(-)

diff --git a/packages/flutter_tools/lib/src/version.dart b/packages/flutter_tools/lib/src/version.dart
index e3b66ffd94..0debc4482a 100644
--- a/packages/flutter_tools/lib/src/version.dart
+++ b/packages/flutter_tools/lib/src/version.dart
@@ -532,7 +532,13 @@ class _FlutterVersionFromFile extends FlutterVersion {
   final String devToolsVersion;
 
   @override
-  void ensureVersionFile() {}
+  void ensureVersionFile() {
+    _ensureLegacyVersionFile(
+      fs: fs,
+      flutterRoot: flutterRoot,
+      frameworkVersion: frameworkVersion,
+    );
+  }
 }
 
 class _FlutterVersionGit extends FlutterVersion {
@@ -599,10 +605,11 @@ class _FlutterVersionGit extends FlutterVersion {
 
   @override
   void ensureVersionFile() {
-    final File legacyVersionFile = fs.file(fs.path.join(flutterRoot, 'version'));
-    if (!legacyVersionFile.existsSync()) {
-      legacyVersionFile.writeAsStringSync(frameworkVersion);
-    }
+    _ensureLegacyVersionFile(
+      fs: fs,
+      flutterRoot: flutterRoot,
+      frameworkVersion: frameworkVersion,
+    );
 
     const JsonEncoder encoder = JsonEncoder.withIndent('  ');
     final File newVersionFile = FlutterVersion.getVersionFile(fs, flutterRoot);
@@ -612,6 +619,17 @@ class _FlutterVersionGit extends FlutterVersion {
   }
 }
 
+void _ensureLegacyVersionFile({
+  required FileSystem fs,
+  required String flutterRoot,
+  required String frameworkVersion,
+}) {
+  final File legacyVersionFile = fs.file(fs.path.join(flutterRoot, 'version'));
+  if (!legacyVersionFile.existsSync()) {
+    legacyVersionFile.writeAsStringSync(frameworkVersion);
+  }
+}
+
 /// Checks if the provided [version] is tracking a standard remote.
 ///
 /// A "standard remote" is one having the same url as(in order of precedence):
diff --git a/packages/flutter_tools/test/general.shard/version_test.dart b/packages/flutter_tools/test/general.shard/version_test.dart
index 0d82dedc5b..7a0b182402 100644
--- a/packages/flutter_tools/test/general.shard/version_test.dart
+++ b/packages/flutter_tools/test/general.shard/version_test.dart
@@ -554,6 +554,43 @@ void main() {
     Cache: () => cache,
   });
 
+  testUsingContext('_FlutterVersionFromFile.ensureVersionFile ensures legacy version file exists', () async {
+    final MemoryFileSystem fs = MemoryFileSystem.test();
+    final Directory flutterRoot = fs.directory('/path/to/flutter');
+    final Directory cacheDir = flutterRoot
+        .childDirectory('bin')
+        .childDirectory('cache')
+        ..createSync(recursive: true);
+    const String devToolsVersion = '0000000';
+    final File legacyVersionFile = flutterRoot.childFile('version');
+    const Map<String, Object> versionJson = <String, Object>{
+      'channel': 'stable',
+      'frameworkVersion': '1.2.3',
+      'repositoryUrl': 'https://github.com/flutter/flutter.git',
+      'frameworkRevision': '1234abcd',
+      'frameworkCommitDate': '2023-04-28 12:34:56 -0400',
+      'engineRevision': 'deadbeef',
+      'dartSdkVersion': 'deadbeef2',
+      'devToolsVersion': devToolsVersion,
+      'flutterVersion': 'foo',
+    };
+    cacheDir.childFile('flutter.version.json').writeAsStringSync(
+      jsonEncode(versionJson),
+    );
+    expect(legacyVersionFile.existsSync(), isFalse);
+    final FlutterVersion flutterVersion = FlutterVersion(
+      clock: _testClock,
+      fs: fs,
+      flutterRoot: flutterRoot.path,
+    );
+    flutterVersion.ensureVersionFile();
+    expect(legacyVersionFile.existsSync(), isTrue);
+    expect(legacyVersionFile.readAsStringSync(), '1.2.3');
+  }, overrides: <Type, Generator>{
+    ProcessManager: () => processManager,
+    Cache: () => cache,
+  });
+
   testUsingContext('FlutterVersion() falls back to git if .version.json is malformed', () async {
     final MemoryFileSystem fs = MemoryFileSystem.test();
     final Directory flutterRoot = fs.directory(fs.path.join('path', 'to', 'flutter'));