[CP-beta]Unset GIT_DIR and other variables before updating (#165841)
This pull request is created by [automatic cherry pick workflow](https://github.com/flutter/flutter/blob/main/docs/releases/Flutter-Cherrypick-Process.md#automatically-creates-a-cherry-pick-request)
Please fill in the form below, and a flutter domain expert will evaluate this cherry pick request.
### Issue Link:
When the `GIT_DIR` environment variable is set; the flutter tool will lose track of engine artifacts.
https://github.com/flutter/flutter/issues/165390
### Changelog Description:
#165390: fix flutter tool finding engine artifacts in special cases where `GIT_DIR` is defined.
### Impact Description:
No impact on apps.
### Workaround:
Unset the environment variable `GIT_DIR` before calling flutter tooling.
### Risk:
### Test Coverage:
### Validation Steps:
Define `GIT_DIR` before this chage and see the flutter tool loses the engine artifacts. After this change it operates correctly.
diff --git a/bin/internal/update_engine_version.ps1 b/bin/internal/update_engine_version.ps1
index 8d8af9f..685eac7 100644
--- a/bin/internal/update_engine_version.ps1
+++ b/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
diff --git a/bin/internal/update_engine_version.sh b/bin/internal/update_engine_version.sh
index 8bc0bca..2f058bd 100755
--- a/bin/internal/update_engine_version.sh
+++ b/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.
diff --git a/dev/tools/test/update_engine_version_test.dart b/dev/tools/test/update_engine_version_test.dart
index bbb03dc..8829365 100644
--- a/dev/tools/test/update_engine_version_test.dart
+++ b/dev/tools/test/update_engine_version_test.dart
@@ -52,17 +52,21 @@
}
}
- io.ProcessResult run(String executable, List<String> args) {
- print('Running "$executable ${args.join(" ")}"');
+ io.ProcessResult run(String executable, List<String> args, {String? workingPath}) {
+ 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 @@
}
/// Initializes a blank git repo in [testRoot.root].
- void initGitRepoWithBlankInitialCommit() {
- run('git', <String>['init', '--initial-branch', 'master']);
- run('git', <String>['config', '--local', 'user.email', 'test@example.com']);
- run('git', <String>['config', '--local', 'user.name', 'Test User']);
- run('git', <String>['add', '.']);
- run('git', <String>['commit', '-m', 'Initial commit']);
+ void initGitRepoWithBlankInitialCommit({String? workingPath}) {
+ run('git', <String>['init', '--initial-branch', 'master'], workingPath: workingPath);
+ run('git', <String>[
+ 'config',
+ '--local',
+ '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 @@
/// 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}) {
- run('git', <String>['remote', 'add', remote, testRoot.root.path]);
- run('git', <String>['fetch', remote]);
+ void setupRemote({required String remote, String? rootPath}) {
+ run('git', <String>[
+ '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 @@
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';
