Make `git_repository()` ignore `GIT_` env vars #24204

rpwoodbu · 2024-11-05T10:03:44Z

This ensures that this repository rule cannot be influenced by these environment variables, particularly GIT_DIR, keeping the operation limited to the directory specified by the repository_ctx.

In particular, this ensures that calling Bazel (e.g. bazel test) from within a Git hook will work reliably. Git hooks set GIT_DIR (among others), which can cause the repository rule's invocations of git to misbehave. https://git-scm.com/docs/githooks

When using Git worktrees and calling Bazel from a Git hook, it has been observed that git_repository() will not only fail, but will also set core.bare = true, breaking the user's clone.

Fixes #24199.

meteorcloudy · 2024-11-05T15:48:17Z

tools/build_defs/repo/git_worker.bzl

@@ -126,7 +126,7 @@ def _update(ctx, git_repo):

 def init(ctx, git_repo):
    cl = ["git", "init", str(git_repo.directory)]
-    st = ctx.execute(cl, environment = ctx.os.environ)
+    st = ctx.execute(_strip_git_vars(cl), environment = ctx.os.environ)


Can you remove those env vars from ctx.os.environ instead?

I think that you can only force variables to new values (e.g. the empty value), but can't remove them. Or maybe that works if they are set to None? If not, that would probably be a reasonable feature request :⁠-⁠)

@fmeum is correct. This was my first thought, but setting to the empty string upsets Git, and setting to None upsets Bazel. And yes, being able to unset a variable with None would be a great FR!

Also, I would have liked to have been able to unset a variable with --repo_env.

the problem (as indicated by the test results) is that this doesn't work on Windows... We might need to implement that FR first :/

Yeah, I saw the Windows failures. I'll dig into that and see if this can reasonably be solved within this repo rule.

FYI, I sent #24221, so it may be more efficient to wait for that.

Thanks for that! This is definitely much cleaner. However, I still need a solution for Bazel 7. So I will continue to get this working in Windows.

Perhaps this PR (once I fix Windows) could go in and be cherry-picked into the 7 branch, then a follow-up could replace the solution based on your PR? I'm not sure what the process is for point releases.

Or maybe it makes more sense to bring your PR into Bazel 7?

Yes, if there is ever a 7.5 release, we could just cherry-pick both commits.

I have rebased and changed my PR to use your new feature. I haven't been able to get my Windows build of Bazel to pass that failing test even at HEAD, so I'm going to lean on the merge checks here. 🤞

fmeum · 2024-11-06T07:41:08Z

tools/build_defs/repo/git_worker.bzl

