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 push SSH LFS protocol support #31448

Closed
wants to merge 6 commits into from

Conversation

ConcurrentCrab
Copy link

@ConcurrentCrab ConcurrentCrab commented Jun 21, 2024

Fixes #17554

/claim #17554

DONTMERGE, this is very much a minimum-changes-required commit, probably needs cleanup and documentation changes.

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Jun 21, 2024
@pull-request-size pull-request-size bot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Jun 21, 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 Jun 21, 2024
@delvh delvh marked this pull request as draft June 21, 2024 21:55
@pull-request-size pull-request-size bot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Jun 27, 2024
@github-actions github-actions bot removed modifies/translation modifies/api This PR adds API routes or modifies them modifies/templates This PR modifies the template files modifies/migrations modifies/internal modifies/js labels Jun 27, 2024
@ConcurrentCrab ConcurrentCrab changed the title [WIP] Add push SSH LFS protocol support Add push SSH LFS protocol support Jun 28, 2024
@ConcurrentCrab ConcurrentCrab marked this pull request as ready for review June 28, 2024 03:57
@ConcurrentCrab
Copy link
Author

Pushing and cloning work well now. Removing draft label.

To test, run pushes like: GIT_TRACE=1 git push. The trace output should mention "pure SSH connection".

The code here checks if the repo being requested doesn't exist.
If it doesn't, then a write operation might create it. But a read
operation doesn't make any sense, and should error out. So simply
check the access mode.

I assume this was the intent here, but only checked for one "verb"
instead, while there exist other read-only verbs as well. And ofc
more can be introduced in the future ;)

Possibly some write verbs don't make sense as well (presumably
those that only add stuff incrementally to existing repos)?
Coalesce access mode detection into one place.
Yes, "upload" really has opposite semantics for
git commands vs. git-lfs commands. Wow.

This commit makes no functional changes.
A download command in the SSH protocol doesn't specify size, so
this was necessary.
Big changes are inevitable due to the difference in Gitea's approach
to storing LFS files. And upstream hasn't tagged for a while anyway.
Make a few changes to make the transfer backend suit Gitea better.

Merge Upload into one method:
  Makes things simpler since out content store verifies size & hashes
  itself

Change Download return into io.ReadCloser:
  Removes dependency on filesystem

Add size: int64 arguments wherever appropriate
  In arguments for Upload and Verify, and in return for Download
Also add handler in runServ()

The protocol lib supports locking but the backend does not,
as neither does Gitea. Support can be added later and the
capability advertised.
@ConcurrentCrab
Copy link
Author

Changed package from lfs_ssh to lfstransfer to satisfy lint ¯\(ツ)

@ConcurrentCrab
Copy link
Author

Continued in #31516

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/cli PR changes something on the CLI, i.e. gitea doctor or gitea admin modifies/dependencies modifies/docs modifies/go Pull requests that update Go code size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support LFS purely over SSH protocol
2 participants