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

Refactor editor upload, update and delete to use git plumbing and add LFS support #5702

Merged
merged 29 commits into from Feb 12, 2019

Conversation

@zeripath
Copy link
Contributor

zeripath commented Jan 11, 2019

When uploading files using the GUI, Gitea currently does:

a) A full clone - Including the whole object DB
b) A full branch checkout - every file
c) Copies the uploaded file to this cloned repository
d) Add/stages the changes
e) Commits
f) Pushes to the real repository
g) Cleans up the cloned repository - deleting the whole object db and checkout.

This is extremely inefficient.

This PR takes a different approach and, instead of using the git porcelain commands (git add and the like), uses the plumbing commands to (hopefully) significantly speed up this process. See my comments in #601 (#601 (comment)).

The basics of this code was in #5621, however, there has been significant refactoring with code moved out of models.Repository into a module called uploader. LFS support has been added, with transparent recognition of whether a file should be added to the LFS through checking the git attributes.

Issues

  • Currently my approach is to use the git-cli commands which can/could be migrated to go-git commands later. Some of this code could and probably should be moved into the gitea git subproject.
  • There are inefficiencies in the LFS code which could be improved - for example, when files are uploaded to the server, their sha1dcsum and sha256sum should be created at that point rather than read the whole file again - and the LFS ContentStore will currently regenerate the sha256sum and copy the file again.
  • I don't set ORIGINAL_SSH_COMMAND so the push hooks may not actually run. This is to stay in keeping with the current implementation - however, could be very easily set.
  • It's not possible to update LFS files, although it is possible to create new text files that will become LFS files.

Will fix #5600. Will fix #4778. Will fix #3557. Will fix #5727.

@codecov-io

This comment has been minimized.

Copy link

codecov-io commented Jan 11, 2019

Codecov Report

❗️ No coverage uploaded for pull request base (master@baffea1). Click here to learn what that means.
The diff coverage is 32.51%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master    #5702   +/-   ##
=========================================
  Coverage          ?   38.82%           
=========================================
  Files             ?      344           
  Lines             ?    49355           
  Branches          ?        0           
=========================================
  Hits              ?    19163           
  Misses            ?    27428           
  Partials          ?     2764
Impacted Files Coverage Δ
modules/uploader/diff.go 0% <0%> (ø)
modules/uploader/delete.go 0% <0%> (ø)
models/lfs.go 32.72% <0%> (ø)
models/upload.go 0% <0%> (ø)
modules/uploader/upload.go 0% <0%> (ø)
routers/repo/editor.go 28.67% <26.31%> (ø)
modules/uploader/update.go 35.71% <35.71%> (ø)
models/repo_branch.go 58.27% <56%> (ø)
modules/uploader/repo.go 67.04% <67.04%> (ø)

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 baffea1...9c3603b. Read the comment docs.

@bkcsoft bkcsoft added the lgtm/need 2 label Jan 11, 2019

@zeripath

This comment has been minimized.

Copy link
Contributor Author

zeripath commented Jan 11, 2019

As previously described in #5621 this provides around a 10x speed up on the current implementation even for very small repositories.


Speed ups for large repositories should be considerably better.

@lunny lunny added the kind/refactor label Jan 12, 2019

@lunny lunny added this to the 1.8.0 milestone Jan 12, 2019

@zeripath zeripath referenced this pull request Jan 14, 2019

Closed

web edit: dotfiles getting renamed #5727

2 of 7 tasks complete
Show resolved Hide resolved models/upload.go Outdated
Show resolved Hide resolved models/upload.go Outdated
Show resolved Hide resolved models/upload.go

zeripath added some commits Dec 31, 2018

BUGFIX: The default permissions should be 100644
    This is a change from the previous code but is more in keeping
    with the default behaviour of git.

Signed-off-by: Andrew Thornton <art27@cantab.net>
Redirect on bad paths
Signed-off-by: Andrew Thornton <art27@cantab.net>
Refactor to move the uploading functions out to a module
Signed-off-by: Andrew Thornton <art27@cantab.net>
Add LFS support
Signed-off-by: Andrew Thornton <art27@cantab.net>
Update upload.go attribution header
Upload.go is essentially the remnants of repo_editor.go. The remaining code is essentially unchanged from the Gogs code, hence the Gogs attribution.
Ensure that GIT_AUTHOR_NAME etc. are valid for git
see #5774

Signed-off-by: Andrew Thornton <art27@cantab.net>

@zeripath zeripath force-pushed the zeripath:make-upload-not-clone branch from 8686557 to d0d28c9 Jan 20, 2019

Show resolved Hide resolved routers/repo/editor_test.go Outdated

@zeripath zeripath force-pushed the zeripath:make-upload-not-clone branch from 800d6c3 to d6ceb0c Feb 6, 2019

zeripath and others added some commits Feb 6, 2019

zeripath added some commits Feb 7, 2019

Add space between gitea and github imports
Signed-off-by: Andrew Thornton <art27@cantab.net>
more examples in TestCleanUploadName
Signed-off-by: Andrew Thornton <art27@cantab.net>
fix formatting
Signed-off-by: Andrew Thornton <art27@cantab.net>
@lafriks

lafriks approved these changes Feb 7, 2019

@GiteaBot GiteaBot added lgtm/done and removed lgtm/need 2 labels Feb 7, 2019

@zeripath

This comment has been minimized.

Copy link
Contributor Author

zeripath commented Feb 8, 2019

@lunny needs your review.

zeripath added some commits Feb 8, 2019

Set the SSH_ORIGINAL_COMMAND to ensure hooks are run
Signed-off-by: Andrew Thornton <art27@cantab.net>

@zeripath zeripath referenced this pull request Feb 9, 2019

Open

no git hook executed during "gitea squash and merge" #6003

2 of 7 tasks complete

zeripath added some commits Feb 9, 2019

Switch off SSH_ORIGINAL_COMMAND
Signed-off-by: Andrew Thornton <art27@cantab.net>
@zeripath

This comment has been minimized.

Copy link
Contributor Author

zeripath commented Feb 12, 2019

Ping @lunny

@lunny

lunny approved these changes Feb 12, 2019

@zeripath

This comment has been minimized.

Copy link
Contributor Author

zeripath commented Feb 12, 2019

Make lg TM work

@lafriks lafriks merged commit 296814e into go-gitea:master Feb 12, 2019

2 checks passed

approvals/lgtm this commit looks good
continuous-integration/drone/pr the build was successful
Details

@zeripath zeripath deleted the zeripath:make-upload-not-clone branch Feb 12, 2019

@richmahn

This comment has been minimized.

Copy link
Contributor

richmahn commented Feb 15, 2019

Any way to retain commit history when a file is renamed in the GUI? Now it deletes the old file, makes a new file. That isn't the behavior of GitHub when you change the name or path of an existing file.

@lafriks

This comment has been minimized.

Copy link
Member

lafriks commented Feb 16, 2019

@richmahn please submit issue for this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment