From e74f17fd27abd2a1af0689be3cbf6f48c8506137 Mon Sep 17 00:00:00 2001 From: CrazyMax <1951866+crazy-max@users.noreply.github.com> Date: Wed, 29 Jul 2026 10:33:40 +0200 Subject: [PATCH] source/git: suppress git advice by default Signed-off-by: CrazyMax <1951866+crazy-max@users.noreply.github.com> --- client/llb/git_test.go | 21 +++ client/llb/source.go | 13 ++ frontend/dockerfile/dockerfile2llb/convert.go | 2 + .../dockerfile/dockerfile2llb/convert_copy.go | 3 + .../dockerfile/dockerfile2llb/convert_test.go | 50 +++++- frontend/dockerfile/dockerfile2llb/epoch.go | 11 +- frontend/dockerfile/dockerfile_addgit_test.go | 147 ++++++++++++++++++ frontend/dockerfile/docs/reference.md | 1 + frontend/dockerui/config.go | 8 + frontend/dockerui/context.go | 8 + frontend/dockerui/namedcontext.go | 4 +- solver/pb/attr.go | 1 + source/git/bundle.go | 8 +- source/git/identifier.go | 1 + source/git/identifier_test.go | 10 ++ source/git/source.go | 8 +- source/git/source_test.go | 28 +++- util/gitutil/git_cli.go | 18 +++ util/gitutil/git_cli_test.go | 34 ++++ 19 files changed, 361 insertions(+), 15 deletions(-) diff --git a/client/llb/git_test.go b/client/llb/git_test.go index 666d3b7fbc1f..219b87535afa 100644 --- a/client/llb/git_test.go +++ b/client/llb/git_test.go @@ -68,6 +68,27 @@ func TestGit(t *testing.T) { "git.fullurl": "https://github.com/foo/bar.git", }, }, + { + name: "git advice", + st: Git("github.com/foo/bar.git", "ref", GitAdvice(true)), + identifier: "git://github.com/foo/bar.git#ref", + attrs: map[string]string{ + "git.authheadersecret": "GIT_AUTH_HEADER", + "git.authtokensecret": "GIT_AUTH_TOKEN", + "git.fullurl": "https://github.com/foo/bar.git", + "git.advice": "true", + }, + }, + { + name: "git advice disabled", + st: Git("github.com/foo/bar.git", "ref", GitAdvice(false)), + identifier: "git://github.com/foo/bar.git#ref", + attrs: map[string]string{ + "git.authheadersecret": "GIT_AUTH_HEADER", + "git.authtokensecret": "GIT_AUTH_TOKEN", + "git.fullurl": "https://github.com/foo/bar.git", + }, + }, { name: "bundle", st: Git( diff --git a/client/llb/source.go b/client/llb/source.go index dab72baae457..cf79fa56687a 100644 --- a/client/llb/source.go +++ b/client/llb/source.go @@ -483,6 +483,10 @@ func Git(url, fragment string, opts ...GitOption) State { addCap(&gi.Constraints, pb.CapSourceGitMTime) } + if gi.Advice { + attrs[pb.AttrGitAdvice] = "true" + } + if gi.FetchByCommit { attrs[pb.AttrGitFetchByCommit] = "true" addCap(&gi.Constraints, pb.CapSourceGitFetchByCommit) @@ -531,6 +535,7 @@ type GitInfo struct { SubDir string SkipSubmodules bool MTime string + Advice bool Bundle string BundleOCISessionID string BundleOCIStoreID string @@ -570,6 +575,14 @@ func GitMTime(v string) GitOption { }) } +// GitAdvice controls whether Git advice messages are emitted while resolving +// this git source. +func GitAdvice(enabled bool) GitOption { + return gitOptionFunc(func(gi *GitInfo) { + gi.Advice = enabled + }) +} + func KeepGitDir() GitOption { return gitOptionFunc(func(gi *GitInfo) { gi.KeepGitDir = true diff --git a/frontend/dockerfile/dockerfile2llb/convert.go b/frontend/dockerfile/dockerfile2llb/convert.go index 4a4aa7eec448..1fe35af81dc7 100644 --- a/frontend/dockerfile/dockerfile2llb/convert.go +++ b/frontend/dockerfile/dockerfile2llb/convert.go @@ -833,6 +833,7 @@ func (dctx *dispatchContext) dispatchStages(ctx context.Context, allReachable ma buildPlatforms: dctx.platformOpt.buildPlatforms, targetPlatform: dctx.platformOpt.targetPlatform, extraHosts: dctx.opt.ExtraHosts, + gitAdvice: dctx.opt.GitAdvice, shmSize: dctx.opt.ShmSize, ulimit: dctx.opt.Ulimits, devices: dctx.opt.Devices, @@ -989,6 +990,7 @@ type dispatchOpt struct { targetPlatform ocispecs.Platform buildPlatforms []ocispecs.Platform extraHosts []llb.HostIP + gitAdvice bool shmSize int64 ulimit []*pb.Ulimit devices []*pb.CDIDevice diff --git a/frontend/dockerfile/dockerfile2llb/convert_copy.go b/frontend/dockerfile/dockerfile2llb/convert_copy.go index 75d5913d347c..9e3a0ff4f529 100644 --- a/frontend/dockerfile/dockerfile2llb/convert_copy.go +++ b/frontend/dockerfile/dockerfile2llb/convert_copy.go @@ -139,6 +139,9 @@ func dispatchCopy(d *dispatchState, cfg copyConfig) error { llb.WithCustomName(pgName), llb.GitRef(gitRef.Ref), } + if cfg.opt.gitAdvice { + gitOptions = append(gitOptions, llb.GitAdvice(true)) + } if cfg.keepGitDir != nil && gitRef.KeepGitDir != nil { if *cfg.keepGitDir != *gitRef.KeepGitDir { return errors.New("inconsistent keep-git-dir configuration") diff --git a/frontend/dockerfile/dockerfile2llb/convert_test.go b/frontend/dockerfile/dockerfile2llb/convert_test.go index 2d6d5471c51a..154786263839 100644 --- a/frontend/dockerfile/dockerfile2llb/convert_test.go +++ b/frontend/dockerfile/dockerfile2llb/convert_test.go @@ -12,6 +12,7 @@ import ( "github.com/moby/buildkit/frontend/dockerfile/parser" "github.com/moby/buildkit/frontend/dockerfile/shell" "github.com/moby/buildkit/frontend/dockerui" + "github.com/moby/buildkit/solver/pb" "github.com/moby/buildkit/util/appcontext" dockerspec "github.com/moby/docker-image-spec/specs-go/v1" digest "github.com/opencontainers/go-digest" @@ -355,7 +356,7 @@ ADD $URL / require.NoError(t, err) require.Len(t, stages, 1) - state, err := sourceDateEpochStageSource(stages[0], nil, &llb.EnvList{}, shell.NewLex('\\')) + state, err := sourceDateEpochStageSource(stages[0], nil, &llb.EnvList{}, shell.NewLex('\\'), false) require.NoError(t, err) require.NotNil(t, state) sourceOp, err := sourceOpFromState(t.Context(), state) @@ -364,6 +365,51 @@ ADD $URL / assert.Equal(t, "src.tar", sourceOp.Attrs["http.filename"]) } +func TestDockerfileGitAdviceBuildArgADD(t *testing.T) { + t.Parallel() + + df := []byte(` +FROM scratch +ADD https://github.com/moby/buildkit.git#master / +`) + + for _, tc := range []struct { + name string + gitAdvice bool + wantAttr bool + }{ + { + name: "default", + }, + { + name: "enabled", + gitAdvice: true, + wantAttr: true, + }, + } { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + res, err := Dockerfile2LLB(appcontext.Context(), df, ConvertOpt{ + Config: dockerui.Config{ + GitAdvice: tc.gitAdvice, + }, + }) + require.NoError(t, err) + + sourceOp, err := sourceOpFromState(t.Context(), &res.State) + require.NoError(t, err) + require.NotNil(t, sourceOp) + + if tc.wantAttr { + require.Equal(t, "true", sourceOp.Attrs[pb.AttrGitAdvice]) + } else { + require.NotContains(t, sourceOp.Attrs, pb.AttrGitAdvice) + } + }) + } +} + func TestSourceDateEpochStageSourceRequiresScratch(t *testing.T) { t.Parallel() @@ -379,7 +425,7 @@ ADD https://example.com/src.tar / require.NoError(t, err) require.Len(t, stages, 1) - _, err = sourceDateEpochStageSource(stages[0], nil, &llb.EnvList{}, shell.NewLex('\\')) + _, err = sourceDateEpochStageSource(stages[0], nil, &llb.EnvList{}, shell.NewLex('\\'), false) require.ErrorContains(t, err, "SOURCE_DATE_EPOCH stage must use FROM scratch") } diff --git a/frontend/dockerfile/dockerfile2llb/epoch.go b/frontend/dockerfile/dockerfile2llb/epoch.go index 03e7d667d310..58a252af21ce 100644 --- a/frontend/dockerfile/dockerfile2llb/epoch.go +++ b/frontend/dockerfile/dockerfile2llb/epoch.go @@ -98,7 +98,7 @@ func resolveSourceDateEpochState(ctx context.Context, value string, opt ConvertO args = &updated } - sourceState, err := sourceDateEpochStageSource(stages[i], opt.BuildArgs, args, shlex) + sourceState, err := sourceDateEpochStageSource(stages[i], opt.BuildArgs, args, shlex, opt.GitAdvice) if err != nil { return nil, sourceDateEpochStateOpt{}, parser.WithLocation(err, stages[i].Location) } @@ -110,7 +110,7 @@ func resolveSourceDateEpochState(ctx context.Context, value string, opt ConvertO return nil, sourceDateEpochStateOpt{}, errors.Errorf("invalid SOURCE_DATE_EPOCH: %s", value) } -func sourceDateEpochStageSource(stage instructions.Stage, buildArgs map[string]string, globalArgs *llb.EnvList, shlex *shell.Lex) (*llb.State, error) { +func sourceDateEpochStageSource(stage instructions.Stage, buildArgs map[string]string, globalArgs *llb.EnvList, shlex *shell.Lex, gitAdvice bool) (*llb.State, error) { stageBaseName, _, err := shlex.ProcessWord(stage.BaseName, globalArgs) if err != nil { return nil, errors.Wrapf(err, "failed to process source stage base name %q", stage.BaseName) @@ -133,7 +133,7 @@ func sourceDateEpochStageSource(stage instructions.Stage, buildArgs map[string]s if sourceState != nil { return nil, errors.New("SOURCE_DATE_EPOCH stage must contain exactly one remote ADD") } - sourceState, err = sourceDateEpochAddSource(c, env, shlex) + sourceState, err = sourceDateEpochAddSource(c, env, shlex, gitAdvice) if err != nil { return nil, err } @@ -167,7 +167,7 @@ func applySourceDateEpochStageArgs(args []instructions.KeyValuePairOptional, env return env, nil } -func sourceDateEpochAddSource(cmd *instructions.AddCommand, env *llb.EnvList, shlex *shell.Lex) (*llb.State, error) { +func sourceDateEpochAddSource(cmd *instructions.AddCommand, env *llb.EnvList, shlex *shell.Lex, gitAdvice bool) (*llb.State, error) { if len(cmd.SourceContents) != 0 || len(cmd.SourcePaths) != 1 { return nil, errors.New("SOURCE_DATE_EPOCH stage must contain exactly one remote ADD source") } @@ -201,6 +201,9 @@ func sourceDateEpochAddSource(cmd *instructions.AddCommand, env *llb.EnvList, sh gitOptions := []llb.GitOption{ llb.GitRef(gitRef.Ref), } + if gitAdvice { + gitOptions = append(gitOptions, llb.GitAdvice(true)) + } if cmd.KeepGitDir != nil && *cmd.KeepGitDir { gitOptions = append(gitOptions, llb.KeepGitDir()) } diff --git a/frontend/dockerfile/dockerfile_addgit_test.go b/frontend/dockerfile/dockerfile_addgit_test.go index 40db6893ab86..ea035dd21fae 100644 --- a/frontend/dockerfile/dockerfile_addgit_test.go +++ b/frontend/dockerfile/dockerfile_addgit_test.go @@ -25,6 +25,7 @@ var addGitTests = integration.TestFuncs( testAddGitSHA256, testAddGitChecksumCache, testGitQueryString, + testGitAdviceBuildArg, ) func init() { @@ -716,6 +717,152 @@ FROM main } } +func testGitAdviceBuildArg(t *testing.T, sb integration.Sandbox) { + integration.SkipOnPlatform(t, "windows", "Git source handler submodule update not supported on Windows") + f := getFrontend(t, sb) + + c, err := client.New(sb.Context(), sb.Address()) + require.NoError(t, err) + defer c.Close() + + const detachedHeadAdvice = "detached HEAD" + + for _, tc := range []struct { + name string + buildArg string + wantAdvice bool + }{ + { + name: "default", + }, + { + name: "enabled", + buildArg: "1", + wantAdvice: true, + }, + } { + t.Run("context_"+tc.name, func(t *testing.T) { + serverURL, closeServer := newGitAdviceHTTPRepo(t, map[string]string{ + "Dockerfile": "FROM scratch\nCOPY .git/HEAD /head\n", + "unique": "context " + tc.name, + }) + defer closeServer() + + dest := t.TempDir() + attrs := map[string]string{ + "context": serverURL + "/.git?tag=v0.0.1&keep-git-dir=true", + } + if tc.buildArg != "" { + attrs["build-arg:BUILDKIT_GIT_ADVICE"] = tc.buildArg + } + logs := solveWithGitAdviceLogs(t, sb, f, c, client.SolveOpt{ + FrontendAttrs: attrs, + Exports: []client.ExportEntry{ + { + Type: client.ExporterLocal, + OutputDir: dest, + }, + }, + }) + + _, err := os.ReadFile(filepath.Join(dest, "head")) + require.NoError(t, err) + if tc.wantAdvice { + require.Contains(t, logs, detachedHeadAdvice) + } else { + require.NotContains(t, logs, detachedHeadAdvice) + } + }) + + t.Run("add_"+tc.name, func(t *testing.T) { + serverURL, closeServer := newGitAdviceHTTPRepo(t, map[string]string{ + "foo": "bar\n", + "unique": "add " + tc.name, + }) + defer closeServer() + + dockerfile := fmt.Appendf(nil, "FROM scratch\nADD --keep-git-dir=true %s/.git#v0.0.1 /repo\n", serverURL) + dir := integration.Tmpdir(t, + fstest.CreateFile("Dockerfile", dockerfile, 0600), + ) + + dest := t.TempDir() + attrs := map[string]string{} + if tc.buildArg != "" { + attrs["build-arg:BUILDKIT_GIT_ADVICE"] = tc.buildArg + } + logs := solveWithGitAdviceLogs(t, sb, f, c, client.SolveOpt{ + FrontendAttrs: attrs, + Exports: []client.ExportEntry{ + { + Type: client.ExporterLocal, + OutputDir: dest, + }, + }, + LocalMounts: map[string]fsutil.FS{ + dockerui.DefaultLocalNameDockerfile: dir, + dockerui.DefaultLocalNameContext: dir, + }, + }) + + dt, err := os.ReadFile(filepath.Join(dest, "repo", "foo")) + require.NoError(t, err) + require.Equal(t, "bar\n", string(dt)) + if tc.wantAdvice { + require.Contains(t, logs, detachedHeadAdvice) + } else { + require.NotContains(t, logs, detachedHeadAdvice) + } + }) + } +} + +func newGitAdviceHTTPRepo(t *testing.T, files map[string]string) (string, func()) { + t.Helper() + + gitDir := t.TempDir() + for name, data := range files { + p := filepath.Join(gitDir, filepath.FromSlash(name)) + require.NoError(t, os.MkdirAll(filepath.Dir(p), 0700)) + require.NoError(t, os.WriteFile(p, []byte(data), 0600)) + } + + err := runShell(gitDir, + "git init", + "git config --local user.email test", + "git config --local user.name test", + "git add .", + "git commit -m initial", + "git tag v0.0.1", + "git update-server-info", + ) + require.NoError(t, err) + + server := httptest.NewServer(http.FileServer(http.Dir(filepath.Clean(gitDir)))) + return server.URL, server.Close +} + +func solveWithGitAdviceLogs(t *testing.T, sb integration.Sandbox, f frontend, c *client.Client, opt client.SolveOpt) string { + t.Helper() + + statusCh := make(chan *client.SolveStatus) + logsCh := make(chan string, 1) + go func() { + var logs bytes.Buffer + for status := range statusCh { + for _, l := range status.Logs { + logs.Write(l.Data) + } + } + logsCh <- logs.String() + }() + + _, err := f.Solve(sb.Context(), c, opt, statusCh) + logs := <-logsCh + require.NoError(t, err) + return logs +} + func applyTemplate(tmpl string, x any) (string, error) { var buf bytes.Buffer parsed, err := template.New("").Parse(tmpl) diff --git a/frontend/dockerfile/docs/reference.md b/frontend/dockerfile/docs/reference.md index 5a366a1b90e9..507d368ff6c5 100644 --- a/frontend/dockerfile/docs/reference.md +++ b/frontend/dockerfile/docs/reference.md @@ -2733,6 +2733,7 @@ RUN echo "I'm building for $TARGETPLATFORM" | `BUILDKIT_BUILD_NAME` | String | Override the build name shown in [`buildx history` command](https://docs.docker.com/reference/cli/docker/buildx/history/) and [Docker Desktop Builds view](https://docs.docker.com/desktop/use-desktop/builds/). | | `BUILDKIT_CACHE_MOUNT_NS` | String | Set optional cache ID namespace. | | `BUILDKIT_CONTEXT_KEEP_GIT_DIR` | Bool | Trigger Git context to keep the `.git` directory. | +| `BUILDKIT_GIT_ADVICE` | Bool | Show Git advice messages from BuildKit-managed Git operations. Defaults to `false`. | | `BUILDKIT_INLINE_CACHE`[^2] | Bool | Inline cache metadata to image config or not. | | `BUILDKIT_MULTI_PLATFORM` | Bool | Opt into deterministic output regardless of multi-platform output or not. | | `BUILDKIT_SANDBOX_HOSTNAME` | String | Set the hostname (default `buildkitsandbox`) | diff --git a/frontend/dockerui/config.go b/frontend/dockerui/config.go index a9c6cd3f15f3..fed608f4574e 100644 --- a/frontend/dockerui/config.go +++ b/frontend/dockerui/config.go @@ -57,6 +57,7 @@ const ( keyHostnameArg = "build-arg:BUILDKIT_SANDBOX_HOSTNAME" keyDockerfileLintArg = "build-arg:BUILDKIT_DOCKERFILE_CHECK" keyContextKeepGitDirArg = "build-arg:BUILDKIT_CONTEXT_KEEP_GIT_DIR" + keyGitAdviceArg = "build-arg:BUILDKIT_GIT_ADVICE" ) type Config struct { @@ -68,6 +69,7 @@ type Config struct { ImageResolveMode llb.ResolveMode Labels map[string]string NetworkMode pb.NetMode + GitAdvice bool ShmSize int64 Target string Ulimits []*pb.Ulimit @@ -311,6 +313,12 @@ func (bc *Client) init() error { return errors.Wrapf(err, "failed to parse %s", keyDockerfileLintArg) } } + if v := opts[keyGitAdviceArg]; v != "" { + bc.GitAdvice, err = strconv.ParseBool(v) + if err != nil { + return errors.Wrapf(err, "failed to parse %s", keyGitAdviceArg) + } + } bc.localsSessionIDs = parseLocalSessionIDs(opts) diff --git a/frontend/dockerui/context.go b/frontend/dockerui/context.go index 6fb3ebd1be60..3f715d80021b 100644 --- a/frontend/dockerui/context.go +++ b/frontend/dockerui/context.go @@ -90,6 +90,7 @@ func (bc *Client) initContext(ctx context.Context) (*buildContext, error) { if opts[buildArgPrefix+"SOURCE_DATE_EPOCH"] != "" { extraGitOpts = append(extraGitOpts, llb.GitMTimeCommit()) } + extraGitOpts = append(extraGitOpts, gitAdviceOpts(bc.GitAdvice)...) if st, ok, err := DetectGitContext(opts[localNameContext], keepGit, extraGitOpts...); ok { if err != nil { return nil, err @@ -325,6 +326,13 @@ func DetectGitContext(ref string, keepGit *bool, opts ...llb.GitOption) (*llb.St return &st, true, nil } +func gitAdviceOpts(enabled bool) []llb.GitOption { + if enabled { + return []llb.GitOption{llb.GitAdvice(true)} + } + return nil +} + func DetectHTTPContext(ref string) (*llb.State, string, bool) { filename := "context" if httpPrefix.MatchString(ref) { diff --git a/frontend/dockerui/namedcontext.go b/frontend/dockerui/namedcontext.go index 06e396e05ef7..1d27ddcf488e 100644 --- a/frontend/dockerui/namedcontext.go +++ b/frontend/dockerui/namedcontext.go @@ -141,7 +141,7 @@ func (nc *NamedContext) load(ctx context.Context, count int) (*llb.State, *docke } return &st, &img, nil case "git": - st, ok, err := DetectGitContext(nc.input, nil) + st, ok, err := DetectGitContext(nc.input, nil, gitAdviceOpts(nc.bc.GitAdvice)...) if !ok { return nil, nil, errors.Errorf("invalid git context %s", nc.input) } @@ -150,7 +150,7 @@ func (nc *NamedContext) load(ctx context.Context, count int) (*llb.State, *docke } return st, nil, nil case "http", "https": - st, ok, err := DetectGitContext(nc.input, nil) + st, ok, err := DetectGitContext(nc.input, nil, gitAdviceOpts(nc.bc.GitAdvice)...) if ok { if err != nil { return nil, nil, err diff --git a/solver/pb/attr.go b/solver/pb/attr.go index 4089fce2c412..7a0a68d4ab78 100644 --- a/solver/pb/attr.go +++ b/solver/pb/attr.go @@ -12,6 +12,7 @@ const AttrGitMTime = "git.mtime" const AttrGitFetchByCommit = "git.fetchbycommit" const AttrGitBundle = "git.bundle" const AttrGitCheckoutBundle = "git.checkoutbundle" +const AttrGitAdvice = "git.advice" const AttrGitSignatureVerifyPubKey = "git.sig.pubkey" const AttrGitSignatureVerifyRejectExpired = "git.sig.rejectexpired" diff --git a/source/git/bundle.go b/source/git/bundle.go index 374008d2147c..ce39e455d8d5 100644 --- a/source/git/bundle.go +++ b/source/git/bundle.go @@ -71,8 +71,8 @@ func bundleTargetRef(ref string) string { // sha256 object IDs. git ls-remote reads bundle files directly, so this works // before a destination repo exists and lets callers initialize the right object // format for a subsequent fetch/import. -func detectBundleSHA256(ctx context.Context, bundlePath string) (bool, error) { - buf, err := gitCLI().Run(ctx, "ls-remote", "--", bundlePath) +func detectBundleSHA256(ctx context.Context, bundlePath string, gitAdvice bool) (bool, error) { + buf, err := gitCLI(gitutil.WithGitAdvice(gitAdvice)).Run(ctx, "ls-remote", "--", bundlePath) if err != nil { return false, errors.Wrapf(err, "failed to inspect git bundle %s", bundlePath) } @@ -119,7 +119,7 @@ func (gs *gitSourceHandler) stageBundle(ctx context.Context, g session.Group) (_ return "", nil, err } bundlePath := filepath.Join(tmpDir, bundleImportFileName) - sha256, err := detectBundleSHA256(ctx, bundlePath) + sha256, err := detectBundleSHA256(ctx, bundlePath, gs.src.Advice) if err != nil { return "", nil, err } @@ -132,7 +132,7 @@ func (gs *gitSourceHandler) stageBundle(ctx context.Context, g session.Group) (_ if err := os.Mkdir(tmpRepoDir, 0700); err != nil { return "", nil, errors.Wrap(err, "failed to create temp bare repo dir") } - tmpGit := gitCLI(gitutil.WithGitDir(tmpRepoDir)) + tmpGit := gitCLI(gitutil.WithGitAdvice(gs.src.Advice), gitutil.WithGitDir(tmpRepoDir)) initArgs := []string{"-c", "init.defaultBranch=master", "init", "--bare"} if sha256 { initArgs = append(initArgs, "--object-format=sha256") diff --git a/source/git/identifier.go b/source/git/identifier.go index 66188c75f953..1675761bde9d 100644 --- a/source/git/identifier.go +++ b/source/git/identifier.go @@ -26,6 +26,7 @@ type GitIdentifier struct { KnownSSHHosts string SkipSubmodules bool MTime string // "checkout" (default) or "commit" + Advice bool FetchByCommit bool // Bundle, when non-empty, instructs the git source to fetch commits from a diff --git a/source/git/identifier_test.go b/source/git/identifier_test.go index 362be97b0a19..0871f2d25548 100644 --- a/source/git/identifier_test.go +++ b/source/git/identifier_test.go @@ -284,6 +284,16 @@ func TestIdentifierBundleValidation(t *testing.T) { require.Empty(t, id.Bundle) }, }, + { + name: "git advice", + url: "https://example.com/repo.git", + attrs: map[string]string{ + pb.AttrGitAdvice: "true", + }, + assert: func(t *testing.T, id *GitIdentifier) { + require.True(t, id.Advice) + }, + }, { name: "bundle+checkoutbundle", url: "https://example.com/repo.git", diff --git a/source/git/source.go b/source/git/source.go index 4a658535d056..35398b842658 100644 --- a/source/git/source.go +++ b/source/git/source.go @@ -147,6 +147,8 @@ func (gs *Source) Identifier(scheme, ref string, attrs map[string]string, platfo id.VerifySignature.IgnoreSignedTag = v == "true" case pb.AttrGitMTime: id.MTime = v + case pb.AttrGitAdvice: + id.Advice = v == "true" case pb.AttrGitFetchByCommit: id.FetchByCommit = v == "true" case pb.AttrGitBundle: @@ -170,7 +172,7 @@ func (gs *Source) Identifier(scheme, ref string, attrs map[string]string, platfo } // needs to be called with repo lock -func (gs *Source) mountRemote(ctx context.Context, remote string, authArgs []string, sha256 bool, reset bool, g session.Group) (target string, release func() error, retErr error) { +func (gs *Source) mountRemote(ctx context.Context, remote string, authArgs []string, gitAdvice bool, sha256 bool, reset bool, g session.Group) (target string, release func() error, retErr error) { sis, err := searchGitRemote(ctx, gs.cache, remote) if err != nil { return "", nil, errors.Wrapf(err, "failed to search metadata for %s", urlutil.RedactCredentials(remote)) @@ -233,6 +235,7 @@ func (gs *Source) mountRemote(ctx context.Context, remote string, authArgs []str }() git := gitCLI( + gitutil.WithGitAdvice(gitAdvice), gitutil.WithGitDir(dir), gitutil.WithArgs(authArgs...), ) @@ -971,7 +974,7 @@ func (gs *gitSourceHandler) tryRemoteFetch(ctx context.Context, jobCtx solver.Jo } } - gitDir, unmountGitDir, err := gs.mountRemote(ctx, gs.src.Remote, authArgs, gs.sha256, reset, g) + gitDir, unmountGitDir, err := gs.mountRemote(ctx, gs.src.Remote, authArgs, gs.src.Advice, gs.sha256, reset, g) if err != nil { return nil, err } @@ -1528,6 +1531,7 @@ func (gs *gitSourceHandler) emptyGitCli(ctx context.Context, g session.Group, op } opts = append([]gitutil.Option{ + gitutil.WithGitAdvice(gs.src.Advice), gitutil.WithArgs(gs.authArgs...), gitutil.WithSSHAuthSock(sock), gitutil.WithSSHKnownHosts(knownHosts), diff --git a/source/git/source_test.go b/source/git/source_test.go index 385592b65905..1e56dff57fb6 100644 --- a/source/git/source_test.go +++ b/source/git/source_test.go @@ -2847,6 +2847,32 @@ func TestGetDefaultBranchRejectsDashPrefixedRef(t *testing.T) { require.Empty(t, ref) } +func TestGitCLIAdvice(t *testing.T) { + run := func(t *testing.T, gitAdvice bool) (env, args []string) { + t.Helper() + cli := gitCLI(gitutil.WithGitAdvice(gitAdvice), gitutil.WithExec(func(ctx context.Context, cmd *exec.Cmd) error { + env = append([]string(nil), cmd.Env...) + args = append([]string(nil), cmd.Args...) + return nil + })) + _, err := cli.Run(t.Context(), "status") + require.NoError(t, err) + return env, args + } + + t.Run("disabled by default", func(t *testing.T) { + env, args := run(t, false) + require.Contains(t, env, "GIT_ADVICE=0") + require.Contains(t, args, "advice.detachedHead=false") + }) + + t.Run("enabled", func(t *testing.T) { + env, args := run(t, true) + require.Contains(t, env, "GIT_ADVICE=1") + require.NotContains(t, args, "advice.detachedHead=false") + }) +} + func TestFetchCommitForBundleUsesOptionTerminator(t *testing.T) { if runtime.GOOS == "windows" { t.Skip("Depends on shell script git stub") @@ -2902,7 +2928,7 @@ func TestDetectBundleSHA256(t *testing.T) { bundlePath := filepath.Join(t.TempDir(), "bundle.pack") runShell(t, repo.mainPath, "git bundle create "+bundlePath+" refs/heads/master") - sha256, err := detectBundleSHA256(ctx, bundlePath) + sha256, err := detectBundleSHA256(ctx, bundlePath, false) require.NoError(t, err) require.Equal(t, format == "sha256", sha256) diff --git a/util/gitutil/git_cli.go b/util/gitutil/git_cli.go index e10733912bd1..2e7c65820534 100644 --- a/util/gitutil/git_cli.go +++ b/util/gitutil/git_cli.go @@ -21,6 +21,7 @@ type GitCLI struct { args []string dir string streams StreamFunc + advice *bool workTree string gitDir string @@ -54,6 +55,13 @@ func WithArgs(args ...string) Option { } } +// WithGitAdvice controls whether Git advice messages are emitted. +func WithGitAdvice(enabled bool) Option { + return func(b *GitCLI) { + b.advice = &enabled + } +} + // WithDir sets working directory. // // This should be a path to any directory within a standard git repository. @@ -165,6 +173,9 @@ func (cli *GitCLI) Run(ctx context.Context, args ...string) (_ []byte, err error // Block sneaky repositories from using repos from the filesystem as submodules. cmd.Args = append(cmd.Args, "-c", "protocol.file.allow=user") + if cli.advice != nil && !*cli.advice { + cmd.Args = append(cmd.Args, "-c", "advice.detachedHead=false") + } if cli.workTree != "" { cmd.Args = append(cmd.Args, "--work-tree", cli.workTree) } @@ -203,6 +214,13 @@ func (cli *GitCLI) Run(ctx context.Context, args ...string) (_ []byte, err error // "GIT_TRACE=1", "LC_ALL=C", // Ensure consistent output. } + if cli.advice != nil { + if *cli.advice { + cmd.Env = append(cmd.Env, "GIT_ADVICE=1") + } else { + cmd.Env = append(cmd.Env, "GIT_ADVICE=0") + } + } if cli.hostGitConfig { for _, ev := range [...]string{ "HOME", diff --git a/util/gitutil/git_cli_test.go b/util/gitutil/git_cli_test.go index 4808d149ef5c..6f644580e81a 100644 --- a/util/gitutil/git_cli_test.go +++ b/util/gitutil/git_cli_test.go @@ -69,3 +69,37 @@ func TestGitCLIConfigEnv(t *testing.T) { require.Contains(t, got, "SUDO_UID=1000") }) } + +func TestGitCLIAdviceOption(t *testing.T) { + run := func(t *testing.T, opts ...Option) (env, args []string) { + t.Helper() + opts = append(opts, WithExec(func(ctx context.Context, cmd *exec.Cmd) error { + env = append([]string(nil), cmd.Env...) + args = append([]string(nil), cmd.Args...) + return nil + })) + cli := NewGitCLI(opts...) + _, err := cli.Run(t.Context(), "status") + require.NoError(t, err) + return env, args + } + + t.Run("unset by default", func(t *testing.T) { + env, args := run(t) + require.NotContains(t, env, "GIT_ADVICE=0") + require.NotContains(t, env, "GIT_ADVICE=1") + require.NotContains(t, args, "advice.detachedHead=false") + }) + + t.Run("disabled", func(t *testing.T) { + env, args := run(t, WithGitAdvice(false)) + require.Contains(t, env, "GIT_ADVICE=0") + require.Contains(t, args, "advice.detachedHead=false") + }) + + t.Run("enabled", func(t *testing.T) { + env, args := run(t, WithGitAdvice(true)) + require.Contains(t, env, "GIT_ADVICE=1") + require.NotContains(t, args, "advice.detachedHead=false") + }) +}