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

Git LFS support #120

Closed
wants to merge 16 commits into from
Closed

Git LFS support #120

wants to merge 16 commits into from

Conversation

fabian-z
Copy link
Contributor

@fabian-z fabian-z commented Nov 8, 2016

PR as requested by @lunny, merge conflicts originate from gitea commits. I'm using Gogs as upstream so I can't promise to correct those. Original PR: gogs/gogs#3868

Feature requested in gogs/gogs#1322

This PR implements Git LFS support for Gogs, supporting the legacy v1 API as well as the current batch v1 API.

Originally imported LFS server code base from https://github.com/git-lfs/lfs-test-server.

Features:

  • Uses recommended paths with HTTP and SSH remotes, no custom user settings necessary
  • Supports basic authentication with user credentials
  • Supports authentication with SSH (passing JWT under the hood)
  • Supports no authentication when pulling public repositories (however git-lfs still asks for credentials, where the prompt can be skipped - likely a bug in git-lfs)
  • Settings transparently integrated into installation process
  • Removes LFS objects when repositories are deleted. Works with forks (e.g. when parent repository is deleted)

Notes:

Looking forward to your review!

@lunny lunny added the type/proposal The new feature has not been accepted yet but needs to be discussed first. label Nov 8, 2016
@lunny lunny added this to the 1.1.0 milestone Nov 8, 2016
@lunny
Copy link
Member

lunny commented Nov 8, 2016

This will resolved #80

@strk
Copy link
Member

strk commented Nov 8, 2016

Can you please rebase and avoid generating bindata.go in the PR ?
Also don't bother touching glide, we use govendor here

@fabian-z
Copy link
Contributor Author

fabian-z commented Nov 8, 2016

@strk

Please read the note in my PR. I sent this PR to gogits/gogs and only copied it here on request by @lunny. As my time is limited and my upstream is Gogs, I currently won't be able to rebase / fix issues for gitea. Thanks for your understanding.

Only removes objects from content store when deleted repo is the only
referencing repository
"git-upload-pack": models.ACCESS_MODE_READ,
"git-upload-archive": models.ACCESS_MODE_READ,
"git-receive-pack": models.ACCESS_MODE_WRITE,
"git-lfs-authenticate": models.ACCESS_MODE_NONE,
Copy link
Member

Choose a reason for hiding this comment

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

Why not use LFSAuthenticateVerb here?

@strk
Copy link
Member

strk commented Nov 8, 2016

On Tue, Nov 08, 2016 at 07:28:27AM -0800, bkcsoft wrote:

  • "git-upload-pack":    models.ACCESS_MODE_READ,
    
  • "git-upload-archive": models.ACCESS_MODE_READ,
    
  • "git-receive-pack":   models.ACCESS_MODE_WRITE,
    
  • "git-upload-pack":      models.ACCESS_MODE_READ,
    
  • "git-upload-archive":   models.ACCESS_MODE_READ,
    
  • "git-receive-pack":     models.ACCESS_MODE_WRITE,
    
  • "git-lfs-authenticate": models.ACCESS_MODE_NONE,
    

And I think those constants are named AccessModeNone etc. now.
(but I understand the author is not interested in porting this to
Gitea)

Fixes bug where LFS would not work after installation without
restarting Gogs
@fabian-z
Copy link
Contributor Author

fabian-z commented Nov 8, 2016

Let me clarify on my statement.
Please correct me if I'm wrong or I misunderstood the situation, but Gitea seems to be maintained as a constantly rebased fork of Gogs (like a changeset maintained by downstream distributions). Therefore, my priority would be to get this reviewed and merged upstream with Gogs. If this is possible, a rebase would bring the feature to Gitea and the changeset which needs to be maintained can remain as small as possible.
However, my main interest is to bring this feature into active usage and to have continued development together with other contributors.
If my PR is not reviewed or merged into Gogs master in a reasonable amount of time, I'd be glad to work with you to port my changes to Gitea.

@strk
Copy link
Member

strk commented Nov 8, 2016

@fabian-z you misunderstood: Gitea will never be rebased against Gogs, but rather continue on its own.
The fork was driven by the willingness to have a more open development method, so you're welcome to join the effort. You can compare Gogs and Gitea activity via the Pulse screen:

https://github.com/go-gitea/gitea/pulse
https://github.com/gogits/gogs/pulse

@fabian-z
Copy link
Contributor Author

fabian-z commented Nov 8, 2016

@strk
Thank you for elaborating on the project status. Indeed I must have misjudged on first glance - thanks again for pointing me to the Pulse graphs.
I definitely prefer an open, collaborative development, too.
I will try to rebase this on Gitea master and provide a new PR incorporating your reviews.

@fabian-z fabian-z closed this Nov 8, 2016
@fabian-z fabian-z mentioned this pull request Nov 8, 2016
@lunny lunny removed this from the 1.1.0 milestone Nov 9, 2016
@tboerger tboerger removed the type/proposal The new feature has not been accepted yet but needs to be discussed first. label Nov 12, 2016
@fabian-z fabian-z changed the title Git LFS support 2Git LFS support Nov 14, 2016
@fabian-z fabian-z changed the title 2Git LFS support Git LFS support Nov 15, 2016
lunny added a commit to lunny/gitea that referenced this pull request Feb 7, 2019
@go-gitea go-gitea locked and limited conversation to collaborators Nov 23, 2020
@delvh delvh added issue/not-a-bug The reported issue is the intended behavior or the problem is not inside Gitea and removed reviewed/invalid labels Oct 7, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
issue/not-a-bug The reported issue is the intended behavior or the problem is not inside Gitea
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Build error with 0.9.99.0915
6 participants