From a84a3ab65fa872146c598fcbc8bd2579258c1bdb Mon Sep 17 00:00:00 2001 From: Divya Rani Date: Tue, 1 Jul 2025 09:54:54 +0530 Subject: [PATCH 1/2] Deprecate the ability to use an arbitrary Git installation To support deployment from Git's master branch bundled git should be the default and only supported method for Git execution in Gitaly. This commit deprecates `use_bundled_binaries` and `bin_path config options`. --- .../roles/gitaly/templates/config.toml.j2 | 1 - config.toml.example | 2 - internal/cli/gitaly/subcmd_git_test.go | 3 -- internal/git/gitcmd/command_factory_test.go | 50 +++---------------- internal/git/gitcmd/command_options_test.go | 3 +- internal/git/gitcmd/execution_environment.go | 6 ++- .../git/gitcmd/execution_environment_test.go | 33 ------------ internal/git/gittest/command.go | 2 +- internal/gitaly/config/config.go | 6 +-- internal/gitaly/config/config_test.go | 6 --- internal/testhelper/testcfg/gitaly.go | 4 -- tools/test-boot/main.go | 4 -- 12 files changed, 17 insertions(+), 103 deletions(-) diff --git a/_support/benchmarking/roles/gitaly/templates/config.toml.j2 b/_support/benchmarking/roles/gitaly/templates/config.toml.j2 index 4e50eaa1db8..f4c13edb3ad 100644 --- a/_support/benchmarking/roles/gitaly/templates/config.toml.j2 +++ b/_support/benchmarking/roles/gitaly/templates/config.toml.j2 @@ -25,7 +25,6 @@ prometheus_listen_addr = "127.0.0.1:9236" # # Git settings [git] -use_bundled_binaries = true catfile_cache_size = 10 [[storage]] diff --git a/config.toml.example b/config.toml.example index f88bcd20d0e..acd5c3f49a0 100644 --- a/config.toml.example +++ b/config.toml.example @@ -43,8 +43,6 @@ bin_dir = "/home/git/gitaly/_build/bin" # # Git settings # [git] -# # Path to Git binary. If not set, is resolved using PATH. -# bin_path = "/usr/bin/git" # # Maximum number of cached 'cat-file' processes, which constitute a pair of 'git cat-file --batch' and # # 'git cat-file --batch-check' processes. Defaults to '100'. # catfile_cache_size = 100 diff --git a/internal/cli/gitaly/subcmd_git_test.go b/internal/cli/gitaly/subcmd_git_test.go index 467a394d92f..4df3596f96c 100644 --- a/internal/cli/gitaly/subcmd_git_test.go +++ b/internal/cli/gitaly/subcmd_git_test.go @@ -2,7 +2,6 @@ package gitaly import ( "bytes" - "os" "os/exec" "strings" "testing" @@ -17,8 +16,6 @@ func TestGitalyGitCommand(t *testing.T) { ctx := testhelper.Context(t) cfg := testcfg.Build(t) - cfg.Git.BinPath = os.Getenv("GITALY_TESTING_GIT_BINARY") - testcfg.BuildGitaly(t, cfg) _, repo := gittest.CreateRepository(t, ctx, cfg, gittest.CreateRepositoryConfig{ diff --git a/internal/git/gitcmd/command_factory_test.go b/internal/git/gitcmd/command_factory_test.go index c0124c7e899..12adc447215 100644 --- a/internal/git/gitcmd/command_factory_test.go +++ b/internal/git/gitcmd/command_factory_test.go @@ -7,7 +7,6 @@ import ( "net/http" "net/http/httptest" "os" - "os/exec" "path/filepath" "strings" "testing" @@ -429,27 +428,10 @@ func TestCommandFactory_ExecutionEnvironment(t *testing.T) { // We need to compare the execution environments manually because they also have // some private variables which we cannot easily check here. actualExecEnv := gitCmdFactory.GetExecutionEnvironment(ctx) - require.Equal(t, expectedExecEnv.BinaryPath, actualExecEnv.BinaryPath) + require.Equal(t, expectedExecEnv.EnvironmentVariables, actualExecEnv.EnvironmentVariables) } - t.Run("set in config", func(t *testing.T) { - assertExecEnv(t, config.Cfg{ - Git: config.Git{ - BinPath: "/path/to/myGit", - }, - }, gitcmd.ExecutionEnvironment{ - BinaryPath: "/path/to/myGit", - EnvironmentVariables: []string{ - "LANG=en_US.UTF-8", - "GIT_TERMINAL_PROMPT=0", - "GIT_CONFIG_GLOBAL=/dev/null", - "GIT_CONFIG_SYSTEM=/dev/null", - "XDG_CONFIG_HOME=/dev/null", - }, - }) - }) - t.Run("set using GITALY_TESTING_GIT_BINARY", func(t *testing.T) { t.Setenv("GITALY_TESTING_GIT_BINARY", "/path/to/env_git") @@ -496,27 +478,11 @@ func TestCommandFactory_ExecutionEnvironment(t *testing.T) { } }) - t.Run("not set, get from system", func(t *testing.T) { - resolvedPath, err := exec.LookPath("git") - require.NoError(t, err) - - assertExecEnv(t, config.Cfg{}, gitcmd.ExecutionEnvironment{ - BinaryPath: resolvedPath, - EnvironmentVariables: []string{ - "LANG=en_US.UTF-8", - "GIT_TERMINAL_PROMPT=0", - "GIT_CONFIG_GLOBAL=/dev/null", - "GIT_CONFIG_SYSTEM=/dev/null", - "XDG_CONFIG_HOME=/dev/null", - }, - }) - }) - t.Run("doesn't exist in the system", func(t *testing.T) { testhelper.Unsetenv(t, "PATH") _, _, err := gitcmd.NewExecCommandFactory(config.Cfg{}, testhelper.SharedLogger(t), gitcmd.WithSkipHooks()) - require.EqualError(t, err, "setting up Git execution environment: could not set up any Git execution environments") + require.ErrorContains(t, err, "setting up Git execution environment: constructing Git environment: checking bundled Git binary") }) } @@ -524,9 +490,8 @@ func TestExecCommandFactoryHooksPath(t *testing.T) { ctx := testhelper.Context(t) t.Run("temporary hooks", func(t *testing.T) { - cfg := config.Cfg{ - BinDir: testhelper.TempDir(t), - } + cfg := testcfg.Build(t) + cfg.BinDir = testhelper.TempDir(t) t.Run("no overrides", func(t *testing.T) { gitCmdFactory := gittest.NewCommandFactory(t, cfg) @@ -550,9 +515,10 @@ func TestExecCommandFactoryHooksPath(t *testing.T) { }) t.Run("hooks path", func(t *testing.T) { - gitCmdFactory := gittest.NewCommandFactory(t, config.Cfg{ - BinDir: testhelper.TempDir(t), - }, gitcmd.WithHooksPath("/hooks/path")) + cfg := testcfg.Build(t) + cfg.BinDir = testhelper.TempDir(t) + + gitCmdFactory := gittest.NewCommandFactory(t, cfg, gitcmd.WithHooksPath("/hooks/path")) // The environment variable shouldn't override an explicitly set hooks path. require.Equal(t, "/hooks/path", gitCmdFactory.HooksPath(ctx)) diff --git a/internal/git/gitcmd/command_options_test.go b/internal/git/gitcmd/command_options_test.go index a5aef34ed65..2387a7221a4 100644 --- a/internal/git/gitcmd/command_options_test.go +++ b/internal/git/gitcmd/command_options_test.go @@ -388,7 +388,8 @@ func TestWithConfigEnv(t *testing.T) { } func TestWithInternalFetch(t *testing.T) { - cfg := config.Cfg{BinDir: testhelper.TempDir(t)} + cfg := testcfg.Build(t) + cfg.BinDir = testhelper.TempDir(t) gitCmdFactory := newCommandFactory(t, cfg, WithSkipHooks()) ctx := testhelper.Context(t) diff --git a/internal/git/gitcmd/execution_environment.go b/internal/git/gitcmd/execution_environment.go index a0846ba27c2..73dbddae1c4 100644 --- a/internal/git/gitcmd/execution_environment.go +++ b/internal/git/gitcmd/execution_environment.go @@ -91,7 +91,7 @@ type DistributedGitEnvironmentConstructor struct{} // For testing purposes, this function overrides the configured Git binary path if the // `GITALY_TESTING_GIT_BINARY` environment variable is set. func (c DistributedGitEnvironmentConstructor) Construct(cfg config.Cfg) (ExecutionEnvironment, error) { - binaryPath := cfg.Git.BinPath + var binaryPath string var environmentVariables []string if override := os.Getenv("GITALY_TESTING_GIT_BINARY"); binaryPath == "" && override != "" { binaryPath = override @@ -138,7 +138,9 @@ type BundledGitEnvironmentConstructor struct { // environment variable to point to that directory such that Git is able to locate its auxiliary // binaries. func (c BundledGitEnvironmentConstructor) Construct(cfg config.Cfg) (_ ExecutionEnvironment, returnedErr error) { - if !cfg.Git.UseBundledBinaries { + // During testing, if GITALY_TESTING_GIT_BINARY is set, we want the DistributedGitEnvironmentConstructor + // to handle it instead of trying to use bundled binaries. + if os.Getenv("GITALY_TESTING_GIT_BINARY") != "" { return ExecutionEnvironment{}, ErrNotConfigured } diff --git a/internal/git/gitcmd/execution_environment_test.go b/internal/git/gitcmd/execution_environment_test.go index caeb2424817..f6a6d0ba65f 100644 --- a/internal/git/gitcmd/execution_environment_test.go +++ b/internal/git/gitcmd/execution_environment_test.go @@ -24,19 +24,6 @@ func TestDistributedGitEnvironmentConstructor(t *testing.T) { require.Equal(t, gitcmd.ErrNotConfigured, err) }) - t.Run("configuration with Git binary path succeeds", func(t *testing.T) { - execEnv, err := constructor.Construct(config.Cfg{ - Git: config.Git{ - BinPath: "/foo/bar", - }, - }) - require.NoError(t, err) - defer func() { require.NoError(t, execEnv.Cleanup()) }() - - require.Equal(t, "/foo/bar", execEnv.BinaryPath) - require.Equal(t, []string(nil), execEnv.EnvironmentVariables) - }) - t.Run("empty configuration with environment override", func(t *testing.T) { t.Setenv("GITALY_TESTING_GIT_BINARY", "/foo/bar") @@ -49,21 +36,6 @@ func TestDistributedGitEnvironmentConstructor(t *testing.T) { "NO_SET_GIT_TEMPLATE_DIR=YesPlease", }, execEnv.EnvironmentVariables) }) - - t.Run("configuration overrides environment variable", func(t *testing.T) { - t.Setenv("GITALY_TESTING_GIT_BINARY", "envvar") - - execEnv, err := constructor.Construct(config.Cfg{ - Git: config.Git{ - BinPath: "config", - }, - }) - require.NoError(t, err) - defer func() { require.NoError(t, execEnv.Cleanup()) }() - - require.Equal(t, "config", execEnv.BinaryPath) - require.Equal(t, []string(nil), execEnv.EnvironmentVariables) - }) } func TestBundledGitEnvironmentConstructor(t *testing.T) { @@ -71,11 +43,6 @@ func TestBundledGitEnvironmentConstructor(t *testing.T) { constructor := gitcmd.BundledGitConstructors[0] - t.Run("disabled bundled Git fails", func(t *testing.T) { - _, err := constructor.Construct(config.Cfg{}) - require.Equal(t, gitcmd.ErrNotConfigured, err) - }) - t.Run("complete binary directory succeeds", func(t *testing.T) { cfg := testcfg.Build(t) execEnv, err := constructor.Construct(cfg) diff --git a/internal/git/gittest/command.go b/internal/git/gittest/command.go index ab964a84a16..717bc544fef 100644 --- a/internal/git/gittest/command.go +++ b/internal/git/gittest/command.go @@ -76,7 +76,7 @@ func handleExecErr(tb testing.TB, cfg config.Cfg, execCfg ExecConfig, args []str } stderr = exitErr.Stderr } - tb.Log(cfg.Git.BinPath, args) + tb.Log(args) if len(stderr) > 0 { tb.Logf("%s\n", stderr) } diff --git a/internal/gitaly/config/config.go b/internal/gitaly/config/config.go index bb8eeba1686..a3393715697 100644 --- a/internal/gitaly/config/config.go +++ b/internal/gitaly/config/config.go @@ -372,10 +372,8 @@ func (ss HTTPSettings) Validate() error { // Git contains the settings for the Git executable type Git struct { - UseBundledBinaries bool `json:"use_bundled_binaries" toml:"use_bundled_binaries,omitempty"` - BinPath string `json:"bin_path" toml:"bin_path,omitempty"` - CatfileCacheSize int `json:"catfile_cache_size" toml:"catfile_cache_size,omitempty"` - Config []GitConfig `json:"config" toml:"config,omitempty"` + CatfileCacheSize int `json:"catfile_cache_size" toml:"catfile_cache_size,omitempty"` + Config []GitConfig `json:"config" toml:"config,omitempty"` // SigningKey is the private key used for signing commits created by Gitaly SigningKey string `json:"signing_key" toml:"signing_key,omitempty"` // RotatedSigningKeys are the private keys that have used for commit signing before. diff --git a/internal/gitaly/config/config_test.go b/internal/gitaly/config/config_test.go index b441dd6dcb9..7b9ed97a988 100644 --- a/internal/gitaly/config/config_test.go +++ b/internal/gitaly/config/config_test.go @@ -541,13 +541,9 @@ func TestLoadConfigCommand(t *testing.T) { return setupData{ cfg: Cfg{ ConfigCommand: cmd, - Git: Git{ - BinPath: "foo/bar", - }, }, expectedCfg: modifyDefaultConfig(func(cfg *Cfg) { cfg.ConfigCommand = cmd - cfg.Git.BinPath = "foo/bar" cfg.Git.SigningKey = "signing_key" cfg.Git.RotatedSigningKeys = []string{"rotated_key_1", "rotated_key_2"} cfg.Git.CommitterEmail = "noreply@gitlab.com" @@ -809,7 +805,6 @@ func TestStoragePath(t *testing.T) { func TestLoadGit(t *testing.T) { tmpFile := strings.NewReader(`[git] -bin_path = "/my/git/path" catfile_cache_size = 50 [[git.config]] @@ -825,7 +820,6 @@ value = "second-value" require.NoError(t, err) require.Equal(t, Git{ - BinPath: "/my/git/path", CatfileCacheSize: 50, Config: []GitConfig{ {Key: "first.key", Value: "first-value"}, diff --git a/internal/testhelper/testcfg/gitaly.go b/internal/testhelper/testcfg/gitaly.go index 992ed51a7af..0d235356b44 100644 --- a/internal/testhelper/testcfg/gitaly.go +++ b/internal/testhelper/testcfg/gitaly.go @@ -185,10 +185,6 @@ func (gc *GitalyCfgBuilder) Build(tb testing.TB) config.Cfg { cfg.Raft.SnapshotDir = testhelper.TempDir(tb) } - // We force the use of bundled (embedded) binaries unless we're specifically executing tests - // against a custom version of Git. - cfg.Git.UseBundledBinaries = os.Getenv("GITALY_TESTING_GIT_BINARY") == "" - require.NoError(tb, cfg.Sanitize()) require.NoError(tb, cfg.ValidateV2()) diff --git a/tools/test-boot/main.go b/tools/test-boot/main.go index 73a6a418bf5..6a14e9596aa 100644 --- a/tools/test-boot/main.go +++ b/tools/test-boot/main.go @@ -35,10 +35,6 @@ bin_dir = "{{.BinDir}}" name = "default" path = "{{.Dir}}" -[git] -use_bundled_binaries = {{.UseBundledGit}} -bin_path = "{{.GitPath}}" - [gitlab] url = 'http://gitlab_url' secret = "{{.GitlabSecret}}" -- GitLab From cf5cd0ee34184656801571e92f8bf78ee05eae30 Mon Sep 17 00:00:00 2001 From: Divya Rani Date: Tue, 8 Jul 2025 12:51:42 +0530 Subject: [PATCH 2/2] Update git-execution-environment doc Since external Git distributions will no longer be supported, this commit updates the documentation to reflect that change. --- doc/configuration/README.md | 1 - doc/git-execution-environments.md | 83 ++----------------------------- 2 files changed, 5 insertions(+), 79 deletions(-) diff --git a/doc/configuration/README.md b/doc/configuration/README.md index f55dc843b14..db3884ccb1b 100644 --- a/doc/configuration/README.md +++ b/doc/configuration/README.md @@ -101,7 +101,6 @@ The following values can be set in the `[git]` section of the configuration file | Name | Type | Required | Notes | |:---------------------|:--------|:---------|:----------------------------------------------------------------------| -| `bin_path` | string | no | Path to Git binary. If not set, will be resolved using PATH. | | `catfile_cache_size` | integer | no | Maximum number of cached cat-file processes (see below). Default 100. | #### cat-file cache diff --git a/doc/git-execution-environments.md b/doc/git-execution-environments.md index e7cadf4e8f7..7619c595f99 100644 --- a/doc/git-execution-environments.md +++ b/doc/git-execution-environments.md @@ -1,24 +1,10 @@ -# Methods for Gitaly to access Git +# Bundled Git binaries Because Git is at the heart of almost all interfaces provided by Gitaly, Gitaly -must have access to Git. Gitaly supports three ways of accessing Git: - -- Bundled Git (recommended): Gitaly accesses bundled Git binaries. Unlike the - other two ways of accessing Git, this is not a complete Git installation but - only contains a subset of binaries that are required at runtime. Bundled Git - binaries are directly embedded into the Gitaly binary during compilation. -- External Git distribution (not recommended): Gitaly accesses Git provided by - the operating system or manually installed by the administrator. -- Gitaly Git distribution (not recommended): Gitaly accesses it's own version of - Git that may carry custom patches which are recommended, but not required. - These patches may fix known bugs or fix performance issues. - -The bundled Git execution environment is the recommended way to set up Gitaly. - -## Bundled Git (recommended) - -Instead of using a full Git distribution, you can use a bundled Git execution -environment. +must have access to Git. To ensure Gitaly always has access to Git, +Gitaly accesses bundled Git binaries. These binaries are not a complete +Git installation but only contains a subset of binaries that are required at runtime. +Bundled Git binaries are directly embedded into the Gitaly binary during compilation. Bundled Git binaries are compiled whenever Gitaly is compiled. There's no need to explicitly build and install them. If you're running a test without @@ -56,62 +42,3 @@ To fix this, Gitaly has to bootstrap a Git execution environment on startup by: To tell Git where to find those symlinks, we run all Git commands with the `GIT_EXEC_PATH` environment variable that points into that temporary execution environment. - -Gitaly can be configured to use bundled binaries by setting the following keys: - -```toml -[git] -use_bundled_binaries = true -``` - -## External Git Distribution - -You can run Gitaly with an external Git distribution that is not provided by -Gitaly itself. For example: - -- Git distributions provided by the operating system's own package repositories. -- A custom version of Git installed by the system administrator. - -To configure Gitaly to use an external Git distribution, point the Git binary -path in its configuration file to the Git executable: - -```toml -[git] -bin_path = "/usr/bin/git" -``` - -If the binary path is not configured and Gitaly is not configured to use bundled -binaries, Gitaly tries to resolve a Git executable automatically using the -`PATH` variable. Relying on this behavior is not recommended and results in a -warning on start up. - -External Git distributions must meet a minimum required version (for example, -for [GitLab 17.8.0](https://docs.gitlab.com/update/versions/gitlab_17_changes/#1780)) which can change -when upgrading Gitaly. Gitaly refuses to boot if the external Git distribution -does not fulfill this version requirement. - -## Gitaly Git Distribution - -Gitaly provides its own version of Git that may contain a custom set of patches. -These patches fix known issues we experience with Gitaly and may fix performance -issues we have observed. - -While we backport patches that have been accepted in the upstream Git project, -we do not plan to apply patches that will not eventually end up in Git itself. - -To use Gitaly's Git distribution, run `make install-git`. This Make target -automatically fetches, builds, and installs Git into a specific directory that -is configurable with the `GIT_PREFIX` environment variable. The version of Git -is configurable using the `GIT_VERSION` environment variable. For example, to -install the Git version 2.37.3 into `/usr/local`: - -1. Run the command `make GIT_VERSION=2.37.3 GIT_PREFIX=/usr/local install-git` - to build Git v2.37.3 and install it in `/usr/local`. -1. Configure Gitaly to use the Git distribution: - -```toml -# Git settings -[git] -use_bundled_binaries = false -bin_path = "/usr/local/bin/git" -``` -- GitLab