Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move git command run on a special repository to standalone methods to hide the implemention detail #28940

Draft
wants to merge 23 commits into
base: main
Choose a base branch
from

Conversation

lunny
Copy link
Member

@lunny lunny commented Jan 26, 2024

Purpose

For managed repositories, running a git command now will use a new method opposite to use git.Command. So this will not expose the running directory which making it running remotely possible.

Follow #28937

What's changed

This PR introduced 3 methods on modules/gitrepo about how to run the git.Command. The 3 methods will ask the Options' Dir is empty because the function internally know where the repository directory is. For those command which will be running on the managed repositories, they should use the new run methods with a repo_model.Repository as the parameter but not a repoPath.

For other git command which will be used on temporory repositories, it can use git.Command as before.

@lunny lunny added the type/refactoring Existing code has been cleaned up. There should be no new functionality. label Jan 26, 2024
@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Jan 26, 2024
@pull-request-size pull-request-size bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Jan 26, 2024
6543 pushed a commit that referenced this pull request Jan 27, 2024
## Purpose
This is a refactor toward building an abstraction over managing git
repositories.
Afterwards, it does not matter anymore if they are stored on the local
disk or somewhere remote.

## What this PR changes
We used `git.OpenRepository` everywhere previously.
Now, we should split them into two distinct functions:

Firstly, there are temporary repositories which do not change:

```go
git.OpenRepository(ctx, diskPath)
```

Gitea managed repositories having a record in the database in the
`repository` table are moved into the new package `gitrepo`:

```go
gitrepo.OpenRepository(ctx, repo_model.Repo)
```

Why is `repo_model.Repository` the second parameter instead of file
path?
Because then we can easily adapt our repository storage strategy.
The repositories can be stored locally, however, they could just as well
be stored on a remote server.

## Further changes in other PRs
- A Git Command wrapper on package `gitrepo` could be created. i.e.
`NewCommand(ctx, repo_model.Repository, commands...)`. `git.RunOpts{Dir:
repo.RepoPath()}`, the directory should be empty before invoking this
method and it can be filled in the function only. #28940
- Remove the `RepoPath()`/`WikiPath()` functions to reduce the
possibility of mistakes.

---------

Co-authored-by: delvh <dev.lh@web.de>
@lunny lunny mentioned this pull request Jan 28, 2024
@github-actions github-actions bot added the modifies/api This PR adds API routes or modifies them label Jan 28, 2024
@wxiaoguang wxiaoguang removed their request for review January 28, 2024 11:16
henrygoodman pushed a commit to henrygoodman/gitea that referenced this pull request Jan 31, 2024
## Purpose
This is a refactor toward building an abstraction over managing git
repositories.
Afterwards, it does not matter anymore if they are stored on the local
disk or somewhere remote.

## What this PR changes
We used `git.OpenRepository` everywhere previously.
Now, we should split them into two distinct functions:

Firstly, there are temporary repositories which do not change:

```go
git.OpenRepository(ctx, diskPath)
```

Gitea managed repositories having a record in the database in the
`repository` table are moved into the new package `gitrepo`:

```go
gitrepo.OpenRepository(ctx, repo_model.Repo)
```

Why is `repo_model.Repository` the second parameter instead of file
path?
Because then we can easily adapt our repository storage strategy.
The repositories can be stored locally, however, they could just as well
be stored on a remote server.

## Further changes in other PRs
- A Git Command wrapper on package `gitrepo` could be created. i.e.
`NewCommand(ctx, repo_model.Repository, commands...)`. `git.RunOpts{Dir:
repo.RepoPath()}`, the directory should be empty before invoking this
method and it can be filled in the function only. go-gitea#28940
- Remove the `RepoPath()`/`WikiPath()` functions to reduce the
possibility of mistakes.

---------

Co-authored-by: delvh <dev.lh@web.de>
@pull-request-size pull-request-size bot removed the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Feb 8, 2024
@pull-request-size pull-request-size bot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Feb 8, 2024
@lunny lunny marked this pull request as draft February 19, 2024 03:47
silverwind pushed a commit to silverwind/gitea that referenced this pull request Feb 20, 2024
## Purpose
This is a refactor toward building an abstraction over managing git
repositories.
Afterwards, it does not matter anymore if they are stored on the local
disk or somewhere remote.

## What this PR changes
We used `git.OpenRepository` everywhere previously.
Now, we should split them into two distinct functions:

Firstly, there are temporary repositories which do not change:

```go
git.OpenRepository(ctx, diskPath)
```

Gitea managed repositories having a record in the database in the
`repository` table are moved into the new package `gitrepo`:

```go
gitrepo.OpenRepository(ctx, repo_model.Repo)
```

Why is `repo_model.Repository` the second parameter instead of file
path?
Because then we can easily adapt our repository storage strategy.
The repositories can be stored locally, however, they could just as well
be stored on a remote server.

## Further changes in other PRs
- A Git Command wrapper on package `gitrepo` could be created. i.e.
`NewCommand(ctx, repo_model.Repository, commands...)`. `git.RunOpts{Dir:
repo.RepoPath()}`, the directory should be empty before invoking this
method and it can be filled in the function only. go-gitea#28940
- Remove the `RepoPath()`/`WikiPath()` functions to reduce the
possibility of mistakes.

---------

Co-authored-by: delvh <dev.lh@web.de>
@pull-request-size pull-request-size bot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Apr 13, 2024
@github-actions github-actions bot added modifies/go Pull requests that update Go code modifies/cli PR changes something on the CLI, i.e. gitea doctor or gitea admin labels Apr 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. modifies/api This PR adds API routes or modifies them modifies/cli PR changes something on the CLI, i.e. gitea doctor or gitea admin modifies/go Pull requests that update Go code size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. type/refactoring Existing code has been cleaned up. There should be no new functionality.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants