From 2a22162a209fa66e045f26ff249d425676295cf5 Mon Sep 17 00:00:00 2001 From: CH-Chang Date: Sat, 11 Jan 2025 03:58:51 +0000 Subject: [PATCH 1/6] feat: add git clone with thinkpack option to support self hosted azure devops --- docs/env-variables.md | 1 + git/git.go | 4 +++- options/options.go | 8 ++++++++ options/options_test.go | 8 ++++++++ options/testdata/options.golden | 3 +++ 5 files changed, 23 insertions(+), 1 deletion(-) diff --git a/docs/env-variables.md b/docs/env-variables.md index 861f31b0..a015883d 100644 --- a/docs/env-variables.md +++ b/docs/env-variables.md @@ -27,6 +27,7 @@ | `--git-url` | `ENVBUILDER_GIT_URL` | | The URL of a Git repository containing a Devcontainer or Docker image to clone. This is optional. | | `--git-clone-depth` | `ENVBUILDER_GIT_CLONE_DEPTH` | | The depth to use when cloning the Git repository. | | `--git-clone-single-branch` | `ENVBUILDER_GIT_CLONE_SINGLE_BRANCH` | | Clone only a single branch of the Git repository. | +| `--git-clone-thinpack` | `ENVBUILDER_GIT_CLONE_THINPACK` | | Clone with thin pack compabilities. | | `--git-username` | `ENVBUILDER_GIT_USERNAME` | | The username to use for Git authentication. This is optional. | | `--git-password` | `ENVBUILDER_GIT_PASSWORD` | | The password to use for Git authentication. This is optional. | | `--git-ssh-private-key-path` | `ENVBUILDER_GIT_SSH_PRIVATE_KEY_PATH` | | Path to an SSH private key to be used for Git authentication. If this is set, then GIT_SSH_PRIVATE_KEY_BASE64 cannot be set. | diff --git a/git/git.go b/git/git.go index f37b9682..e3279b55 100644 --- a/git/git.go +++ b/git/git.go @@ -37,6 +37,7 @@ type CloneRepoOptions struct { Progress sideband.Progress Insecure bool SingleBranch bool + ThinPack bool Depth int CABundle []byte ProxyOptions transport.ProxyOptions @@ -53,7 +54,7 @@ func CloneRepo(ctx context.Context, logf func(string, ...any), opts CloneRepoOpt return false, fmt.Errorf("parse url %q: %w", opts.RepoURL, err) } logf("Parsed Git URL as %q", parsed.Redacted()) - if parsed.Hostname() == "dev.azure.com" { + if parsed.Hostname() == "dev.azure.com" || opts.ThinPack { // Azure DevOps requires capabilities multi_ack / multi_ack_detailed, // which are not fully implemented and by default are included in // transport.UnsupportedCapabilities. @@ -347,6 +348,7 @@ func CloneOptionsFromOptions(logf func(string, ...any), options options.Options) Storage: options.Filesystem, Insecure: options.Insecure, SingleBranch: options.GitCloneSingleBranch, + ThinPack: options.GitCloneThinPack, Depth: int(options.GitCloneDepth), CABundle: caBundle, } diff --git a/options/options.go b/options/options.go index c2b6efe6..ecac6477 100644 --- a/options/options.go +++ b/options/options.go @@ -106,6 +106,8 @@ type Options struct { GitCloneDepth int64 // GitCloneSingleBranch clone only a single branch of the Git repository. GitCloneSingleBranch bool + // GitCloneThinPack clone with thin pack compabilities. This is optional. + GitCloneThinPack bool // GitUsername is the username to use for Git authentication. This is // optional. GitUsername string @@ -375,6 +377,12 @@ func (o *Options) CLI() serpent.OptionSet { Value: serpent.BoolOf(&o.GitCloneSingleBranch), Description: "Clone only a single branch of the Git repository.", }, + { + Flag: "git-clone-thinpack", + Env: WithEnvPrefix("GIT_CLONE_THINPACK"), + Value: serpent.BoolOf(&o.GitCloneThinPack), + Description: "Clone with thin pack compabilities.", + }, { Flag: "git-username", Env: WithEnvPrefix("GIT_USERNAME"), diff --git a/options/options_test.go b/options/options_test.go index bf7a216c..1a92c8b9 100644 --- a/options/options_test.go +++ b/options/options_test.go @@ -38,31 +38,39 @@ func TestEnvOptionParsing(t *testing.T) { t.Run("lowercase", func(t *testing.T) { t.Setenv(options.WithEnvPrefix("SKIP_REBUILD"), "true") t.Setenv(options.WithEnvPrefix("GIT_CLONE_SINGLE_BRANCH"), "false") + t.Setenv(options.WithEnvPrefix("GIT_CLONE_THINPACK"), "false") o := runCLI() require.True(t, o.SkipRebuild) require.False(t, o.GitCloneSingleBranch) + require.False(t, o.GitCloneThinPack) }) t.Run("uppercase", func(t *testing.T) { t.Setenv(options.WithEnvPrefix("SKIP_REBUILD"), "TRUE") t.Setenv(options.WithEnvPrefix("GIT_CLONE_SINGLE_BRANCH"), "FALSE") + t.Setenv(options.WithEnvPrefix("GIT_CLONE_THINPACK"), "FALSE") o := runCLI() require.True(t, o.SkipRebuild) require.False(t, o.GitCloneSingleBranch) + require.False(t, o.GitCloneThinPack) }) t.Run("numeric", func(t *testing.T) { t.Setenv(options.WithEnvPrefix("SKIP_REBUILD"), "1") t.Setenv(options.WithEnvPrefix("GIT_CLONE_SINGLE_BRANCH"), "0") + t.Setenv(options.WithEnvPrefix("GIT_CLONE_THINPACK"), "0") o := runCLI() require.True(t, o.SkipRebuild) require.False(t, o.GitCloneSingleBranch) + require.False(t, o.GitCloneThinPack) }) t.Run("empty", func(t *testing.T) { t.Setenv(options.WithEnvPrefix("GIT_CLONE_SINGLE_BRANCH"), "") + t.Setenv(options.WithEnvPrefix("GIT_CLONE_THINPACK"), "") o := runCLI() require.False(t, o.GitCloneSingleBranch) + require.False(t, o.GitCloneThinPack) }) }) } diff --git a/options/testdata/options.golden b/options/testdata/options.golden index 6a8145ad..d20961aa 100644 --- a/options/testdata/options.golden +++ b/options/testdata/options.golden @@ -99,6 +99,9 @@ OPTIONS: --git-clone-single-branch bool, $ENVBUILDER_GIT_CLONE_SINGLE_BRANCH Clone only a single branch of the Git repository. + --git-clone-thinpack bool, $ENVBUILDER_GIT_CLONE_THINPACK + Clone with thin pack compabilities. + --git-http-proxy-url string, $ENVBUILDER_GIT_HTTP_PROXY_URL The URL for the HTTP proxy. This is optional. From e4fb15a390bc10f9cc6fabfe111d0e88cc7f4144 Mon Sep 17 00:00:00 2001 From: CH-Chang Date: Thu, 13 Mar 2025 04:25:09 +0000 Subject: [PATCH 2/6] feat: adjust thinpack logging and option definition --- git/git.go | 50 ++++++++++++++++++++------------- options/options.go | 3 +- options/options_test.go | 2 +- options/testdata/options.golden | 2 +- 4 files changed, 35 insertions(+), 22 deletions(-) diff --git a/git/git.go b/git/git.go index e3279b55..90968470 100644 --- a/git/git.go +++ b/git/git.go @@ -54,28 +54,40 @@ func CloneRepo(ctx context.Context, logf func(string, ...any), opts CloneRepoOpt return false, fmt.Errorf("parse url %q: %w", opts.RepoURL, err) } logf("Parsed Git URL as %q", parsed.Redacted()) - if parsed.Hostname() == "dev.azure.com" || opts.ThinPack { - // Azure DevOps requires capabilities multi_ack / multi_ack_detailed, - // which are not fully implemented and by default are included in - // transport.UnsupportedCapabilities. - // - // The initial clone operations require a full download of the repository, - // and therefore those unsupported capabilities are not as crucial, so - // by removing them from that list allows for the first clone to work - // successfully. - // - // Additional fetches will yield issues, therefore work always from a clean - // clone until those capabilities are fully supported. - // - // New commits and pushes against a remote worked without any issues. - // See: https://github.com/go-git/go-git/issues/64 - // - // This is knowingly not safe to call in parallel, but it seemed - // like the least-janky place to add a super janky hack. + + thinPack := true + + if !opts.ThinPack { + thinPack = false + logf("ThinPack options is false, Marking thin-pack as unsupported") + } else { + if parsed.Hostname() == "dev.azure.com" { + // Azure DevOps requires capabilities multi_ack / multi_ack_detailed, + // which are not fully implemented and by default are included in + // transport.UnsupportedCapabilities. + // + // The initial clone operations require a full download of the repository, + // and therefore those unsupported capabilities are not as crucial, so + // by removing them from that list allows for the first clone to work + // successfully. + // + // Additional fetches will yield issues, therefore work always from a clean + // clone until those capabilities are fully supported. + // + // New commits and pushes against a remote worked without any issues. + // See: https://github.com/go-git/go-git/issues/64 + // + // This is knowingly not safe to call in parallel, but it seemed + // like the least-janky place to add a super janky hack. + thinPack = false + logf("Workaround for Azure DevOps: marking thin-pack as unsupported") + } + } + + if !thinPack { transport.UnsupportedCapabilities = []capability.Capability{ capability.ThinPack, } - logf("Workaround for Azure DevOps: marking thin-pack as unsupported") } err = opts.Storage.MkdirAll(opts.Path, 0o755) diff --git a/options/options.go b/options/options.go index ecac6477..77d7dec9 100644 --- a/options/options.go +++ b/options/options.go @@ -381,7 +381,8 @@ func (o *Options) CLI() serpent.OptionSet { Flag: "git-clone-thinpack", Env: WithEnvPrefix("GIT_CLONE_THINPACK"), Value: serpent.BoolOf(&o.GitCloneThinPack), - Description: "Clone with thin pack compabilities.", + Default: "true", + Description: "Git clone with thin pack compatibility enable.", }, { Flag: "git-username", diff --git a/options/options_test.go b/options/options_test.go index 1a92c8b9..ed5dcd3c 100644 --- a/options/options_test.go +++ b/options/options_test.go @@ -70,7 +70,7 @@ func TestEnvOptionParsing(t *testing.T) { t.Setenv(options.WithEnvPrefix("GIT_CLONE_THINPACK"), "") o := runCLI() require.False(t, o.GitCloneSingleBranch) - require.False(t, o.GitCloneThinPack) + require.True(t, o.GitCloneThinPack) }) }) } diff --git a/options/testdata/options.golden b/options/testdata/options.golden index d20961aa..87f5ff9a 100644 --- a/options/testdata/options.golden +++ b/options/testdata/options.golden @@ -100,7 +100,7 @@ OPTIONS: Clone only a single branch of the Git repository. --git-clone-thinpack bool, $ENVBUILDER_GIT_CLONE_THINPACK - Clone with thin pack compabilities. + Git clone with thin pack compatibility enable. --git-http-proxy-url string, $ENVBUILDER_GIT_HTTP_PROXY_URL The URL for the HTTP proxy. This is optional. From 46530100b3f839ae6d85561c4fb5dc7266889706 Mon Sep 17 00:00:00 2001 From: CH-Chang Date: Thu, 13 Mar 2025 04:28:43 +0000 Subject: [PATCH 3/6] docs: update git clone thin pack option description --- docs/env-variables.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/env-variables.md b/docs/env-variables.md index a015883d..df1691e8 100644 --- a/docs/env-variables.md +++ b/docs/env-variables.md @@ -27,7 +27,7 @@ | `--git-url` | `ENVBUILDER_GIT_URL` | | The URL of a Git repository containing a Devcontainer or Docker image to clone. This is optional. | | `--git-clone-depth` | `ENVBUILDER_GIT_CLONE_DEPTH` | | The depth to use when cloning the Git repository. | | `--git-clone-single-branch` | `ENVBUILDER_GIT_CLONE_SINGLE_BRANCH` | | Clone only a single branch of the Git repository. | -| `--git-clone-thinpack` | `ENVBUILDER_GIT_CLONE_THINPACK` | | Clone with thin pack compabilities. | +| `--git-clone-thinpack` | `ENVBUILDER_GIT_CLONE_THINPACK` | | Git clone with thin pack compatibility enabled, ensuring that even when thin pack compatibility is activated, it will not be turned on for the domain dev.zaure.com. | | `--git-username` | `ENVBUILDER_GIT_USERNAME` | | The username to use for Git authentication. This is optional. | | `--git-password` | `ENVBUILDER_GIT_PASSWORD` | | The password to use for Git authentication. This is optional. | | `--git-ssh-private-key-path` | `ENVBUILDER_GIT_SSH_PRIVATE_KEY_PATH` | | Path to an SSH private key to be used for Git authentication. If this is set, then GIT_SSH_PRIVATE_KEY_BASE64 cannot be set. | From 98985a208f537fff4d61a9d724b3fa5639c0e0aa Mon Sep 17 00:00:00 2001 From: CH-Chang Date: Thu, 13 Mar 2025 04:37:31 +0000 Subject: [PATCH 4/6] feat: sync command line option description with docs --- options/options.go | 12 +++++++----- options/testdata/options.golden | 6 ++++-- 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/options/options.go b/options/options.go index 77d7dec9..8cdf723a 100644 --- a/options/options.go +++ b/options/options.go @@ -378,11 +378,13 @@ func (o *Options) CLI() serpent.OptionSet { Description: "Clone only a single branch of the Git repository.", }, { - Flag: "git-clone-thinpack", - Env: WithEnvPrefix("GIT_CLONE_THINPACK"), - Value: serpent.BoolOf(&o.GitCloneThinPack), - Default: "true", - Description: "Git clone with thin pack compatibility enable.", + Flag: "git-clone-thinpack", + Env: WithEnvPrefix("GIT_CLONE_THINPACK"), + Value: serpent.BoolOf(&o.GitCloneThinPack), + Default: "true", + Description: "Git clone with thin pack compatibility enabled, " + + "ensuring that even when thin pack compatibility is activated," + + "it will not be turned on for the domain dev.zaure.com.", }, { Flag: "git-username", diff --git a/options/testdata/options.golden b/options/testdata/options.golden index 87f5ff9a..92a85232 100644 --- a/options/testdata/options.golden +++ b/options/testdata/options.golden @@ -99,8 +99,10 @@ OPTIONS: --git-clone-single-branch bool, $ENVBUILDER_GIT_CLONE_SINGLE_BRANCH Clone only a single branch of the Git repository. - --git-clone-thinpack bool, $ENVBUILDER_GIT_CLONE_THINPACK - Git clone with thin pack compatibility enable. + --git-clone-thinpack bool, $ENVBUILDER_GIT_CLONE_THINPACK (default: true) + Git clone with thin pack compatibility enabled, ensuring that even + when thin pack compatibility is activated,it will not be turned on for + the domain dev.zaure.com. --git-http-proxy-url string, $ENVBUILDER_GIT_HTTP_PROXY_URL The URL for the HTTP proxy. This is optional. From cbd387e3b6a4c721220ca4824af27d0c9be5b899 Mon Sep 17 00:00:00 2001 From: CH-Chang Date: Thu, 13 Mar 2025 10:23:23 +0000 Subject: [PATCH 5/6] docs: update docs with make docs/env-variables.md --- docs/env-variables.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/env-variables.md b/docs/env-variables.md index df1691e8..e6fa7ca5 100644 --- a/docs/env-variables.md +++ b/docs/env-variables.md @@ -27,7 +27,7 @@ | `--git-url` | `ENVBUILDER_GIT_URL` | | The URL of a Git repository containing a Devcontainer or Docker image to clone. This is optional. | | `--git-clone-depth` | `ENVBUILDER_GIT_CLONE_DEPTH` | | The depth to use when cloning the Git repository. | | `--git-clone-single-branch` | `ENVBUILDER_GIT_CLONE_SINGLE_BRANCH` | | Clone only a single branch of the Git repository. | -| `--git-clone-thinpack` | `ENVBUILDER_GIT_CLONE_THINPACK` | | Git clone with thin pack compatibility enabled, ensuring that even when thin pack compatibility is activated, it will not be turned on for the domain dev.zaure.com. | +| `--git-clone-thinpack` | `ENVBUILDER_GIT_CLONE_THINPACK` | `true` | Git clone with thin pack compatibility enabled, ensuring that even when thin pack compatibility is activated,it will not be turned on for the domain dev.zaure.com. | | `--git-username` | `ENVBUILDER_GIT_USERNAME` | | The username to use for Git authentication. This is optional. | | `--git-password` | `ENVBUILDER_GIT_PASSWORD` | | The password to use for Git authentication. This is optional. | | `--git-ssh-private-key-path` | `ENVBUILDER_GIT_SSH_PRIVATE_KEY_PATH` | | Path to an SSH private key to be used for Git authentication. If this is set, then GIT_SSH_PRIVATE_KEY_BASE64 cannot be set. | From af75d61df2a9f8d1ab270d55603fc8eb45710fbe Mon Sep 17 00:00:00 2001 From: CH-Chang Date: Thu, 13 Mar 2025 10:25:51 +0000 Subject: [PATCH 6/6] refactor: adjust if statement --- git/git.go | 42 ++++++++++++++++++++---------------------- 1 file changed, 20 insertions(+), 22 deletions(-) diff --git a/git/git.go b/git/git.go index 90968470..efcffa91 100644 --- a/git/git.go +++ b/git/git.go @@ -60,28 +60,26 @@ func CloneRepo(ctx context.Context, logf func(string, ...any), opts CloneRepoOpt if !opts.ThinPack { thinPack = false logf("ThinPack options is false, Marking thin-pack as unsupported") - } else { - if parsed.Hostname() == "dev.azure.com" { - // Azure DevOps requires capabilities multi_ack / multi_ack_detailed, - // which are not fully implemented and by default are included in - // transport.UnsupportedCapabilities. - // - // The initial clone operations require a full download of the repository, - // and therefore those unsupported capabilities are not as crucial, so - // by removing them from that list allows for the first clone to work - // successfully. - // - // Additional fetches will yield issues, therefore work always from a clean - // clone until those capabilities are fully supported. - // - // New commits and pushes against a remote worked without any issues. - // See: https://github.com/go-git/go-git/issues/64 - // - // This is knowingly not safe to call in parallel, but it seemed - // like the least-janky place to add a super janky hack. - thinPack = false - logf("Workaround for Azure DevOps: marking thin-pack as unsupported") - } + } else if parsed.Hostname() == "dev.azure.com" { + // Azure DevOps requires capabilities multi_ack / multi_ack_detailed, + // which are not fully implemented and by default are included in + // transport.UnsupportedCapabilities. + // + // The initial clone operations require a full download of the repository, + // and therefore those unsupported capabilities are not as crucial, so + // by removing them from that list allows for the first clone to work + // successfully. + // + // Additional fetches will yield issues, therefore work always from a clean + // clone until those capabilities are fully supported. + // + // New commits and pushes against a remote worked without any issues. + // See: https://github.com/go-git/go-git/issues/64 + // + // This is knowingly not safe to call in parallel, but it seemed + // like the least-janky place to add a super janky hack. + thinPack = false + logf("Workaround for Azure DevOps: marking thin-pack as unsupported") } if !thinPack {