From b1b9dee1327e623f644bcc96eb74e9018985ca97 Mon Sep 17 00:00:00 2001 From: flutteractionsbot <154381524+flutteractionsbot@users.noreply.github.com> Date: Tue, 8 Apr 2025 12:26:20 -0700 Subject: [PATCH] [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. --- bin/internal/update_engine_version.ps1 | 5 ++ bin/internal/update_engine_version.sh | 5 ++ .../test/update_engine_version_test.dart | 79 ++++++++++++++++--- 3 files changed, 76 insertions(+), 13 deletions(-) diff --git a/bin/internal/update_engine_version.ps1 b/bin/internal/update_engine_version.ps1 index 8d8af9f9bb3..685eac70e1e 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 8bc0bcaf8af..2f058bdd658 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 bbb03dca5ce..88293659e30 100644 --- a/dev/tools/test/update_engine_version_test.dart +++ b/dev/tools/test/update_engine_version_test.dart @@ -52,17 +52,21 @@ void main() { } } - io.ProcessResult run(String executable, List args) { - print('Running "$executable ${args.join(" ")}"'); + io.ProcessResult run(String executable, List 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 @@ void main() { } /// Initializes a blank git repo in [testRoot.root]. - void initGitRepoWithBlankInitialCommit() { - run('git', ['init', '--initial-branch', 'master']); - run('git', ['config', '--local', 'user.email', 'test@example.com']); - run('git', ['config', '--local', 'user.name', 'Test User']); - run('git', ['add', '.']); - run('git', ['commit', '-m', 'Initial commit']); + void initGitRepoWithBlankInitialCommit({String? workingPath}) { + run('git', ['init', '--initial-branch', 'master'], workingPath: workingPath); + run('git', [ + 'config', + '--local', + 'user.email', + 'test@example.com', + ], workingPath: workingPath); + run('git', ['config', '--local', 'user.name', 'Test User'], workingPath: workingPath); + run('git', ['add', '.'], workingPath: workingPath); + run('git', [ + '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}) { - run('git', ['remote', 'add', remote, testRoot.root.path]); - run('git', ['fetch', remote]); + void setupRemote({required String remote, String? rootPath}) { + run('git', [ + 'remote', + 'add', + remote, + rootPath ?? testRoot.root.path, + ], workingPath: rootPath); + run('git', ['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', ['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';