| Age | Commit message (Collapse) | Author |
|---|
|
The `backchannel.ServerFactory` in the flaky test is checking a feature
flag at the same time from the context as the Praefect metadata is being
injected into few lines below. Fix the race by checkign the feature flag
status and using the result inside the ServerFactory instead of accessing
the context.
|
|
Multiplex connections between Praefect and Gitaly by default
See merge request gitlab-org/gitaly!3360
|
|
ci: Fix nightly jobs failing because git patches do not apply
See merge request gitlab-org/gitaly!3402
|
|
golangci: Upgrade to v1.39.0
See merge request gitlab-org/gitaly!3397
|
|
When using loop variables, then one must take care to not store pointers
to these loop variables without rescoping them. The exportloopref linter
tries to find such invalid cases where the rescoping wasn't done.
Enable the linter.
|
|
It's a common pitfall to create a slice with a prespecified length and
then append to it. If the initial length hasn't been set to zero, then
this pattern causes the first `n` entries to be the zero value of its
type. The makezero linter finds such patterns.
Enable the linter and fix the single violation it surfaces.
|
|
The wastedassign linter will check whether a newly assigned variable is
used in any code path after its assignment. This seems like a useful
check to have, e.g. to not forget checking a reassigned error value.
Enable the linter and fix the single violation it surfaces.
|
|
Upgrade golangci-lint to v1.39.0 and fix all newly reported errors. This
upgrade disables the maligned linter, which has been deprecated upstream
in favor of govet.
|
|
Update golang.org/x/crypto to the latest to address CVE-2020-29652
Closes #3584
See merge request gitlab-org/gitaly!3400
|
|
testhelper: Convert `TempDir()` to use `t.Cleanup()`
See merge request gitlab-org/gitaly!3375
|
|
Signed-off-by: Takuya Noguchi <takninnovationresearch@gmail.com>
|
|
The `tempDir()` receiver function of `GitalyCfgBuilder` is only a mere
wrapper around `testhelper.TempDir()` now that it automatically cleans
up the temporary directory after the test. Inline it.
|
|
Now that `testhelper.TempDir()` automatically cleans up the temporary
directory after the test, there's no need for GitalyCfgBuilder's
`Cleanup()` function anymore. Let's remove it.
|
|
Convert `TempDir()` to use `t.Cleanup()` instead of returning a cleanup
function.
|
|
Convert `GenerateTestCerts()` to use `t.Cleanup()` instead of returning
a cleanup functions. While at it, rename the function to
`GenerateCerts()` -- it's clear it's about testing given that it's part
of the `testhelper` package anyway.
|
|
Our nightly job tests against git's next and master branches to verify
that there's not going to be any new incompatibilities with the upcoming
git version. But since we have started to apply custom patches to git,
this nightly job has started to fail because the patch does not apply on
top of these branches.
Override GIT_PATCHES for this job such that we test against unpatched
versions of git.
|
|
While our Makefile provides the ability to override options like git
patches or libgit2 build options, we currently do not treat empty values
as an override. This makes it hard to e.g. use no patches at all.
Fix the issue by testing for whether respective options have been
defined at all instead of testing whether they're non-empty.
|
|
conflicts: Fix semantic merge conflict with testhelper.CreateCommit()
See merge request gitlab-org/gitaly!3401
|
|
Propagate feature flags through gitaly-hooks
See merge request gitlab-org/gitaly!3372
|
|
In commit 3ff8154b9 (Change gittest.CreateCommit to use provided
configuration, 2021-04-21), the signature of `gittest.CreateCommit()`
was changed to get the configuration as parameter, while 8bc5d2dbe
(conflict: Fix trailing newline handling when resolving conflicts,
2021-04-21) added some new invocations of this function. Given that both
series have crossed, we now have a semantic merge conflict.
Fix the issue by adding the `config.Cfg` parameter to those new
invocation.
|
|
blob: Drop LFS pointer bitmap experiment
See merge request gitlab-org/gitaly!3385
|
|
[ci skip]
|
|
Introduce gittest.Exec|ExecStream to execute git commands
See merge request gitlab-org/gitaly!3389
|
|
conflicts: Fix various issues with ResolveConflicts
See merge request gitlab-org/gitaly!3386
|
|
Remove UserCommitFiles' Ruby implementation
See merge request gitlab-org/gitaly!3297
|
|
This commit removes the `gitaly_connection_multiplexing` feature flag
and switches to multiplexed connections between Praefect and Gitaly.
The feature flag was introduced to performance test the connection in
production prior to fully switching them. No issues have come up, so
let's remove the feature flag and switch fully to multiplexed connections.
With the feature flag, only the routed RPCs would use the multiplexed
connections. Now that they are the only connections available, the
health checks and replication job operations will also use them.
|
|
trim down lookup commit errors
See merge request gitlab-org/gitaly!3374
|
|
When resolving conflicts, we're currently always appending newlines
after each line. This is wrong though and doesn't match Ruby's
behaviour: when the pre-image of our blob didn't contain a trailing
newline, then we wouldn't append a newline either.
Fix the bug by inspecting the preimage contents of our blob and only
conditionally appending the newline.
|
|
In order to properly resolve conflicts, we'll need information about the
pre-images of the conflict entries. Refactor the parser to accept
conflict entries instead of only their paths as a preparatory step.
Note that this commit also changes the order of parameters: this is done
to more closely match other interfaces, which all have the ancestor as
first parameter. Further note that the testcase refactoring gets rid of
the differing paths given that all testcases use the same path for all
of ours, theirs and ancestor anyway.
No functional change is expected from this commit.
|
|
In order to fix another bug with trailing newlines in the post-image of
a resolved merge conflict, we'll need to make available blob contents of
the conflicting sides. In order to avoid lookup of blobs multiple times,
this commit refactors the code and extracts `readConflictEntries()`:
given an index conflict, it returns conflict entries for all three
sides.
No functional change is expected from this commit.
|
|
When resolving conflicts of an add-add conflict, then we do not have an
ancestor for this conflict. We still access the ancestor unconditionally
though in case that user-provided conflict resolutions do not solve all
existing conflicts when computing the error message.
Fix the issue by taking "our" path instead of the ancestor path if it
doesn't exist. "our" must always exist because there cannot be a
conflict without either "our" or "their" in a three-way merge.
This commit does not add a testcase because a later commit in this
series will add one that has inadvertently triggered the segfault.
|
|
When fetching a currently unknown revision from the target repository in
`ResolveConflicts()`, we first check whether the object already exists
in the source repo by calling `git rev-parse --verify $OID`. But as long
as the OID is validly formatted, this query will always return success
because git-rev-parse(1) only checks whether the revision can be
resolved to a valid OID, and if given an OID then this is always true.
Fix the issue by peeling the given OID to a commit, which causes
git-rev-parse(1) to resolve the actual object to see whether it's a
commit.
|
|
When resolving conflicts, we resolve the target branch via the
unqualified target branch name. But when the repository has both a
branch and tag with that name, then the query is ambiguous and would
return the resolved OID of the tag, not of the branch.
Fix the issue by converting branches to their fully qualified reference
names first.
|
|
The source branhc isn't relevant when fetching the target branch in
`repoWithBranchCommit()`, and as thus it isn't used. Remove it.
|
|
The `ResolveConflicts()` RPC is currently only passing around protobuf
repositories and constructs ad-hoc repo abstractions from them. Convert
this to construct the abstractions up front and pass them down.
|
|
The `sameRepo()` function is currently working on `*gitalypb.Repository`
structures, but in fact it works perfectly fine if only given two
`repository.GitRepo` interfaces.
Convert it to accept the interfaces to make it more flexible.
|
|
While ResolveConflicts is all but trivial, it's protobuf interface isn't
documented at all. Fix this by adding documentation.
|
|
While the testhelper already provides an interface to write multiple
blobs with indeterministic contents (the current time), this is not a
feasible interface for other tests which require deterministic contents.
This commit thus pulls out the logic to write the blob into a separate
function `WriteBlob()` which allows the caller to specify the blob's
contents.
|
|
The `localrepo.Repo` type already implements the `repository.GitRepo`
interface, but the `remoterepo.Repo` doesn't. This often forces the user
to pass down both the remoterepo and its `*gitalypb.Repository`, which
is annoying.
Have the `remoterepo.Repo` embed its `*gitalypb.Repository` to fix this
weakness. In contrast to the localrepo, it doesn't only embed the
interface but the complete protobuf repository: this is because for
remote repos, it's quite likely that the user needs to call additional
RPCs, so they're tied together anyway.
|
|
In contrast to `localrepo.Repo`, the `remoterepo.Repo` is working on a
plain struct instead of on a pointer. Let's align both interfaces and
use pointers in both cases.
|
|
While `remoterepo.Repository` is the remote equivalent to
`localrepo.Repo`, both types have diverging naming. Rename the former to
`remoterepo.Repo` to align them.
|
|
Stops these errors from including the commit reference multiple times
and brings them closer to the gitaly style guide.
|
|
Use promauto to register Prometheus metrics
Closes #1777
See merge request gitlab-org/gitaly!3388
|
|
Revert "commit: Raise error if skipping commit offsets fails"
See merge request gitlab-org/gitaly!3390
|
|
[ci skip]
|
|
|
|
This reverts commit 2c56d75cd763e98b70a346b1a40d5af6548961d1.
We saw this triggered elevated error rates because GitLab Rails can't
distinguish between a real failure and a `--skip` offset failure.
Relates to:
1. https://gitlab.com/gitlab-com/gl-infra/production/-/issues/4299
2. https://gitlab.com/gitlab-org/gitlab/-/issues/328461
|
|
The function gittest.CreateCommit changed to accept
configuration as an input parameter and now uses
Exec and ExecStream functions internally to run git commands.
All dependencies aligned on that change.
Part of: https://gitlab.com/gitlab-org/gitaly/-/issues/2699
|
|
The function testhelper.MustRunCommand designed for
running commands in the shell with reporting back result
of the execution. It server for too many purposes and
behaves differently in case 'git' command needs to be run.
It also uses a globally defined variable config.Config to
get path to the git binary. Most of the tests now
doesn't rely on that global variable and use one created
locally for each test.
In order to run only git related commands with locally defined
configuration the new gittest.ExecStream function is added.
As a use case example gittest.CommitBlobWithName was changed
to use gittest.Exec and gittest.ExecStream function and local
configuration.
Part of: https://gitlab.com/gitlab-org/gitaly/-/issues/2699
|
|
|
