diff --git a/_support/benchmarking/roles/gitaly/templates/config.toml.j2 b/_support/benchmarking/roles/gitaly/templates/config.toml.j2 index 4e50eaa1db8cea3a4f15aab66ca4a2d5f55810da..f4c13edb3adb1c711b8120274df4149f371e97a2 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 f88bcd20d0e8d77771897d764487b6a22e878f0a..acd5c3f49a04bcb36f80d92aadb5b6453c7fe82f 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/doc/configuration/README.md b/doc/configuration/README.md index f55dc843b142861064fc068331218186034780c8..db3884ccb1bbb3808ba121ef6811dd68136b6c49 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 e7cadf4e8f721876f33f3ce64645738f8e226dce..7619c595f993d076cd897f7cf238688b478921cc 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" -``` diff --git a/internal/cli/gitaly/subcmd_git_test.go b/internal/cli/gitaly/subcmd_git_test.go index 467a394d92f412105ef5c6761a34196d227f56ee..4df3596f96cb079802216d7d82e2b48a99bbdc88 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 c0124c7e89979ba2f9627d662ce50573396a7963..12adc447215bee1d0260adba6216352d02839a7b 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 a5aef34ed652eb81203446de5f18c18a2b6d7870..2387a7221a4d9de7c515f2d437c3342812ea57b0 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 a0846ba27c267f4b78732e32e39b188e491cf3a3..73dbddae1c451e57949e803888113849654c22b1 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 caeb2424817656a3907641a6f9aedb1173cdeb9a..f6a6d0ba65f7c58b04aa3fceeea0166494af5f14 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 ab964a84a165bb0d41096d2c5c75a34ac036703b..717bc544fef1082af7ae1e6092325bb11052efe1 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 bb8eeba1686e768957837b21740a32c60e0144be..a339371569785dddad4d5e8b345a22b3471b4c76 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 b441dd6dcb9cb0ff36f183c896998b4ee870c184..7b9ed97a988cb9aff189f05f510b3ad8dbbea198 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 992ed51a7af3391ca1d729376821322cc82dc805..0d235356b44c80a4c3b28eb02586d3f4e39b340d 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 73a6a418bf504cf72193ef08bab5491f73303bc9..6a14e9596aafaeff52dcce38f97439fd77558b52 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}}"