Skip to content

refactor transport: plumbing: transport, refactor and define transport operations (2/5) #1337

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

Merged
merged 4 commits into from
Jan 2, 2025

Conversation

aymanbagabas
Copy link
Member

@aymanbagabas aymanbagabas commented Dec 31, 2024

This change redefines the transport package to be more modular and
extensible. It redefines the transport.Transport, transport.Session,
and transport.Commander and transport.Command interfaces. It also
introduces a new transport.Connection that abstracts the logic of
sending and receiving git data to and from a server.

There are 2 main transports. The pack transport which is a full-duplex
command based transport that uses transport.PackSession. And the other
is the HTTP transport which is a half-duplex transport that uses
transport.HTTPSession. All of file, git, and ssh transports use the
pack transport.

The rest of the abstractions are defined in transport.NegotiatePack,
transport.FetchPack, transport.UploadPack, transport.SendPack, and
tansport.ReceivePack. All of these static functions are used by the
transport.Connection to send and receive data to and from a
storage.Storer interface type.

Reference: https://gist.github.com/aymanbagabas/1b5ccc265ee62dd574c62d378b4b57bb

Based on: #1303

@aymanbagabas aymanbagabas force-pushed the v6-transport-transport branch from 15e2048 to 0f3b3ef Compare December 31, 2024 13:20
@aymanbagabas aymanbagabas changed the title refactor transport: plumbing: transport, refactor and define transport operations refactor transport: plumbing: transport, refactor and define transport operations (2/5) Dec 31, 2024
@aymanbagabas aymanbagabas force-pushed the v6-transport-transport branch from 0f3b3ef to f077b31 Compare December 31, 2024 16:00
@aymanbagabas aymanbagabas changed the base branch from v6-exp to v6-transport December 31, 2024 17:50
@aymanbagabas aymanbagabas marked this pull request as ready for review December 31, 2024 17:50
@aymanbagabas aymanbagabas requested a review from pjbgf December 31, 2024 17:50
Copy link
Member

@pjbgf pjbgf left a comment

Choose a reason for hiding this comment

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

This needs a rebase now that #1303 is merged.

@@ -0,0 +1,11 @@
package transport
Copy link
Member

Choose a reason for hiding this comment

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

This change may break the fuzzing configuration in oss-fuzz. But we will look at that prior to converting the v6-exp into main.

Copy link
Member Author

Choose a reason for hiding this comment

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

sg

return "", err
// NewTransport returns a new file transport that users go-git built-in server
// implementation to serve repositories.
func NewTransport(loader transport.Loader) transport.Transport {
Copy link
Member

Choose a reason for hiding this comment

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

I'd rather we start leaning on the options pattern instead - as per recent packfile changes. But we can refine that at a later point, before the V6 is released.

Copy link
Member Author

Choose a reason for hiding this comment

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

Sounds good

if c == nil {
c = &http.Client{
Transport: http.DefaultTransport,
func NewTransport(opts *TransportOptions) transport.Transport {
Copy link
Member

Choose a reason for hiding this comment

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

Same about the use of options here, ideally they would follow the same approach as this. But we can do that as a follow-up change.

@aymanbagabas aymanbagabas force-pushed the v6-transport-transport branch from f077b31 to 2d37d80 Compare January 1, 2025 13:51
This change redefines the transport package to be more modular and
extensible. It redefines the `transport.Transport`, `transport.Session`,
and `transport.Commander` and `transport.Command` interfaces. It also
introduces a new `transport.Connection` that abstracts the logic of
sending and receiving git data to and from a server.

There are 2 main transports. The pack transport which is a full-duplex
command based transport that uses `transport.PackSession`. And the other
is the HTTP transport which is a half-duplex transport that uses
`transport.HTTPSession`. All of file, git, and ssh transports use the
pack transport.

The rest of the abstractions are defined in `transport.NegotiatePack`,
`transport.FetchPack`, `transport.UploadPack`, `transport.SendPack`, and
`tansport.ReceivePack`. All of these static functions are used by the
`transport.Connection` to send and receive data to and from a
`storage.Storer` interface type.

Reference: https://gist.github.com/aymanbagabas/1b5ccc265ee62dd574c62d378b4b57bb
We no longer need a separate package for the server protocol, as it is
now implemented in the transport package.
It makes more sense to have this living in the transport package, as it
is related to the transport layer.
@aymanbagabas aymanbagabas force-pushed the v6-transport-transport branch from 2d37d80 to 95f4718 Compare January 1, 2025 13:57
return "", err
// NewTransport returns a new file transport that users go-git built-in server
// implementation to serve repositories.
func NewTransport(loader transport.Loader) transport.Transport {
Copy link
Member Author

Choose a reason for hiding this comment

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

Sounds good

@@ -0,0 +1,11 @@
package transport
Copy link
Member Author

Choose a reason for hiding this comment

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

sg

Co-authored-by: Paulo Gomes <paulo.gomes.uk@gmail.com>

Update remote.go

Co-authored-by: Paulo Gomes <paulo.gomes.uk@gmail.com>
@aymanbagabas aymanbagabas force-pushed the v6-transport-transport branch from a639d95 to 2279b8a Compare January 1, 2025 14:14
@aymanbagabas aymanbagabas requested a review from pjbgf January 1, 2025 14:14
@pjbgf pjbgf merged commit 8e38c94 into go-git:v6-transport Jan 2, 2025
2 of 14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants