From db2367f389170b7bb5d76325524fe0559e1604a3 Mon Sep 17 00:00:00 2001 From: Michael Kozono Date: Mon, 15 Sep 2025 11:05:35 -1000 Subject: [PATCH 1/2] Revert "Merge branch 'jtapiab-add-done-pkt-git-clone-depth-option' into 'main'" This reverts commit 69df28766ad141691517c9709c787681b1d62215, reversing changes made to 63992a590c8ee4c2c68923e27cdae875d97b769f. --- internal/command/githttp/pull.go | 17 +++-------- internal/command/githttp/pull_test.go | 33 +++++---------------- internal/gitlabnet/accessverifier/client.go | 19 ++++++------ 3 files changed, 20 insertions(+), 49 deletions(-) diff --git a/internal/command/githttp/pull.go b/internal/command/githttp/pull.go index cefa8850..e82d4975 100644 --- a/internal/command/githttp/pull.go +++ b/internal/command/githttp/pull.go @@ -61,7 +61,7 @@ func (c *PullCommand) Execute(ctx context.Context) error { return err } - return c.requestUploadPack(ctx, client, data.GeoProxyFetchDirectToPrimaryWithOptions) + return c.requestUploadPack(ctx, client) } func (c *PullCommand) requestSSHUploadPack(ctx context.Context, client *git.Client) error { @@ -76,9 +76,9 @@ func (c *PullCommand) requestSSHUploadPack(ctx context.Context, client *git.Clie return err } -func (c *PullCommand) requestUploadPack(ctx context.Context, client *git.Client, geoProxyFetchDirectToPrimaryWithOptions bool) error { +func (c *PullCommand) requestUploadPack(ctx context.Context, client *git.Client) error { pipeReader, pipeWriter := io.Pipe() - go c.readFromStdin(pipeWriter, geoProxyFetchDirectToPrimaryWithOptions) + go c.readFromStdin(pipeWriter) response, err := client.UploadPack(ctx, pipeReader) if err != nil { @@ -91,7 +91,7 @@ func (c *PullCommand) requestUploadPack(ctx context.Context, client *git.Client, return err } -func (c *PullCommand) readFromStdin(pw *io.PipeWriter, geoProxyFetchDirectToPrimaryWithOptions bool) { +func (c *PullCommand) readFromStdin(pw *io.PipeWriter) { scanner := pktline.NewScanner(c.ReadWriter.In) for scanner.Scan() { @@ -105,15 +105,6 @@ func (c *PullCommand) readFromStdin(pw *io.PipeWriter, geoProxyFetchDirectToPrim if pktline.IsDone(line) { break } - - if pktline.IsFlush(line) && geoProxyFetchDirectToPrimaryWithOptions { - _, err := pw.Write(pktline.PktDone()) - if err != nil { - log.WithError(err).Error("failed to write packet done line") - } - - break - } } err := pw.Close() diff --git a/internal/command/githttp/pull_test.go b/internal/command/githttp/pull_test.go index dcf29136..2732b4c0 100644 --- a/internal/command/githttp/pull_test.go +++ b/internal/command/githttp/pull_test.go @@ -21,12 +21,13 @@ import ( var cloneResponse = `0090want 11d731b83788cd556abea7b465c6bee52d89923c multi_ack_detailed side-band-64k thin-pack ofs-delta deepen-since deepen-not agent=git/2.41.0 0032want e56497bb5f03a90a51293fc6d516788730953899 +00000009done ` func TestPullExecute(t *testing.T) { url := setupPull(t, http.StatusOK) output := &bytes.Buffer{} - input := strings.NewReader(cloneResponse + "00000009done\n") + input := strings.NewReader(cloneResponse) cmd := &PullCommand{ Config: &config.Config{GitlabUrl: url}, @@ -42,29 +43,10 @@ func TestPullExecute(t *testing.T) { require.Equal(t, infoRefsWithoutPrefix, output.String()) } -func TestPullExecuteWithDepth(t *testing.T) { - url := setupPull(t, http.StatusOK) - output := &bytes.Buffer{} - input := strings.NewReader(cloneResponse + "0000\n") - - cmd := &PullCommand{ - Config: &config.Config{GitlabUrl: url}, - ReadWriter: &readwriter.ReadWriter{Out: output, In: input}, - Response: &accessverifier.Response{ - Payload: accessverifier.CustomPayload{ - Data: accessverifier.CustomPayloadData{PrimaryRepo: url, GeoProxyFetchDirectToPrimaryWithOptions: true}, - }, - }, - } - - require.NoError(t, cmd.Execute(context.Background())) - require.Equal(t, infoRefsWithoutPrefix, output.String()) -} - func TestPullExecuteWithSSHUploadPack(t *testing.T) { url := setupSSHPull(t, http.StatusOK) output := &bytes.Buffer{} - input := strings.NewReader(cloneResponse + "0009done\n") + input := strings.NewReader(cloneResponse) cmd := &PullCommand{ Config: &config.Config{GitlabUrl: url}, @@ -72,10 +54,9 @@ func TestPullExecuteWithSSHUploadPack(t *testing.T) { Response: &accessverifier.Response{ Payload: accessverifier.CustomPayload{ Data: accessverifier.CustomPayloadData{ - PrimaryRepo: url, - GeoProxyFetchDirectToPrimaryWithOptions: true, - GeoProxyFetchSSHDirectToPrimary: true, - RequestHeaders: map[string]string{"Authorization": "token"}, + PrimaryRepo: url, + GeoProxyFetchSSHDirectToPrimary: true, + RequestHeaders: map[string]string{"Authorization": "token"}, }, }, }, @@ -144,7 +125,7 @@ func TestPullExecuteWithFailedInfoRefs(t *testing.T) { func TestExecuteWithFailedUploadPack(t *testing.T) { url := setupPull(t, http.StatusForbidden) output := &bytes.Buffer{} - input := strings.NewReader(cloneResponse + "00000009done\n") + input := strings.NewReader(cloneResponse) cmd := &PullCommand{ Config: &config.Config{GitlabUrl: url}, diff --git a/internal/gitlabnet/accessverifier/client.go b/internal/gitlabnet/accessverifier/client.go index 1b356fa0..ead052fe 100644 --- a/internal/gitlabnet/accessverifier/client.go +++ b/internal/gitlabnet/accessverifier/client.go @@ -48,16 +48,15 @@ type Gitaly struct { // CustomPayloadData represents custom payload data type CustomPayloadData struct { - APIEndpoints []string `json:"api_endpoints"` - Username string `json:"gl_username"` - PrimaryRepo string `json:"primary_repo"` - UserID string `json:"gl_id,omitempty"` - RequestHeaders map[string]string `json:"request_headers"` - GeoProxyDirectToPrimary bool `json:"geo_proxy_direct_to_primary"` - GeoProxyFetchDirectToPrimary bool `json:"geo_proxy_fetch_direct_to_primary"` - GeoProxyFetchDirectToPrimaryWithOptions bool `json:"geo_proxy_fetch_direct_to_primary_with_options"` - GeoProxyFetchSSHDirectToPrimary bool `json:"geo_proxy_fetch_ssh_direct_to_primary"` - GeoProxyPushSSHDirectToPrimary bool `json:"geo_proxy_push_ssh_direct_to_primary"` + APIEndpoints []string `json:"api_endpoints"` + Username string `json:"gl_username"` + PrimaryRepo string `json:"primary_repo"` + UserID string `json:"gl_id,omitempty"` + RequestHeaders map[string]string `json:"request_headers"` + GeoProxyDirectToPrimary bool `json:"geo_proxy_direct_to_primary"` + GeoProxyFetchDirectToPrimary bool `json:"geo_proxy_fetch_direct_to_primary"` + GeoProxyFetchSSHDirectToPrimary bool `json:"geo_proxy_fetch_ssh_direct_to_primary"` + GeoProxyPushSSHDirectToPrimary bool `json:"geo_proxy_push_ssh_direct_to_primary"` } // CustomPayload represents a custom payload -- GitLab From cc9c2c5961d5a9bb635cd993677f22ca2a8a8838 Mon Sep 17 00:00:00 2001 From: Michael Kozono Date: Mon, 15 Sep 2025 13:02:14 -1000 Subject: [PATCH 2/2] Accept new lint error It is not worth the indirection to deduplicate these two similar functions --- support/lint_last_known_acceptable.txt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/support/lint_last_known_acceptable.txt b/support/lint_last_known_acceptable.txt index 8bb2e647..ef4015a4 100644 --- a/support/lint_last_known_acceptable.txt +++ b/support/lint_last_known_acceptable.txt @@ -46,6 +46,8 @@ internal/command/command.go:37:1: exported: exported function CheckForVersionFla internal/command/command.go:48:1: exported: comment on exported function Setup should be of the form "Setup ..." (revive) internal/command/command.go:80:15: Error return value of `closer.Close` is not checked (errcheck) internal/command/command.go:84:1: exported: exported function NewLogData should have comment or be unexported (revive) +internal/command/githttp/pull.go:48: 48-65 lines are duplicate of `internal/command/githttp/push.go:45-62` (dupl) +internal/command/githttp/push.go:45: 45-62 lines are duplicate of `internal/command/githttp/pull.go:48-65` (dupl) internal/command/lfsauthenticate/lfsauthenticate.go:87:13: Error return value of `fmt.Fprintf` is not checked (errcheck) internal/command/lfstransfer/gitlab_backend.go:40:6: exported: exported type GitlabAuthentication should have comment or be unexported (revive) internal/command/lfstransfer/gitlab_backend.go:45:6: exported: exported type GitlabBackend should have comment or be unexported (revive) -- GitLab