Skip to content

feat: add git clone with thinpack option to support self hosted azure devops #446

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 7 commits into from
Mar 13, 2025
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions docs/env-variables.md
Original file line number Diff line number Diff line change
Expand Up @@ -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. |
Copy link
Member

Choose a reason for hiding this comment

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

Can you add a note regarding the behaviour of thinpack here for repos hosted on dev.zaure.com?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have added the description regarding the behavior of thinpack for repositories hosted on dev.zaure.com, where the option remains ineffective even when enabled. Thank you!

| `--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. |
Expand Down
4 changes: 3 additions & 1 deletion git/git.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ type CloneRepoOptions struct {
Progress sideband.Progress
Insecure bool
SingleBranch bool
ThinPack bool
Depth int
CABundle []byte
ProxyOptions transport.ProxyOptions
Expand All @@ -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 {
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for this! Would you mind updating the logf message below? It still states that this is for azure specifically, which could be misleading if ThinPack were set in non Azure environments.

Copy link
Member

Choose a reason for hiding this comment

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

I think the semantics of the option as currently defined are also somewhat misleading. Setting ENVBUILDER_GIT_CLONE_THINPACK=true will add thinpack to the unsupported capabilities of the Git transport when attempting to clone. Shouldn't it be the other way round?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for this! Would you mind updating the logf message below? It still states that this is for azure specifically, which could be misleading if ThinPack were set in non Azure environments.

I've updated the log message to differentiate between disabling the feature via the command line and disabling it due to encountering the corresponding domain.

I think the semantics of the option as currently defined are also somewhat misleading. Setting ENVBUILDER_GIT_CLONE_THINPACK=true will add thinpack to the unsupported capabilities of the Git transport when attempting to clone. Shouldn't it be the other way round?

I've adjusted the definition of the option. By default, the option is set to true, which enables Git clone thin pack compatibility.

// Azure DevOps requires capabilities multi_ack / multi_ack_detailed,
// which are not fully implemented and by default are included in
// transport.UnsupportedCapabilities.
Expand Down Expand Up @@ -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,
}
Expand Down
8 changes: 8 additions & 0 deletions options/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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"),
Expand Down
8 changes: 8 additions & 0 deletions options/options_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})
})
}
Expand Down
3 changes: 3 additions & 0 deletions options/testdata/options.golden
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand Down