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

Add LFS Migration and Mirror #14726

Merged
merged 85 commits into from
Apr 8, 2021
Merged

Conversation

KN4CK3R
Copy link
Member

@KN4CK3R KN4CK3R commented Feb 18, 2021

Implemented downloading of remote LFS files with a custom HTTP/Filesystem client. The GIT LFS Client isn't flexible enough to integrate into the Gitea model.

LFS is supported for migrations and mirrors files periodicaly if mirroring is enabled. The client respects file size limits set in the Gitea config and uses the internal LFS structure to store the files. The client determines the LFS endpoint from the clone url if not specified. Migration of local filesystem repositories is supported too. In case of a mirrored repository LFS support can be (de)activated later in the repository settings.

Migration page:
migration

Migration of local repositories is supported:
local

Repository settings page:
settings

ToDo:

  • restructured code into clean modules / services seperation
  • file size restrictions on import
  • add support for custom LFS servers
  • add support for local filesystem repositories
  • integrate into ui
  • integrate into clone
  • integrate into mirror update
  • remember custom LFS server in mirror
  • manage LFS settings from repository settings page
  • use authentication
  • use different credentials should be implemented with Add Gitea secrets storage and management #14483

close #14718
close #849

@KN4CK3R KN4CK3R mentioned this pull request Feb 18, 2021
@6543 6543 added this to the 1.15.0 milestone Feb 18, 2021
@6543 6543 added the type/feature Completely new functionality. Can only be merged if feature freeze is not active. label Feb 18, 2021
@KN4CK3R
Copy link
Member Author

KN4CK3R commented Feb 19, 2021

After talking with @zeripath yesterday on Discord I moved some parts of the old LFS infrastructure to /services. So not all changed files are really "part of this PR". The change was needed because of endless import cycle errors.

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Feb 19, 2021
modules/lfs/client.go Outdated Show resolved Hide resolved
models/lfs.go Outdated Show resolved Hide resolved
modules/lfs/client.go Outdated Show resolved Hide resolved
modules/lfs/pointer.go Outdated Show resolved Hide resolved
return false, err
}

return true, nil
}

// ReadMetaObject will read a models.LFSMetaObject and return a reader
func ReadMetaObject(pointer *Pointer) (io.ReadCloser, error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

May need to consider cancellation here and elsewhere in this file - but that probably can wait for another pr.

modules/lfs/pointer_scanner_nogogit.go Outdated Show resolved Hide resolved
@zeripath
Copy link
Contributor

There are lots of places here where you are simply checking for an error and returning it straight up.

This is the equivalent of try-catch exception-throw exception.

Some of these errors shouldn't happen eg. Json errors but some of them will be almost indistinguishable from each other once they bubble up and get logged.

The simplest thing you can do is stick a log.Error before passing up the err. That is the absolute minimum that needs to happen around the http requests - unless some error is expected here. That would at least allow us to find the error. Some errors though should be being transformed to gain extra context.

I would also argue that some log.Debugs and Traces wouldn't go amiss. Tracing the http request URLs might not be a bad idea at all.

In general we're way to keen to make things * pointers in gitea - not everything needs to be a pointer - in particular immutable things probably shouldn't be.

modules/repository/repo.go Outdated Show resolved Hide resolved
@kdumontnu
Copy link
Contributor

@KN4CK3R can you resolve the conflicts here now that #15166 is merged? Will try to get this reviewed this week.

@kdumontnu
Copy link
Contributor

kdumontnu commented Apr 6, 2021

@zeripath @lunny Anything else you would like to see in this PR?

routers/repo/setting.go Outdated Show resolved Hide resolved
ctx.RenderWithErr(ctx.Tr("repo.migrate.invalid_lfs_endpoint"), tpl, &form)
return
}
err = migrations.IsMigrateURLAllowed(form.LFSEndpoint, ctx.User)
Copy link
Contributor

Choose a reason for hiding this comment

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

may need to run this on the ep instead.

errChan := make(chan error, 1)
wg := sync.WaitGroup{}
wg.Add(5)
for i, pointerBlob := range pointerBlobs {
Copy link
Contributor

Choose a reason for hiding this comment

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

pointerBlobs likely needs to become a channel

templates/repo/migrate/options.tmpl Outdated Show resolved Hide resolved
@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Apr 8, 2021
routers/repo/migrate.go Outdated Show resolved Hide resolved
@techknowlogick
Copy link
Member

🚀

@techknowlogick techknowlogick merged commit c03e488 into go-gitea:master Apr 8, 2021
@GlassedSilver
Copy link

Maybe it escaped my knowledge (which is quite limited in terms of how git works under the hood, granted), but apparently this doesn't automatically enable me to mirror releases, huh?

Or does the access token I generated on GitHub not have enough privileges? (I am deploying this on my personal LAN-only server, so I was already quite generous on the permission barring only the writes and some that seemed unrelated)

I am running the latest tag in docker as of now if that helps.

@techknowlogick
Copy link
Member

@GlassedSilver This PR is not for releases, but rather LFS. Release mirroring is not currently possible. Please also don't ask for help in closed tickets as they have a possibility of getting lost. For support please use our discourse forum or the discord chat.

@go-gitea go-gitea locked and limited conversation to collaborators Apr 11, 2021
@KN4CK3R KN4CK3R deleted the feature-lfs-clone branch May 7, 2021 06:45
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. type/feature Completely new functionality. Can only be merged if feature freeze is not active.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Repo mirroring doesn't mirror LFS data.
10 participants