-
Notifications
You must be signed in to change notification settings - Fork 809
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
refactor transport: plumbing: transport, refactor and define transport operations (2/5) #1337
Conversation
15e2048
to
0f3b3ef
Compare
0f3b3ef
to
f077b31
Compare
There was a problem hiding this 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 |
There was a problem hiding this comment.
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
.
There was a problem hiding this comment.
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 { |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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 { |
There was a problem hiding this comment.
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.
f077b31
to
2d37d80
Compare
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.
2d37d80
to
95f4718
Compare
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 { |
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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>
a639d95
to
2279b8a
Compare
This change redefines the transport package to be more modular and
extensible. It redefines the
transport.Transport
,transport.Session
,and
transport.Commander
andtransport.Command
interfaces. It alsointroduces a new
transport.Connection
that abstracts the logic ofsending 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 otheris the HTTP transport which is a half-duplex transport that uses
transport.HTTPSession
. All of file, git, and ssh transports use thepack transport.
The rest of the abstractions are defined in
transport.NegotiatePack
,transport.FetchPack
,transport.UploadPack
,transport.SendPack
, andtansport.ReceivePack
. All of these static functions are used by thetransport.Connection
to send and receive data to and from astorage.Storer
interface type.Reference: https://gist.github.com/aymanbagabas/1b5ccc265ee62dd574c62d378b4b57bb
Based on: #1303