Skip to content

chore: redirect git clones to use local files in CI conditional tests#16652

Merged
jskeet merged 1 commit intogoogleapis:mainfrom
jskeet:git-clone-redirection
Apr 15, 2026
Merged

chore: redirect git clones to use local files in CI conditional tests#16652
jskeet merged 1 commit intogoogleapis:mainfrom
jskeet:git-clone-redirection

Conversation

@jskeet
Copy link
Copy Markdown
Contributor

@jskeet jskeet commented Apr 15, 2026

Redirects git clones for core dependencies to the local repository for both performance (avoiding unnecessary cloning) and correctness (testing HEAD of everything).

Redirects git clones for core dependencies to the local repository for
both performance (avoiding unnecessary cloning) and correctness
(testing HEAD of everything).
@jskeet jskeet requested a review from parthea April 15, 2026 11:54
@jskeet jskeet requested a review from a team as a code owner April 15, 2026 11:54
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request updates the CI script to redirect git clones for core dependencies to the local repository, improving performance and ensuring local changes are tested. The review identifies critical issues regarding git configuration overwriting and global state leakage on CI runners, recommending the use of the --add flag and a cleanup trap.

Comment on lines +44 to +45
git config --global url."${PROJECT_ROOT}".insteadOf "https://github.com/googleapis/google-cloud-python"
git config --global url."${PROJECT_ROOT}".insteadOf "https://github.com/googleapis/google-cloud-python.git"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

There are two significant issues with this implementation:

  1. Configuration Overwrite: By default, git config replaces the existing value for a key. Line 45 will overwrite the setting from line 44, meaning only URLs ending in .git will be redirected. To support both URL variants, you must use the --add flag for the second command.
  2. Global State Leakage: Using --global modifies the git configuration for the user profile on the CI runner. In environments where runners are reused (e.g., self-hosted runners), this configuration will persist and may interfere with subsequent jobs that expect to clone from the real GitHub repository.

It is recommended to use a trap to ensure the configuration is unset when the script exits, and to use --add for the second configuration entry.

Suggested change
git config --global url."${PROJECT_ROOT}".insteadOf "https://github.com/googleapis/google-cloud-python"
git config --global url."${PROJECT_ROOT}".insteadOf "https://github.com/googleapis/google-cloud-python.git"
git config --global url."${PROJECT_ROOT}".insteadOf "https://github.com/googleapis/google-cloud-python"
git config --global --add url."${PROJECT_ROOT}".insteadOf "https://github.com/googleapis/google-cloud-python.git"
trap 'git config --global --unset-all url."${PROJECT_ROOT}".insteadOf' EXIT

@jskeet jskeet merged commit 7f98c19 into googleapis:main Apr 15, 2026
30 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants