config: honor GIT_CONFIG_GLOBAL and GIT_CONFIG_SYSTEM; add tests#1702
Open
mdelapenya wants to merge 4 commits intogo-git:mainfrom
Open
config: honor GIT_CONFIG_GLOBAL and GIT_CONFIG_SYSTEM; add tests#1702mdelapenya wants to merge 4 commits intogo-git:mainfrom
mdelapenya wants to merge 4 commits intogo-git:mainfrom
Conversation
…precedence coverage
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR adds support for the GIT_CONFIG_GLOBAL and GIT_CONFIG_SYSTEM environment variables to mirror git's native behavior, allowing users and tests to redirect config file locations. The changes modify the config loader to check these environment variables before falling back to default paths.
Key changes:
- Modified
Paths()function to check and honorGIT_CONFIG_GLOBALandGIT_CONFIG_SYSTEMenvironment variables - Added comprehensive unit tests verifying environment variable behavior and config precedence rules
- Tests validate the precedence order: GIT_CONFIG_GLOBAL > XDG_CONFIG_HOME > HOME/.gitconfig for global scope, and local > global > system for repository-scoped configs
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| config/config.go | Added environment variable checks in Paths() for GIT_CONFIG_GLOBAL and GIT_CONFIG_SYSTEM |
| config/config_test.go | Added three test cases verifying env var behavior and precedence rules for global and system scopes |
| repository_test.go | Added integration test verifying repository-level config precedence across system, global, and local scopes |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Manuel de la Peña <social.mdelapenya@gmail.com>
Contributor
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
config/config.go:208
- The closing brace on line 302 closes the entire switch statement, but it should only close the SystemScope case. The switch statement is missing its closing brace. This would cause a compilation error.
}
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
1. Use os.LookupEnv() instead of os.Getenv() to distinguish between "not set" and "set to empty string" 2. When GIT_CONFIG_GLOBAL is set to "", skip adding the fallback paths (matching Git's behavior) 3. Apply the same logic to GIT_CONFIG_SYSTEM for consistency This fix makes the library behave like Git does: when you explicitly set GIT_CONFIG_GLOBAL="", it means "don't read any global config", not "ignore my setting and use defaults."
* main: (97 commits) x: plumbing/worktree, Fix fuzzing issues and PR feedback The underlying oss-fuzz logic has issues when the Fuzz tests are in a different package than the logic being tests, resulting in build failures. This moves the test into the worktree package. x: plumbing/worktree, Add E2E test for linked worktrees x: plumbing/worktree, Add a new Init This is a go-git concept, which adds flexibility to the way linked worktrees work. It enables a fs to be connected to an existing metadata, which is particularly useful cross-filesystem implementations. For example, in-memory worktrees that are connected pre-existing worktree metadata on disk - or vice versa. _examples: Run linked worktree example x: plumbing/worktree, Add WithDetachedHead() option Previously the new worktrees were created detached by default, which does not align with upstream behaviour. This option is opt-in, and when not used it will default to a branch based on the worktree name. x: plumbing/worktree, Add support for listing linked worktrees x: plumbing/worktree, Add documentation examples x: plumbing/worktree, Add fuzz tests x: plumbing/worktree, Add support for opening linked worktrees build: Add support for x in PR validation x: plumbing/worktree, Add support for worktrees removal storage: filesystem/dotgit, fixing issue where absolute paths to commonDotGitFs ended up in dotGitFs x: plumbing/worktree, Add support for worktree creation build: Update go-git build: Update github.com/go-git/go-git-fixtures/v5 digest to 6900f24 storage: fix pack file permissions *: drop goproxy dependency build: Update module github.com/go-git/go-billy/v6 to v6.0.0-20251209065551-8afc3eb64e4d *: enable paralleltest linter plumbing: filter password/sensitive headers from logging ...
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Author
Author
|
@pjbgf any update on this? Cheers! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This change makes the config loader respect the environment variables GIT_CONFIG_GLOBAL and GIT_CONFIG_SYSTEM (if set) for Global and System scopes respectively. It also adds unit tests verifying the env-var behavior and precedence:
config/config_test.go.Repository.ConfigScoped(SystemScope).This mirrors git's behavior as documented in the git-config manual (ENVIRONMENT section): https://git-scm.com/docs/git-config#ENVIRONMENT
The env vars are preferred (placed before defaults) and existing fallback behavior is preserved.
Closes #1701