+    return [
+        "bash",
+        "-c",
+        "unset $(git rev-parse --local-env-vars); " + " ".join(args),


Is it clear that unsetting all GIT_ vars is correct? What about something like GIT_SSH or GIT_SSL_NO_VERIFY?

$ git rev-parse --local-env-vars GIT_ALTERNATE_OBJECT_DIRECTORIES GIT_CONFIG GIT_CONFIG_PARAMETERS GIT_CONFIG_COUNT GIT_OBJECT_DIRECTORY GIT_DIR GIT_WORK_TREE GIT_IMPLICIT_WORK_TREE GIT_GRAFT_FILE GIT_INDEX_FILE GIT_NO_REPLACE_OBJECTS GIT_REPLACE_REF_BASE GIT_PREFIX GIT_INTERNAL_SUPER_PREFIX GIT_SHALLOW_FILE GIT_COMMON_DIR

So this doesn't snarf GIT_SSH or GIT_SSL_NO_VERIFY. I'll dig info the Windows issue.

This ensures that this repository rule cannot be influenced by these environment variables, particularly `GIT_DIR`, keeping the operation limited to the directory specified by the `repository_ctx`. In particular, this ensures that calling Bazel (e.g. `bazel test`) from within a Git hook will work reliably. Git hooks set `GIT_DIR` (among others), which can cause the repository rule's invocations of `git` to misbehave. https://git-scm.com/docs/githooks When using Git worktrees and calling Bazel from a Git hook, it has been observed that `git_repository()` will not only fail, but will also set `core.bare = true`, breaking the user's clone.

meteorcloudy

Thanks!

fmeum · 2024-11-11T12:44:54Z

@bazel-io fork 8.0.0

This ensures that this repository rule cannot be influenced by these environment variables, particularly `GIT_DIR`, keeping the operation limited to the directory specified by the `repository_ctx`. In particular, this ensures that calling Bazel (e.g. `bazel test`) from within a Git hook will work reliably. Git hooks set `GIT_DIR` (among others), which can cause the repository rule's invocations of `git` to misbehave. https://git-scm.com/docs/githooks When using Git worktrees and calling Bazel from a Git hook, it has been observed that `git_repository()` will not only fail, but will also set `core.bare = true`, breaking the user's clone. Fixes bazelbuild#24199. Closes bazelbuild#24204. PiperOrigin-RevId: 695642299 Change-Id: Id81e32194117cd8996408b236a6a88a20b14910b

This ensures that this repository rule cannot be influenced by these environment variables, particularly `GIT_DIR`, keeping the operation limited to the directory specified by the `repository_ctx`. In particular, this ensures that calling Bazel (e.g. `bazel test`) from within a Git hook will work reliably. Git hooks set `GIT_DIR` (among others), which can cause the repository rule's invocations of `git` to misbehave. https://git-scm.com/docs/githooks When using Git worktrees and calling Bazel from a Git hook, it has been observed that `git_repository()` will not only fail, but will also set `core.bare = true`, breaking the user's clone. Fixes #24199. Closes #24204. PiperOrigin-RevId: 695642299 Change-Id: Id81e32194117cd8996408b236a6a88a20b14910b Commit 89115d9 Co-authored-by: Richard Woodbury <[email protected]>

This ensures that this repository rule cannot be influenced by these environment variables, particularly `GIT_DIR`, keeping the operation limited to the directory specified by the `repository_ctx`. In particular, this ensures that calling Bazel (e.g. `bazel test`) from within a Git hook will work reliably. Git hooks set `GIT_DIR` (among others), which can cause the repository rule's invocations of `git` to misbehave. https://git-scm.com/docs/githooks When using Git worktrees and calling Bazel from a Git hook, it has been observed that `git_repository()` will not only fail, but will also set `core.bare = true`, breaking the user's clone. Fixes bazelbuild#24199. Closes bazelbuild#24204. PiperOrigin-RevId: 695642299 Change-Id: Id81e32194117cd8996408b236a6a88a20b14910b

github-actions bot added the awaiting-review PR is awaiting review from an assigned reviewer label Nov 5, 2024

meteorcloudy requested changes Nov 5, 2024

View reviewed changes

fmeum reviewed Nov 6, 2024

View reviewed changes

iancha1992 added the team-ExternalDeps External dependency handling, remote repositiories, WORKSPACE file. label Nov 6, 2024

rpwoodbu force-pushed the rwoodbury-git-repository branch from 5d10ac1 to c541924 Compare November 11, 2024 07:43

meteorcloudy reviewed Nov 11, 2024

View reviewed changes

meteorcloudy approved these changes Nov 11, 2024

View reviewed changes

meteorcloudy added awaiting-PR-merge PR has been approved by a reviewer and is ready to be merge internally and removed awaiting-review PR is awaiting review from an assigned reviewer labels Nov 11, 2024

bazel-io mentioned this pull request Nov 11, 2024

[8.0.0] Make git_repository() ignore GIT_ env vars #24268

Closed

copybara-service bot closed this in 89115d9 Nov 12, 2024

github-actions bot removed the awaiting-PR-merge PR has been approved by a reviewer and is ready to be merge internally label Nov 12, 2024

bazel-io mentioned this pull request Nov 12, 2024

[8.0.0] Make git_repository() ignore GIT_ env vars #24300

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make `git_repository()` ignore `GIT_` env vars #24204

Make `git_repository()` ignore `GIT_` env vars #24204

rpwoodbu commented Nov 5, 2024 •

edited

Loading

meteorcloudy Nov 5, 2024

fmeum Nov 5, 2024

rpwoodbu Nov 6, 2024

Wyverald Nov 6, 2024

rpwoodbu Nov 6, 2024

fmeum Nov 6, 2024

rpwoodbu Nov 6, 2024 •

edited

Loading

fmeum Nov 9, 2024

rpwoodbu Nov 11, 2024

fmeum Nov 6, 2024

rpwoodbu Nov 6, 2024

meteorcloudy left a comment

fmeum commented Nov 11, 2024

Make git_repository() ignore GIT_ env vars #24204

Make git_repository() ignore GIT_ env vars #24204

Conversation

rpwoodbu commented Nov 5, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rpwoodbu Nov 6, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

meteorcloudy left a comment

Choose a reason for hiding this comment

fmeum commented Nov 11, 2024

Make `git_repository()` ignore `GIT_` env vars #24204

Make `git_repository()` ignore `GIT_` env vars #24204

rpwoodbu commented Nov 5, 2024 •

edited

Loading

rpwoodbu Nov 6, 2024 •

edited

Loading