Unset GIT_DIR and other variables before updating (#165818)

`update_engine_version.{ps1|sh}` needs to operate on a foreign
repository.
When flutter is run in a git-hook, these environment variables will
override our git calls location and corrupt the install.

fixes: #165390

bin/internal/update_engine_version.ps1

@@ -23,6 +23,11 @@
 
 $ErrorActionPreference = "Stop"
 
+# When called from a submodule hook; these will override `git -C dir`
+$env:GIT_DIR = $null
+$env:GIT_INDEX_FILE = $null
+$env:GIT_WORK_TREE = $null
+
 $progName = Split-Path -parent $MyInvocation.MyCommand.Definition
 $flutterRoot = (Get-Item $progName).parent.parent.FullName
 

bin/internal/update_engine_version.sh

@@ -24,6 +24,11 @@
 
 set -e
 
+# When called from a submodule hook; these will override `git -C dir`
+unset GIT_DIR
+unset GIT_INDEX_FILE
+unset GIT_WORK_TREE
+
 FLUTTER_ROOT="$(dirname "$(dirname "$(dirname "${BASH_SOURCE[0]}")")")"
 
 # Generate a bin/cache directory, which won't initially exist for a fresh checkout.

dev/tools/test/update_engine_version_test.dart

@@ -52,17 +52,21 @@ void main() {
     }
   }
 
-  io.ProcessResult run(String executable, List<String> args) {
+  io.ProcessResult run(String executable, List<String> args, {String? workingPath}) {
-    print('Running "$executable ${args.join(" ")}"');
+    print('Running "$executable ${args.join(" ")}"${workingPath != null ? ' $workingPath' : ''}');
     final io.ProcessResult result = io.Process.runSync(
       executable,
       args,
       environment: environment,
-      workingDirectory: testRoot.root.absolute.path,
+      workingDirectory: workingPath ?? testRoot.root.absolute.path,
       includeParentEnvironment: false,
     );
     if (result.exitCode != 0) {
-      fail('Failed running "$executable $args" (exit code = ${result.exitCode})');
+      fail(
+        'Failed running "$executable $args" (exit code = ${result.exitCode}),'
+        '\nstdout: ${result.stdout}'
+        '\nstderr: ${result.stderr}',
+      );
     }
     printIfNotEmpty('stdout', (result.stdout as String).trim());
     printIfNotEmpty('stderr', (result.stderr as String).trim());
@@ -185,12 +189,22 @@ void main() {
   }
 
   /// Initializes a blank git repo in [testRoot.root].
-  void initGitRepoWithBlankInitialCommit() {
+  void initGitRepoWithBlankInitialCommit({String? workingPath}) {
-    run('git', <String>['init', '--initial-branch', 'master']);
+    run('git', <String>['init', '--initial-branch', 'master'], workingPath: workingPath);
-    run('git', <String>['config', '--local', 'user.email', 'test@example.com']);
+    run('git', <String>[
-    run('git', <String>['config', '--local', 'user.name', 'Test User']);
+      'config',
-    run('git', <String>['add', '.']);
+      '--local',
-    run('git', <String>['commit', '-m', 'Initial commit']);
+      'user.email',
+      'test@example.com',
+    ], workingPath: workingPath);
+    run('git', <String>['config', '--local', 'user.name', 'Test User'], workingPath: workingPath);
+    run('git', <String>['add', '.'], workingPath: workingPath);
+    run('git', <String>[
+      'commit',
+      '--allow-empty',
+      '-m',
+      'Initial commit',
+    ], workingPath: workingPath);
   }
 
   /// Creates a `bin/internal/engine.version` file in [testRoot].
@@ -207,9 +221,14 @@ void main() {
   /// Sets up and fetches a [remote] (such as `upstream` or `origin`) for [testRoot.root].
   ///
   /// The remote points at itself (`testRoot.root.path`) for ease of testing.
-  void setupRemote({required String remote}) {
+  void setupRemote({required String remote, String? rootPath}) {
-    run('git', <String>['remote', 'add', remote, testRoot.root.path]);
+    run('git', <String>[
-    run('git', <String>['fetch', remote]);
+      'remote',
+      'add',
+      remote,
+      rootPath ?? testRoot.root.path,
+    ], workingPath: rootPath);
+    run('git', <String>['fetch', remote], workingPath: rootPath);
   }
 
   /// Returns the SHA computed by `merge-base HEAD {{ref}}/master`.
@@ -222,6 +241,40 @@ void main() {
     return mergeBaseHeadOrigin.stdout as String;
   }
 
+  group('GIT_DIR', () {
+    late Directory externalGit;
+    late String externalHead;
+    setUp(() {
+      externalGit = localFs.systemTempDirectory.createTempSync('GIT_DIR_test.');
+      initGitRepoWithBlankInitialCommit(workingPath: externalGit.path);
+      setupRemote(remote: 'upstream', rootPath: externalGit.path);
+
+      externalHead =
+          (run('git', <String>['rev-parse', 'HEAD'], workingPath: externalGit.path).stdout
+                  as String)
+              .trim();
+    });
+
+    test('un-sets environment variables', () {
+      // Needs to happen before GIT_DIR is set
+      initGitRepoWithBlankInitialCommit();
+      setupRemote(remote: 'upstream');
+
+      environment['GIT_DIR'] = '${externalGit.path}/.git';
+      environment['GIT_INDEX_FILE'] = '${externalGit.path}/.git/index';
+      environment['GIT_WORK_TREE'] = externalGit.path;
+
+      runUpdateEngineVersion();
+
+      final String engineStamp = testRoot.binCacheEngineStamp.readAsStringSync().trim();
+      expect(engineStamp, isNot(equals(externalHead)));
+    });
+
+    tearDown(() {
+      externalGit.deleteSync(recursive: true);
+    });
+  });
+
   group('if FLUTTER_PREBUILT_ENGINE_VERSION is set', () {
     setUp(() {
       environment['FLUTTER_PREBUILT_ENGINE_VERSION'] = '123abc';