| Age | Commit message (Collapse) | Author |
|---|
|
When running our tests, we want Git to use a well-known configuration.
It is thus mandatory that it never picks up the user's gitconfig files,
or otherwise tests may fail due to misconfiguration. To fix this, we
have added a hack that checks whether the currently executing binary has
a `.test` suffix: if it does we assume to be running our tests, and thus
inject a set of environment variable to ignore the gitconfig files.
We now have a better alternative with the newly introduced configuration
to ignore gitconfig files. So let's remove the hack and set this entry
as required. This also proves that the configuration works as expected.
|
|
Fix a cyclic dependency in the gittest package's commit tests so that we
can use the `setup()` function. This change prepares us to drop the test
hack where we inject `GIT_CONFIG_SYSTEM` et al. based on whether the
suffix of the current binary is `.test`.
|
|
We're deprecating use of gitconfig files which exist in the filesystem
in favor of adding `[[git.config]]` entries to Gitaly's `config.toml`.
Add a new option that allows distributions to opt-in to this new
behaviour so that they can migrate earlier if they decide to. This will
become the default with release 16.0.
Changelog: added
|
|
We're in the process of making Gitaly gitconfig-clean: if it is told to
ignore gitconfig files, neither Git nor libgit2 should ever read any
gitconfig file except for the repository-scoped gitconfig.
Add known-broken gitconfigs to our intercepted home directory. Both Git
and libgit2 will fail to parse these files and thus return an error,
which means that we can easily detect if the gitconfig is read by
accident via our testing infrastructure.
|
|
Previous to a40ff540f (Remove HOME overriding hack from tests,
2021-12-15), we were intercepting the home directory of users so that
we didn't pick up their gitconfig. This was required so that none of our
tests change behaviour based on what the user has configured. With that
commit though we had to remove this logic though because some users
require the home directory to be set up correctly to get some auxiliary
tools like asdf work correctly.
We're currently in the process of trying to get Gitaly gitconfig-clean
though, where we want to ensure that Gitaly never picks up any gitconfig
at all if told to do so. And naturally, we want to verify that this is
the case automatically via tests.
Reintroduce intercepting the home directory. In order to not break any
user's workflow, this setup is now opt-in, where our CI will enable it
by default. This allows us to put known-broken gitconfig files into the
home directory that would cause Git invocations to fail.
|
|
gitaly: Become gitconfig-clean
See merge request gitlab-org/gitaly!4578
|
|
While we make sure to override `HOME` in our Ruby specs so that Git
wouldn't pick up the gitconfig, libgit2 in fact parses the passwd file
to obtain the user's home directory. Consequentially, overriding the
environment variable is not sufficient to keep libgit2 from reading the
user's gitconfig files.
Fix this issue by overriding Rugged's gitconfig search paths.
|
|
We're using Git2go in our tests for `gitaly-git2go` to access repos as
part of the test itself. And while we do already tell Git2go to ignore
any gitconfig files as part of `gitaly-git2go`'s main function, we don't
in our tests themselves.
Fix this by configuring the gitconfig search path in our tests.
|
|
The `gitaly-git2go` tests are using a Git2go-based test helper to write
commits. That test helper is duplicating functionality we already have
in `gittest.WriteCommit()`, and furthermore it is not configuring Git2go
to not use gitconfig found in the system.
Convert tests to instead use `gittest.WriteCommit()`. Note that commit
IDs are now different because the author and committer is different now.
|
|
libgit2 is by default reading Git configuration files from their
standard locations. This is nothing we want though: the configuration
should either be explicitly set by us, or not at all.
Fix this by explicitly overriding the gitconfig locations in both Git2go
and Rugged. There is only a single Git configuration that we care about,
which is `core.fsyncObjectfiles`: it must always be set to `true` or we
may cause repository corruption. We already do enable it in Git2go via
`git.EnableFsyncGitDir(true)`, and in Rugged we inject a configuration
that contains this key. So ultimately, this change shouldn't change the
behaviour of libgit2 in any way.
|
|
The git2go executor is not injecting Git environment variables when
spawning `gitaly-git2go`. This is a strict requirement though because
we're sometimes spawning git-apply(1) from it, and Git commands must
have the environment variables of the Git execution environment set up.
Fix this by injecting the environment variables.
|
|
Our command factory is injecting a set of environment variables that
override the location of Git's configuration files when running tests so
that we are running in a well-defined environment that doesn't depend on
what the user has configured. This workaround doesn't cover the XDG
config though, which is located in `$XDG_CONFIG_HOME/git/config`, where
`$XDG_CONFIG_HOME` is typically `$HOME/.config`.
Fix this oversight by also overriding `XDG_CONFIG_HOME`, as documented
in git(1). Unfortunately, there isn't a more direct way to override
this. On the other hand it doesn't matter much: previously, we always
filtered out this environment variable in case it was set, so there
cannot be any users that depend on it being set.
|
|
While almost all parts of spawned Git commands get assembled in our
command factory, we do have a set of Git-specific environment variables
that are instead set up by our `command` package. This is a violation of
our abstractions.
Move the assembly of Git-specific environment variables into the command
factory to fix this layering violation. We now append those environment
variables to the execution environments: all sites where we directly or
indirectly spawn Git commands must inject environment variables of the
Git execution environment anyway.
This change also prepares for an upcoming change where we override
`GIT_CONFIG_GLOBAL` and `GIT_CONFIG_SYSTEM` when a Gitaly configuration
is set that asks us to do this.
|
|
into 'master'
gitpipe: Fix deadlock on context cancellation with unflushed requests
Closes #4253
See merge request gitlab-org/gitaly!4581
|
|
Makefile: Fix compatibility with GNU Make v3.81
See merge request gitlab-org/gitaly!4582
|
|
With GNU Make v3.82, the behaviour of pattern rules was changed.
Previously, pattern rules were applied in definition order, while with
that new version they're instead applied in shortest stem first order.
Quoting release notes:
The pattern-specific variables and pattern rules are now applied in the
shortest stem first order instead of the definition order (variables
and rules with the same stem length are still applied in the definition
order). This produces the usually-desired behavior where more specific
patterns are preferred. To detect this feature search for 'shortest-stem'
in the .FEATURES special variable.
With the introduction of the intermediate build directory we have
started to depend on the newer behaviour with shortest stem first, which
is breaking builds on macOS which installs GNU Make v3.81 by default.
Fix this issue by shifting around our build rules.
|
|
linguist: Implement wrapper to ignore gitconfig in Rugged
See merge request gitlab-org/gitaly!4577
|
|
We are using queues to batch writes to git-cat-file(1) in the gitpipe
package. Given that the queue can only be used by a single user at the
same time, this queue is getting locked when acquired. But by accident,
we're unlocking the queue immediately when we have constructed both the
info and object pipelines even though it is still in use. Luckily, this
programming error is mostly harmless: we finish the tracing span too
early and mark the queue as unused. But as long as no concurrent caller
tries to use the same queue this is not much of an issue.
Fix the bug by using a refcount so that we only close the queues when
they become unused.
Changelog: fixed
|
|
In commit 8773480fd (gitpipe: Propagate context cancellation in object
data pipeline, 2022-05-04), we have fixed an error that cancellation of
the context wasn't properly propagated to callers. While fixing this we
have introduced a new deadlock though: when the context is cancelled, we
may abort the pipeline early without flushing outstanding requests. This
means that the downstream reader which tries to read object data from
the git-cat-file(1) process is blocked indefinitely in case the process
is cached given that it wouldn't be killed by the context cancellation.
Fix this deadlock by flushing any outstanding requests when the context
is cancelled.
Changelog: fixed
|
|
Add field for ZenDesk ticket to template
See merge request gitlab-org/gitaly!4574
|
|
In the past we required a workaround for Linguist so that it picked up
the correct Git executables. We have since converted the code to not use
`git-linguist` anymore, but to use our own `gitaly-linguist` wrapper.
And in fact, while `git-linguist` did use Git to verify that the repo
exists, the Linguist Gem exclusively uses Rugged to access repos. As a
consequence, `gitaly-linguist` doesn't use Git at all anymore.
Remove the workaround to clean up the code. Note that we'd detect the
case where we did unexpectedly execute Git here because we inject our
own `PATH` in the testhelper code that has broken `git` wrappers in it.
So if we did still execute it we'd see that tests failed.
|
|
We're using the `git-linguist` binary to derive programming-language
statics for a repository. This binary is using Rugged to read a given
commit and analyze all blobs referenced by the root tree so that it can
return accumulated lines of counts for every language.
Unfortunately, `git-linguist` reads in gitconfig files by default with
no escape hatch, which sabotages our efforts to get gitconfig-clean in
the Gitaly codebase. We are thus forced to implement our own wrapper
script around the Linguist Gem that allows us to ignore the gitconfig in
Rugged.
Do so and implement a new `gitaly-linguist` binary that mostly mirrors
what `git-linguist` is doing. This allows us to override the Rugged
search path and point it to `/dev/null` so that it won't read any
gitconfig files at all.
Note that we do not use e.g. Gitaly's own gitconfig as computed by the
Git command factory. Ultimately, the Linguist Gem does not read any Git
configuration that would change its behaviour, so it would be overkill
to do so.
Changelog: changed
|
|
When running Ruby commands we need to make sure to expose certain
environment variables to the spawned processes so that they have a valid
Ruby execution environment. This logic is required both by Linguist and
by the Ruby server, but they duplicate the logic.
Create a new `env.AllowedRubyEnvironment()` helper function to unify
this knowledge into a single place. While at it, remove the logic to
handle the generic environment variables `HOME` and `PATH` from
Linguist: they are already injected by our `command` package.
|
|
Makefile: Fix install target using doubly-prefixed Gitaly paths
See merge request gitlab-org/gitaly!4553
|
|
The Go build chain doesn't write a GNU build ID into executables in case
the executable format is not ELF. This is most importantly the case on
macOS, which means that our `replace-buildid` tool fails to work on that
platform because it cannot find the GNU build ID.
Let's fix this by only trying to replace the build ID on Linux.
|
|
With 17f3bf27b (Makefile: Use pattern rules to build Go binaries,
2022-05-10), we have converted the `GITALY_EXECUTABLES` variable to
contain absolute paths instead of only the executables' basenames. The
`install` target was still trying to prefix these binaries though, which
ultimately led to a path which had the binary directory's prefix twice
and thus broke installation.
Fix this bug by not reapplying the prefix.
Changelog: fixed
|
|
into 'master'""
This reverts commit 933109e3e849358daf78c4618d07091b3f36068f.
|
|
'master'
praefect: Remove logic to handle maintenance-style replication events
Closes #4185
See merge request gitlab-org/gitaly!4575
|
|
Upgrade Gitaly module to 15.0
See merge request gitlab-org/gitaly!4493
|
|
Makefile: Fix protoc compile option to disable building tests
See merge request gitlab-org/gitaly!4579
|
|
This commit changes the major version in the package name from v14 to
v15
Updating go.mod & go.sum with new module name v15
Update Makefile to bump major version to v15
Update the gitaly package name in the Makefile. Also update
gitaly-git2go-v14 -> gitaly-git2go-v15. We need to keep
gitaly-git2go-v14 for a release however, for zero downtime upgrades.
This pulls directly from a sha that is v14.
Update package name from v14->v15 for auth, client, cmd, internal packages
This commit changes the package name from v14 to v15 in go and proto
files in the internal, auth, client, cmd packages.
proto: Update major package number in package name
tools: Change major version number in package name from v14 to v15
gitaly-git2go: Change the package name from v14 to v15
update module updater for v15
Update the documentation for the module updater to reflect v15
|
|
[ci skip]
|
|
The Protobuf project has seemingly renamed their CMake options to all
have a `protobuf_` prefix. While the deault options we set still work,
this broke the `-DBUILD_TESTS=OFF` switch.
Fix this and adapt accordingly to significantly reduce build times of
this project. On my machine, it decreases from about 58.9s to 22.8s.
|
|
[ci skip]
|
|
Remove the fallback logic that is handling maintenance-style replication
events. We don't generate any such events since v15.0 anymore, and we do
have a migration in place that ensures that any stale events are removed
from the replication queue. Meaning that we now shouldn't ever see any
such event in the wild anymore.
|
|
In release 15.0 we have stopped creating maintenance-style replication
events in favor of using a best-effort strategy for handling repository
maintenance. And while we still have the logic in place to dequeue these
events, they're currently ignored by Praefect altogether.
We're about to remove this fallback logic now, so let's make clear that
the replication queue really doesn't have any maintenance-style events
left by adding a migration that prunes any stale ones.
|
|
supervisor: Fix leaking logrus Goroutine on spawn failure
Closes #3927
See merge request gitlab-org/gitaly!4567
|
|
ZenDesk tickets are the primary source of truth for the Support team,
and will have the most detail on the problem and troubleshooting steps
taken so far.
Let's add an explicit field for the ticket in the support request
template.
|
|
cmd/praefect: Improve style of test for removal of replicaton jobs
Closes #3944
See merge request gitlab-org/gitaly!4569
|
|
proto: Add DeleteRefErrors
See merge request gitlab-org/gitaly!4501
|
|
operations: Fix flaky test for known-to-fail requests in UserApplyPatch
Closes #4161
See merge request gitlab-org/gitaly!4573
|
|
cgroups: Handle nil repo
See merge request gitlab-org/gitaly!4572
|
|
The test we have which verifies that removal of replication jobs works
as expected for the `praefect remove-repository` subcommand is quite
hard to read. Furthermore, many of the conditions aren't as strict as
they could theoretically be.
Refactor the test to match modern coding style and tighten it to assert
conditions that should be true. This hopefully helps to nail down why
our CI is hanging from time to time in this job.
|
|
operations: Remove SquashUsingMerge feature flag
See merge request gitlab-org/gitaly!4563
|
|
We have observed that one of our tests for `UserApplyPatch` is flaky:
from time to time we receive an unexpected EOF while streaming the patch
data to the server side. As it turns out, this flakiness is our own
fault: we send a request header that is known to cause failure, which
means that the server will return an error as soon at it realizes that.
This is effectively causing a race between the client, which is trying
to send the patch data to the server, and the server which is realizing
the error and thus aborting the RPC call.
Fix this flake by stopping on EOF so that we can test for the real error
condition via `CloseAndRecv()`.
|
|
If a command's repo is nil, handle it gracefully by adding the command
to a cgroup by using the command arguments as a key for the circular
hash.
While we're at it, fix a test that was doing the checksum incorrectly.
This didn't make a difference in the test however, because we were using
the groupID we calculated ourselves to check for the location of the
cgroup file.
Changelog: fixed
|
|
featureflag: Remove ConcurrencyQueueMaxWait feature flag
See merge request gitlab-org/gitaly!4564
|
|
commandstatshandler: Fix flaky test caused by Git version check
Closes #4239
See merge request gitlab-org/gitaly!4568
|
|
ruby: Reduce usage of Git commands
See merge request gitlab-org/gitaly!4559
|
|
In our commandstatshandler we're asserting that when executing RPCs, we
record the number of spawned commands. To do so we use an RPC that
spawns a Git command. Due to recent changes in our Git command factory
we had to adapt the test though as we are now additionaly spawning a
git-version(1) command to auto-detect the version.
This change is causing flakiness though because we cache the Git
version per binary that is executed: we don't reuse the context, and
when running with bundled Git we now thus randomly use either the old or
the new bundled Git binaries based on a feature flag that uses a random
value. So based on whether this value is the same across both RPC calls
or not we may or may not re-execute git-version(1).
Fix this by reusing the same context for both RPC calls to guarantee
that the feature flag has the same state.
|
