Check for tracked engine.version before overriding (#163672)

Checking out a flutter release tag (e.g. `3.29.0`) will see the checked
in engine.version file overridden. Don't do that.

See: #163308

bin/internal/update_engine_version.ps1

@@ -19,6 +19,12 @@ $ErrorActionPreference = "Stop"
 $progName = Split-Path -parent $MyInvocation.MyCommand.Definition
 $flutterRoot = (Get-Item $progName).parent.parent.FullName
 
+# On stable, beta, and release tags, the engine.version is tracked by git - do not override it.
+$trackedEngine = (git -C "$flutterRoot" ls-files bin/internal/engine.version) | Out-String
+if ($trackedEngine.length -ne 0) {
+  return
+}
+
 # Allow overriding the intended engine version via FLUTTER_PREBUILT_ENGINE_VERSION.
 #
 # This is for systems, such as Github Actions, where we know ahead of time the
@@ -35,8 +41,6 @@ if (![string]::IsNullOrEmpty($env:FLUTTER_PREBUILT_ENGINE_VERSION)) {
 
 # Test for fusion repository
 if ([string]::IsNullOrEmpty($engineVersion) -and (Test-Path "$flutterRoot\DEPS" -PathType Leaf) -and (Test-Path "$flutterRoot\engine\src\.gn" -PathType Leaf)) {
-    # Calculate the engine hash from tracked git files.
-    $branch = (git -C "$flutterRoot" rev-parse --abbrev-ref HEAD)
     if ($null -eq $Env:LUCI_CONTEXT) {
         $ErrorActionPreference = "Continue"
         git -C "$flutterRoot" remote get-url upstream *> $null
@@ -53,13 +57,11 @@ if ([string]::IsNullOrEmpty($engineVersion) -and (Test-Path "$flutterRoot\DEPS"
     }
 }
 
-if (($branch -ne "stable" -and $branch -ne "beta")) {
-    # Write the engine version out so downstream tools know what to look for.
-    $utf8NoBom = New-Object System.Text.UTF8Encoding($false)
-    [System.IO.File]::WriteAllText("$flutterRoot\bin\internal\engine.version", $engineVersion, $utf8NoBom)
+# Write the engine version out so downstream tools know what to look for.
+$utf8NoBom = New-Object System.Text.UTF8Encoding($false)
+[System.IO.File]::WriteAllText("$flutterRoot\bin\internal\engine.version", $engineVersion, $utf8NoBom)
 
-    # The realm on CI is passed in.
-    if ($Env:FLUTTER_REALM) {
-        [System.IO.File]::WriteAllText("$flutterRoot\bin\internal\engine.realm", $Env:FLUTTER_REALM, $utf8NoBom)
-    }
+# The realm on CI is passed in.
+if ($Env:FLUTTER_REALM) {
+    [System.IO.File]::WriteAllText("$flutterRoot\bin\internal\engine.realm", $Env:FLUTTER_REALM, $utf8NoBom)
 }

bin/internal/update_engine_version.sh

@@ -33,9 +33,14 @@ fi
 
 FLUTTER_ROOT="$(dirname "$(dirname "$(dirname "${BASH_SOURCE[0]}")")")"
 
+# On stable, beta, and release tags, the engine.version is tracked by git - do not override it.
+TRACKED_ENGINE="$(git -C "$FLUTTER_ROOT" ls-files bin/internal/engine.version)"
+if [[ -n "$TRACKED_ENGINE" ]]; then
+  exit
+fi
+
 # Test for fusion repository and no environment variable override.
 if [ -z "$ENGINE_VERSION" ] && [ -f "$FLUTTER_ROOT/DEPS" ] && [ -f "$FLUTTER_ROOT/engine/src/.gn" ]; then
-  BRANCH=$(git -C "$FLUTTER_ROOT" rev-parse --abbrev-ref HEAD)
   # In a fusion repository; the engine.version comes from the git hashes.
   if [ -z "${LUCI_CONTEXT}" ]; then
     set +e
@@ -54,12 +59,10 @@ if [ -z "$ENGINE_VERSION" ] && [ -f "$FLUTTER_ROOT/DEPS" ] && [ -f "$FLUTTER_ROO
   fi
 fi
 
-if [[ "$BRANCH" != "stable" && "$BRANCH" != "beta" ]]; then
-  # Write the engine version out so downstream tools know what to look for.
-  echo $ENGINE_VERSION > "$FLUTTER_ROOT/bin/internal/engine.version"
+# Write the engine version out so downstream tools know what to look for.
+echo $ENGINE_VERSION > "$FLUTTER_ROOT/bin/internal/engine.version"
 
-  # The realm on CI is passed in.
-  if [ -n "${FLUTTER_REALM}" ]; then
-    echo $FLUTTER_REALM > "$FLUTTER_ROOT/bin/internal/engine.realm"
-  fi
+# The realm on CI is passed in.
+if [ -n "${FLUTTER_REALM}" ]; then
+  echo $FLUTTER_REALM > "$FLUTTER_ROOT/bin/internal/engine.realm"
 fi

dev/tools/test/update_engine_version_test.dart

@@ -72,22 +72,6 @@ void main() {
     return run(executable, args);
   }
 
-  group('if FLUTTER_PREBUILT_ENGINE_VERSION is set', () {
-    setUp(() {
-      environment['FLUTTER_PREBUILT_ENGINE_VERSION'] = '123abc';
-    });
-
-    test('writes it to engine.version with no git interaction', () async {
-      runUpdateEngineVersion();
-
-      expect(testRoot.binInternalEngineVersion, exists);
-      expect(
-        testRoot.binInternalEngineVersion.readAsStringSync(),
-        equalsIgnoringWhitespace('123abc'),
-      );
-    });
-  });
-
   void setupRepo({required String branch}) {
     for (final File f in <File>[testRoot.deps, testRoot.engineSrcGn]) {
       f.createSync(recursive: true);
@@ -101,27 +85,75 @@ void main() {
     }
   }
 
+  const String engineVersionTrackedContents = 'already existing contents';
+  void setupTrackedEngineVersion() {
+    testRoot.binInternalEngineVersion.writeAsStringSync(engineVersionTrackedContents);
+    run('git', <String>['add', '-f', 'bin/internal/engine.version']);
+    run('git', <String>['commit', '-m', 'tracking engine.version']);
+  }
+
   void setupRemote({required String remote}) {
     run('git', <String>['remote', 'add', remote, testRoot.root.path]);
     run('git', <String>['fetch', remote]);
   }
 
+  group('if FLUTTER_PREBUILT_ENGINE_VERSION is set', () {
+    setUp(() {
+      environment['FLUTTER_PREBUILT_ENGINE_VERSION'] = '123abc';
+      setupRepo(branch: 'master');
+    });
+
+    test('writes it to engine.version with no git interaction', () async {
+      runUpdateEngineVersion();
+
+      expect(testRoot.binInternalEngineVersion, exists);
+      expect(
+        testRoot.binInternalEngineVersion.readAsStringSync(),
+        equalsIgnoringWhitespace('123abc'),
+      );
+    });
+  });
+
   test('writes nothing, even if files are set, if we are on "stable"', () async {
     setupRepo(branch: 'stable');
+    setupTrackedEngineVersion();
     setupRemote(remote: 'upstream');
 
     runUpdateEngineVersion();
 
-    expect(testRoot.binInternalEngineVersion, isNot(exists));
+    expect(testRoot.binInternalEngineVersion, exists);
+    expect(
+      testRoot.binInternalEngineVersion.readAsStringSync(),
+      equalsIgnoringWhitespace(engineVersionTrackedContents),
+    );
+  });
+
+  test('writes nothing, even if files are set, if we are on "3.29.0"', () async {
+    setupRepo(branch: '3.29.0');
+    setupTrackedEngineVersion();
+    setupRemote(remote: 'upstream');
+
+    runUpdateEngineVersion();
+
+    expect(testRoot.binInternalEngineVersion, exists);
+    expect(
+      testRoot.binInternalEngineVersion.readAsStringSync(),
+      equalsIgnoringWhitespace(engineVersionTrackedContents),
+    );
   });
 
   test('writes nothing, even if files are set, if we are on "beta"', () async {
     setupRepo(branch: 'beta');
+    setupTrackedEngineVersion();
     setupRemote(remote: 'upstream');
 
     runUpdateEngineVersion();
 
-    expect(testRoot.binInternalEngineVersion, isNot(exists));
+    expect(testRoot.binInternalEngineVersion, exists);
+    expect(
+      testRoot.binInternalEngineVersion.readAsStringSync(),
+      equalsIgnoringWhitespace(engineVersionTrackedContents),
+    );
   });
 
   group('if DEPS and engine/src/.gn are present, engine.version is derived from', () {
@@ -181,6 +213,8 @@ void main() {
       for (final File f in <File>[testRoot.deps, testRoot.engineSrcGn]) {
         f.createSync(recursive: true);
       }
+      setupRepo(branch: 'master');
+      setupRemote(remote: 'origin');
     });
 
     test('[DEPS] engine.version is blank', () async {