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 as a standalone package so that it's easier to move the repositories to another machine #18147

Closed
wants to merge 30 commits into from

Conversation

lunny
Copy link
Member

@lunny lunny commented Jan 1, 2022

As a step to support distributed Git data storage, I make an abstract git service and command. Currently, just LocalService and LocalCommand has been implemented in this PR. But I think it's not very difficult to implement distributed one after this merged.

A new standalone command or a Gitea subcommand could be written to be as a git command service(http/tcp/grpc?). So the git commands could be sent to a remote service and retrieve the result. But this will be in future PRs.

@lunny lunny added the type/refactoring Existing code has been cleaned up. There should be no new functionality. label Jan 1, 2022
@lunny lunny force-pushed the lunny/git_command_as_interface branch 4 times, most recently from 7de8a0b to 19cf8ce Compare January 5, 2022 05:30
@lunny lunny mentioned this pull request Jan 21, 2022
@lunny lunny added this to the 1.17.0 milestone Jan 21, 2022
@lunny lunny force-pushed the lunny/git_command_as_interface branch 3 times, most recently from 19f24a4 to e24a459 Compare February 16, 2022 06:31
@lunny lunny mentioned this pull request Feb 17, 2022
@lunny lunny force-pushed the lunny/git_command_as_interface branch 4 times, most recently from ce34612 to 3c386b2 Compare March 3, 2022 06:49
@lunny lunny force-pushed the lunny/git_command_as_interface branch 5 times, most recently from 112282c to 111de28 Compare March 23, 2022 06:50
@lunny lunny force-pushed the lunny/git_command_as_interface branch 3 times, most recently from 8fd9a4d to aa9527a Compare April 2, 2022 08:56
@lunny lunny requested review from wxiaoguang and zeripath April 2, 2022 12:17
modules/git/blame.go Outdated Show resolved Hide resolved
@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Apr 6, 2022
@wxiaoguang
Copy link
Contributor

wxiaoguang commented Apr 6, 2022

If the git package should be refactored to support local/remote, it might be like this:

git.Command (interface)
git.localcmd.Command
git.remotecmd.Command

However, it seems too early to introduce the local/remote package, is there any real requirement at the moment or a detailed design? If there is no real requirement and design, what is done now might be wrong in future.

So the package level refactoring should be made after there is clear requirement and design.

Copy link
Contributor

@wxiaoguang wxiaoguang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Replacing process.GetManager().Exec with git.Command.Run is fine, and RunStdString could be used to simplify the code.

@lunny lunny force-pushed the lunny/git_command_as_interface branch from 497abbc to 03c1045 Compare June 5, 2022 02:53
@codecov-commenter
Copy link

Codecov Report

Merging #18147 (03c1045) into main (6171ea7) will increase coverage by 0.01%.
The diff coverage is 63.63%.

@@            Coverage Diff             @@
##             main   #18147      +/-   ##
==========================================
+ Coverage   47.28%   47.30%   +0.01%     
==========================================
  Files         957      959       +2     
  Lines      133374   133557     +183     
==========================================
+ Hits        63067    63179     +112     
- Misses      62633    62713      +80     
+ Partials     7674     7665       -9     
Impacted Files Coverage Δ
cmd/serv.go 2.32% <0.00%> (-0.02%) ⬇️
models/action.go 48.92% <0.00%> (ø)
models/db/engine.go 33.55% <ø> (ø)
modules/context/repo.go 51.42% <0.00%> (ø)
modules/git/repo_attribute.go 68.51% <ø> (ø)
modules/markup/external/external.go 1.31% <0.00%> (-0.04%) ⬇️
modules/setting/repository.go 55.71% <ø> (ø)
routers/api/v1/repo/release_attachment.go 0.00% <0.00%> (ø)
routers/install/install.go 1.77% <0.00%> (-0.01%) ⬇️
services/mailer/mailer.go 29.79% <0.00%> (-0.25%) ⬇️
... and 55 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update dadcaa4...03c1045. Read the comment docs.

@lunny lunny modified the milestones: 1.17.0, 1.18.0 Jun 5, 2022
@lunny lunny modified the milestones: 1.18.0, 1.19.0 Oct 17, 2022
@lunny
Copy link
Member Author

lunny commented Dec 11, 2022

Since this PR opened, the command has been changed a lot. Just close it.

@lunny lunny closed this Dec 11, 2022
@lunny lunny removed this from the 1.19.0 milestone Dec 11, 2022
@6543
Copy link
Member

6543 commented Dec 23, 2022

still it should be done at some point ... to make it possible to have different back-ends for this, I'll put that on my "long term todos"

@lunny lunny deleted the lunny/git_command_as_interface branch December 24, 2022 01:20
lunny added a commit that referenced this pull request Jan 3, 2023
extract from #18147

Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>
@go-gitea go-gitea locked and limited conversation to collaborators May 3, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. 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.

5 participants