| Age | Commit message (Collapse) | Author |
|---|
|
Usage of the global var config.Config replaced with
the local setup of the configuration. gitaly-git2go
binary configured only where it is needed, not globally.
Part of: https://gitlab.com/gitlab-org/gitaly/-/issues/2699
|
|
When the tree before and after the cherry-pick are the same, it means
there's nothing to commit. So in this case return an `EmptyError`
instead of building the commit.
|
|
In the request to do a `CherryPick` with `gitaly-git2go` the fields for
the identity of the committer started with `Author`. This is not
correct, when a commit is cherry-picked the original author is kept.
Only the committer identity and date are overwritten with who/when the
commit is picked.
|
|
In order to remove dependency on the config.Config we replace
usage of the config.Config.Git.BinPath with a simple "git" as
it is would be resolved to a proper one by the ConfigureGit call
that is used in TestMain.
Part of: https://gitlab.com/gitlab-org/gitaly/-/issues/2699
|
|
This commit converts commands in our `internal/git2go` wrappers to
return properly typed `git.ObjectID`s instead of plain strings.
|
|
Both the `ReadObject()` and `WriteBlob()` functions currently accept,
respectively return, an untyped string. Convert these functions to use an
ObjectID to better convey its meaning.
|
|
It's currently not possible to use our git DSL in the testhelper package
because of an import cycle between the testhelper and git package. To
fix this import cycle, we thus move test-related git helpers into the
gittest package.
This commit moves the repository helpers. As we're already touching all
sites which use these helpers anyway, it also aligns functions to have
more consistent naming.
|
|
It's currently not possible to use our git DSL in the testhelper package
because of an import cycle between the testhelper and git package. To
fix this import cycle, we thus move test-related git helpers into the
gittest package.
This commit moves the tree helpers.
|
|
Removal of the buildCommand function dependency on the
global config.Config variable.
Part of: https://gitlab.com/gitlab-org/gitaly/-/issues/2699
|
|
UserCommitFiles' Go port doesn't pass all index errors originating from
libgit2 as index error in the response. This has caused some upstream
tests to fail. To match the behavior of the original Ruby implementation,
let's pass up any index class errors from libgit2 as special errors.
|
|
RequireTree used hardcoded git execution name. This commit
changes it, so it now accepts git bin path from the caller.
Part of: https://gitlab.com/gitlab-org/gitaly/-/issues/2699
|
|
This implements the `UserCherryPick` command in Go. It uses
`gitaly-git2go` to call `func (*Repository) Cherrypick`.
This feature is behind a feature flag and is disabled by default. The
Ruby implementation is kept (for now) and is the default.
The feature flag roll-out issue:
https://gitlab.com/gitlab-org/gitaly/-/issues/3281
Closes: https://gitlab.com/gitlab-org/gitaly/-/issues/3071
|
|
Prior to this change, the `UserRevert` command was the only command to
be handling a conflict error. In one of the subsequent commits
`UserCherryPick` will also be rewritten into Golang, and it also has to
handle a conflict error.
This change renames `git2go.RevertConflictError` to
`git2go.HasConflictsError` so the name is more generic and it makes
sense to be reused by `UserCherryPick`.
|
|
Running a gitaly-git2go command is pretty generic in case gob-encoded
input and output are used. So to make it easier to reuse code, this
change extracts that logic into a separate method.
This method:
- takes in any type of request
- encodes the request it into gob
- runs the requested gitaly-git2go subcommand with the encoded input
- decodes the response
- returns error or commit ID as string
|
|
ErrInvalidArgument is used outside merge so it makes more sense to move
the definition to command.go where there are a bunch more functions and
such that are shared across this package.
|
|
Current implementation of the localrepo.Repo creates
*git.ExecCommandFactory internally and use it for
running git commands. This dependency needs to be
provided from the outside instead. The git.CommandFactory
parameter added to the constructor func and all its usages
fixed thor out the code.
|
|
The LocalRepository implementation has grown quite a lot up to a point
where it's warrented to have its own package. So let's move it into
`internal/git/localrepo`, similar to the already existing
`internal/git/remoterepo` package.
|
|
While we ported the UserMergeToRef RPC to Go, a new flag
"allow_conflicts" was added to its request. What it does is that instead
of raising an error when there's any conflict, the RPC will now commit
the conflicting file together with its conflict markers.
Given that both efforts kind of crossed, the parameter isn't supported
by the Go implementation nowadays. This commit lays the groundwork to
support it.
|
|
The `LocalRepository` abstraction indirectly depends on the global
Gitaly configuration as it uses `NewCommand()` to run commands, which in
turn uses the global configuration to resolve repository locations.
As a preparatory step, this commit makes the configuration available to
the `LocalRepository` structure by passing it as an argument of
`NewRepository()`. Note that the configuration is currently not yet
used.
|
|
The golangci-lint project has a set of default excludes for linters
which are applied by default. This means that we cannot choose ourselves
which linting messages we want to include, as some of them cannot be
controlled by us.
One noteworthy linting check which is excluded right now checks whether
comments for types and functions are prefixed with their respective
name. This is something which we try to enforce in Gitaly, too, but
naturally there's cases where it slips through review.
So let's disable the default excludes such that we can enable this
linting check. As it turns out, there's quite a lot of cases where we
failed to adhere to this style, which this commit also fixes. One thing
which we don't do though is to have per-package comments, so let's
actively ignore it for now.
|
|
|
|
When doing merges, git uses a three-way merge between three different
states of the file: ours, theirs and the merge base which is the first
common ancestor of ours and theirs. But when both commits have performed
criss-cross merges with each other, then the merge base is not unique.
For example in the following graph:
o---o <- ours
/ \ /
base o X
\ / \
o---o <- theirs
When merging ours with theirs, both ours^1 and theirs^1 are immediately
reachable from both commits via ours^2 and theirs^2. As such, both could
serve as a potential merge base for this merge.
What git does in this case is to compute a virtual merge base by merging
ours^1 with theirs^1 first, followed by another merge of this virtual
merge base with ours and theirs. In fact, this is even done recursively
such that if there are repeated criss-cross merges, then git will go all
the way back to the first criss-cross merge and compute all the virtual
merge bases from thereon.
While this is all nice and helps to generate better merges, the result
is that a seemingly simple merge may in fact perform dozens of merges
behind your back. For simple trees this wouldn't hurt much, but as soon
as the tree contains lots of files for which a merge is expensive, then
doing that merge dozens of times quickly starts to hurt as the merge may
now require minutes to compute.
We could solve this issue by restricting recursive merges altogether,
but this would negatively impact the quality of our merges and may
result in more conflicts being generated. Instead, we're now restricting
the number of virtual merge bases being computed. This puts an upper
limit on how many merges are performed and should help avoid abuse via
specially crafted commit histories. If that limit is hit, we simply
pick the first merge base and unravel the remaining virtual merge bases.
This commit picks a kind-of arbitrary limit of 20 computed merge bases.
While picking a lower limit would help performance if we hit such cases,
the downside is that picking a merge base at random may lead to more
conflicts being generated. See e.g. the implemented test case which
nicely demonstrates that the merge base limit does cause us to fail some
merges now.
|
|
Function 'command.GitPath()' depends on the global 'config.Config' variable
and uses internal call to change the state of it in case it is not
yet initialized properly. To break this dependency we remove the function
and replaces it with direct access to the configured value.
It could be set from the config.toml file or from env using GITALY_TESTING_GIT_BINARY.
If none used the value will be resolved from the system.
In the tests the value is set on the configuration stage and point to the temporary
directory.
Part of: https://gitlab.com/gitlab-org/gitaly/-/issues/2699
|
|
Implements apply subcommand for gitaly-git2go that allows for
applying patches from diffs. Similar to 'git am', three-way merge
is attempted as a fallback if the patch does not apply cleanly.
|
|
`internal/git` currently exposes a big `Repository` interface which
allows for interacting with git repositories in various ways. The
package also hides the concrete implementations `localRepository`
and `remoteRepository` and instead only ever returns the `Repository`
interface. This has a few problems:
1. The `Repository` interface grows massive as it has to host every
method defined on any of the implementations.
2. Each of the implementations need to contain all of the methods
in order implement the interface. To support this, there is an
`UnimplementedRepository` implementation which simply contains
all of the methods and returns errors.
3. The caller can't be sure if the implementation they use
actually supports the method needed. Only way is to check the
source code or to get a runtime error. In fact, there is only
one common method, `ResolveRefish`, between the implementations.
This is not obvious from just looking at the interface.
This commit trims down the `Repository` interface to only include
shared methods between the two implementations and exposes the
concrete implementations. This benefits us in few ways:
1. With the concrete implementations exposed, it's clearer whether
an operation leads to remote calls or not. In cases where it doesn't
matter, the caller can use the shared interface. In cases where it
matters, the type checker can enforce this.
2. If the caller needs methods which are not implemented on all
implementations, they should define their own interface in the Go
ethos of 'interface is defined where it is used'. This helps keep
the interfaces trimmed down to only relevant methods.
3. We don't need to keep expanding the `UnimplementedRepository` type
as the implementations are not forced to support methods they don't
need.
4. We can require any methods on the common interface to be tested
using a common test suite. This helps enforcing the implementations
behave in the same way.
This is the first commit in series of commits refactoring the package
structure to break cyclic dependencies between services, `git` and the
specific implementations.
|
|
During tests we're accumulating quite some global data, some of which is
hard to clean up separately. As we don't have any shared global test
directory in which we can put all generated test data, they're thus
all over `/tmp` and never get cleaned up. As a result, after a few runs
the temporary directory will be cluttered by thousands of files and
directories generated by our test suite.
This commit is a preparation for such a global unified test directory,
which is going to be set up by `testhelper.Configure()`. In order to
assure it'll get cleaned up, we add a (still empty) cleanup function and
adjust all callers to invoke it. This function will at a later point be
used to delete the temporary test directory.
|
|
There's been some proliferation in our test's main functions. Naturally
enough, this causes some of them to not work correctly, e.g. because
they use `defer` in the `TestMain` function, which is not getting
executed if `os.Exit` is called.
Unify existing setup functions to follow the same pattern, which is that
`TestMain()` calls out to `testMain()`.
|
|
|
|
|
|
The new "resolve" subcommand duplicates most of the Ruby functionality
using the git2go library. This includes:
- Attempts a merge commit
- Iterates over the conflicts remaining in the index
- For each conflict, applies the provided resolutions
- Fails if any conflicts are unresolved
- Creates a commit in the source repository
- Returns this commit to the caller
|
|
Adds 'commit' subcommand to gitaly-git2go to allow for building
commits without worktrees. Commit is built from a set of actions
and written in to the object database.
|
|
gitaly-git2go revert command
See merge request gitlab-org/gitaly!2625
|
|
Due to the process-gap between the conflicts service and its Git2Go
executable, it's not as straight-forward to pass around information as
it usually is. One of those areas which are currently lacking in
conflict handling is how we treat error codes. In case there's a
potentially expected error happening inside the command, it's currently
impossible to tell because all the different errors are lumped together
into a single error return code.
Address this issue by instead serializing error codes into the result
structure. Like this, we can become more informative around the specific
error code and discern different cases.
|
|
When conflicts occur, it's not necessarily the case that the conflict's
contents are valid UTF-8, as conflicting files may have different
encodings. But as we're serializing and deserializing those contents as
strings, any such encoding issues may get stripped away and cause us to
end up with conflict contents which don't really represent the actual
conflict.
Fix this by using a byte array for conflict contents instead of a
string.
|
|
When listing merge conflicts via Git2Go, we currently only pass along
information about conflict paths and contents. In order to implement
ListConflictFiles, we also require the mode of each of the sides.
Add the mode to the conflicts response to cater to this need. In order
to avoid code duplication, this commit also does some refactoring and
sets up a separate structure for conflict entries.
|
|
Adds a new revert command that wraps git.RevertCommit
|
|
In order to get a list of all conflicting files when merging two
commits, this commit implements a new command `gitaly-git2go conflicts`.
As input, it gets the repository as well as the two commits which ought
to be merged with each other. It'll then return the serialized list of
conflicts via its standard output, containing for each conflict the
three paths which are impacted as well as the conflicting hunks.
|
|
When spawning Git2Go commands, communication happens via serialized
messages between parent and its child process. This is currently done by
serializing into a string, only to print that string to standard output
immediately afterwards. This pattern is quite inefficient, as we first
need to serialize the whole string into memory.
This commit implements a new function to serialize directly into a
reader in order to avoid this overhead and adjusts the merge command to
use it.
|
|
|
|
The merge command doesn't have any unit tests expliciting its standalone
behaviour. Add tests to verify that serialization and deserialization of
merge commands and results work.
|
|
Now that we're using a JSON-serialized structure to pass data to
gitaly-git2go, we can use more typing. So let's convert from using a
time string to passing around `time.Time` instead.
|
|
In order to invoke the gitaly-git2go merge subcommand, one currently has
to pass each parameter as a separate flag. This is quite easy to get
wrong and makes refactoring of the interface much harder to accomplish.
This commit refactors the code to instead pass parameters via a
serialized JSON structure. The new `internal/git2go` package contains
all logic required to serialize or deserialize merge parameters as well
as running the actual command.
|
