ronantakizawa/codereview-bench huggingface dataset:
Repository: docker/compose
File: cmd/cmdtrace/cmd_span.go (Go)

Code before:
			defer cancel()
			// TODO(milas): add an env var to enable logging from the
			// OTel components for debugging purposes
			_ = tracingShutdown(ctx)
		}
		return cmdErr
	}
}

// commandName returns the path components for a given command.
//
// The root Compose command and anything before (i.e. "docker")
// are not included.
//
// For example:
//   - docker compose alpha watch -> [alpha, watch]
//   - docker-compose up -> [up]
func commandName(cmd *cobra.Command) []string {
	var name []string
	for c := cmd; c != nil; c = c.Parent() {
		if c.Name() == commands.PluginName {
			break
		}
		name = append(name, c.Name())
	}
	for i, j := 0, len(name)-1; i < j; i, j = i+1, j-1 {
		name[i], name[j] = name[j], name[i]
	}
	return name
}

func getFlags(fs *flag.FlagSet) []string {
	var result []string
	fs.Visit(func(flag *flag.Flag) {
		result = append(result, flag.Name)
	})
	return result
}


Review comment (suggestion):
can use `slices.Reverse` - which basically does the same, but will make the intent obvious

Code after:
			ctx, cancel := context.WithTimeout(context.Background(), 100*time.Millisecond)
			defer cancel()
			// TODO(milas): add an env var to enable logging from the
			// OTel components for debugging purposes
			_ = tracingShutdown(ctx)
		}
		return cmdErr
	}
}

// commandName returns the path components for a given command,
// in reverse alphabetical order for consistent usage metrics.
//
// The root Compose command and anything before (i.e. "docker")
// are not included.
//
// For example:
//   - docker compose alpha watch -> [watch, alpha]
//   - docker-compose up -> [up]
func commandName(cmd *cobra.Command) []string {
	var name []string
	for c := cmd; c != nil; c = c.Parent() {
		if c.Name() == commands.PluginName {
			break
		}
		name = append(name, c.Name())
	}
	sort.Sort(sort.Reverse(sort.StringSlice(name)))
	return name
}

func getFlags(fs *flag.FlagSet) []string {
	var result []string
	fs.Visit(func(flag *flag.Flag) {
		result = append(result, flag.Name)
	})
	return result
}
Repository: docker/compose
File: cmd/compose/version_test.go (Go)

Code before:
   Unless required by applicable law or agreed to in writing, software
   distributed under the License is distributed on an "AS IS" BASIS,
   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
   See the License for the specific language governing permissions and
   limitations under the License.
*/

package compose

import (
	"bytes"
	"testing"

	"github.com/docker/cli/cli/streams"
	"github.com/docker/compose/v2/internal"
	"github.com/docker/compose/v2/pkg/mocks"
	"go.uber.org/mock/gomock"
	"gotest.tools/v3/assert"
)

func TestVersionCommand(t *testing.T) {
	originalVersion := internal.Version
	defer func() {
		internal.Version = originalVersion
	}()
	internal.Version = "v2.33.0-desktop.1"

	tests := []struct {
		name string
		args []string
		want string
	}{
		{
			name: "default",
			args: []string{},
			want: "Docker Compose version v2.33.0-desktop.1\n",
		},
		{
			name: "short flag",
			args: []string{"--short"},
			want: "2.33.0-desktop.1\n",
		},
		{
			name: "json flag",
			args: []string{"--format", "json"},
			want: `{"version":"v2.33.0-desktop.1"}` + "\n",
		},
	}

	for _, test := range tests {
		t.Run(test.name, func(t *testing.T) {

Review comment (suggestion):
This version string is very specific to Docker Desktop and will be outdated once we release v2.34, I fear this will bring some confusion. Better use a pure fake version for testing purpose: `v9.9.9-test` for example

Code after:
   Unless required by applicable law or agreed to in writing, software
   distributed under the License is distributed on an "AS IS" BASIS,
   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
   See the License for the specific language governing permissions and
   limitations under the License.
*/

package compose

import (
	"bytes"
	"testing"

	"github.com/docker/cli/cli/streams"
	"github.com/docker/compose/v2/internal"
	"github.com/docker/compose/v2/pkg/mocks"
	"go.uber.org/mock/gomock"
	"gotest.tools/v3/assert"
)

func TestVersionCommand(t *testing.T) {
	originalVersion := internal.Version
	defer func() {
		internal.Version = originalVersion
	}()
	internal.Version = "v9.9.9-test"

	tests := []struct {
		name string
		args []string
		want string
	}{
		{
			name: "default",
			args: []string{},
			want: "Docker Compose version v9.9.9-test\n",
		},
		{
			name: "short flag",
			args: []string{"--short"},
			want: "9.9.9-test\n",
		},
		{
			name: "json flag",
			args: []string{"--format", "json"},
			want: `{"version":"v9.9.9-test"}` + "\n",
		},
	}

	for _, test := range tests {
		t.Run(test.name, func(t *testing.T) {
Repository: docker/compose
File: pkg/remote/git.go (Go)

Code before:
		}

		local = filepath.Join(cache, ref.Commit)
		if _, err := os.Stat(local); os.IsNotExist(err) {
			if g.offline {
				return "", nil
			}
			err = g.checkout(ctx, local, ref)
			if err != nil {
				return "", err
			}
		}
		g.known[path] = local
	}
	if ref.SubDir != "" {
		local = filepath.Join(local, ref.SubDir)
	}
	stat, err := os.Stat(local)
	if err != nil {
		return "", err
	}
	if stat.IsDir() {
		local, err = findFile(cli.DefaultFileNames, local)
	}
	if progress.Mode != progress.ModeQuiet && progress.Mode != progress.ModeJSON {
		_, _ = fmt.Fprintf(g.dockerCli.Out(), "Your compose stack %q is stored in %q\n", strings.TrimPrefix(path, prefix), local)
	}
	return local, err
}

func (g gitRemoteLoader) Dir(path string) string {
	return g.known[path]
}

func (g gitRemoteLoader) resolveGitRef(ctx context.Context, path string, ref *gitutil.GitRef) error {
	if !commitSHA.MatchString(ref.Commit) {
		cmd := exec.CommandContext(ctx, "git", "ls-remote", "--exit-code", ref.Remote, ref.Commit)
		cmd.Env = g.gitCommandEnv()
		out, err := cmd.Output()
		if err != nil {
			if cmd.ProcessState.ExitCode() == 2 {
				return fmt.Errorf("repository does not contain ref %s, output: %q: %w", path, string(out), err)
			}
			return err
		}
		if len(out) < 40 {
			return fmt.Errorf("unexpected git command output: %q", string(out))
		}
		sha := string(out[:40])
		if !commitSHA.MatchString(sha) {
			return fmt.Errorf("invalid commit sha %q", sha)

Review comment (suggestion):
this message will print for every Git remote loaded by compose, including those involved by `includes`
Maybe better load project, then check if `project.ComposeFiles[0]` is remote _then_ print result of `Dir(".")` as location where remote stack has been stored on disk


Code after:
		local = filepath.Join(cache, ref.Commit)
		if _, err := os.Stat(local); os.IsNotExist(err) {
			if g.offline {
				return "", nil
			}
			err = g.checkout(ctx, local, ref)
			if err != nil {
				return "", err
			}
		}
		g.known[path] = local
	}
	if ref.SubDir != "" {
		local = filepath.Join(local, ref.SubDir)
	}
	stat, err := os.Stat(local)
	if err != nil {
		return "", err
	}
	if stat.IsDir() {
		local, err = findFile(cli.DefaultFileNames, local)
	}
	return local, err
}

func (g gitRemoteLoader) Dir(path string) string {
	return g.known[path]
}

func (g gitRemoteLoader) resolveGitRef(ctx context.Context, path string, ref *gitutil.GitRef) error {
	if !commitSHA.MatchString(ref.Commit) {
		cmd := exec.CommandContext(ctx, "git", "ls-remote", "--exit-code", ref.Remote, ref.Commit)
		cmd.Env = g.gitCommandEnv()
		out, err := cmd.Output()
		if err != nil {
			if cmd.ProcessState.ExitCode() == 2 {
				return fmt.Errorf("repository does not contain ref %s, output: %q: %w", path, string(out), err)
			}
			return err
		}
		if len(out) < 40 {
			return fmt.Errorf("unexpected git command output: %q", string(out))
		}
		sha := string(out[:40])
		if !commitSHA.MatchString(sha) {
			return fmt.Errorf("invalid commit sha %q", sha)
		}
		ref.Commit = sha
	}
	return nil
}
Repository: docker/compose
File: cmd/compose/up.go (Go)

Code before:
			AttachTo:       attach,
			ExitCodeFrom:   upOptions.exitCodeFrom,
			OnExit:         upOptions.OnExit(),
			Wait:           upOptions.wait,
			WaitTimeout:    timeout,
			Watch:          upOptions.watch,
			Services:       services,
			NavigationMenu: upOptions.navigationMenu && ui.Mode != "plain",
		},
	})
}

func setServiceScale(project *types.Project, name string, replicas int) error {
	service, err := project.GetService(name)
	if err != nil {
		return err
	}
	service.SetScale(replicas)
	project.Services[name] = service
	return nil
}

func displayLocationRemoteStack(dockerCli command.Cli, project *types.Project, options buildOptions) {
	mainComposeFile := options.ProjectOptions.ConfigPaths[0]
	if ui.Mode != ui.ModeQuiet && ui.Mode != ui.ModeJSON {
		if strings.HasPrefix(mainComposeFile, remote.OciPrefix) || strings.HasPrefix(mainComposeFile, "git://") {
			artifactName := strings.TrimPrefix(strings.TrimPrefix(mainComposeFile, remote.OciPrefix), "git://")
			_, _ = fmt.Fprintf(dockerCli.Out(), "Your compose stack %q is stored in %q\n", artifactName, project.WorkingDir)
		}
	}
}


Review comment (suggestion):
Better use `remote.Accept` as Git support has a bunch of corner cases

Code after:
			ExitCodeFrom:   upOptions.exitCodeFrom,
			OnExit:         upOptions.OnExit(),
			Wait:           upOptions.wait,
			WaitTimeout:    timeout,
			Watch:          upOptions.watch,
			Services:       services,
			NavigationMenu: upOptions.navigationMenu && ui.Mode != "plain",
		},
	})
}

func setServiceScale(project *types.Project, name string, replicas int) error {
	service, err := project.GetService(name)
	if err != nil {
		return err
	}
	service.SetScale(replicas)
	project.Services[name] = service
	return nil
}

func displayLocationRemoteStack(dockerCli command.Cli, project *types.Project, options buildOptions) {
	if len(options.ProjectOptions.ConfigPaths) == 0 {
		return
	}
	mainComposeFile := options.ProjectOptions.ConfigPaths[0]
	if ui.Mode != ui.ModeQuiet && ui.Mode != ui.ModeJSON {
		for _, loader := range options.ProjectOptions.remoteLoaders(dockerCli) {
			if loader.Accept(mainComposeFile) {
				_, _ = fmt.Fprintf(dockerCli.Out(), "Your compose stack %q is stored in %q\n", mainComposeFile, project.WorkingDir)
				return
			}
		}
	}
}
Repository: docker/compose
File: pkg/compose/publish.go (Go)

Code before:
	}
	if len(errorList) > 0 {
		errMsg := "your Compose stack cannot be published as it only contains a build section for service(s):\n"
		for _, serviceInError := range errorList {
			errMsg += fmt.Sprintf("- %q\n", serviceInError)
		}
		return false, errors.New(errMsg)
	}
	return true, nil
}

func (s *composeService) checkForBindMount(project *types.Project) (bool, error) {
	for name, config := range project.Services {
		for _, volume := range config.Volumes {
			if volume.Type == types.VolumeTypeBind {
				return false, fmt.Errorf("cannot publish compose file: service %q relies on bind-mount. You should use volumes", name)
			}
		}
	}
	return true, nil
}

func (s *composeService) checkForSensitiveData(project *types.Project) ([]secrets.DetectedSecret, error) {
	scan := scanner.NewDefaultScanner()

	input, err := project.MarshalYAML()
	if err != nil {
		return nil, err
	}
	return scan.ScanReader(bytes.NewReader(input))
}


Review comment (security):
you MUST check each individual file in the compose project, not just the final model. Otherwise I _may_ publish:
compose.yaml
```yaml
services:
 test
   some: MY_SECRET_PASSWORD
```

compose.yaml
```yaml
services:
 test
   some: !override ${ENTER_YOUR_OWN_SECRET}
```

Publisher expectation would be that secret is not exposed to consumer, but actually it is

Code after:
		findings, err := scan.ScanReader(in)
		if err != nil {
			return nil, fmt.Errorf("failed to scan compose file %s: %w", file, err)
		}
		allFindings = append(allFindings, findings...)
	}
	for _, service := range project.Services {
		// Check env files
		for _, envFile := range service.EnvFiles {
			findings, err := scan.ScanFile(envFile.Path)
			if err != nil {
				return nil, fmt.Errorf("failed to scan env file %s: %w", envFile.Path, err)
			}
			allFindings = append(allFindings, findings...)
		}
	}

	// Check configs defined by files
	for _, config := range project.Configs {
		if config.File != "" {
			findings, err := scan.ScanFile(config.File)
			if err != nil {
				return nil, fmt.Errorf("failed to scan config file %s: %w", config.File, err)
			}
			allFindings = append(allFindings, findings...)
		}
	}

	// Check secrets defined by files
	for _, secret := range project.Secrets {
		if secret.File != "" {
			findings, err := scan.ScanFile(secret.File)
			if err != nil {
				return nil, fmt.Errorf("failed to scan secret file %s: %w", secret.File, err)
			}
			allFindings = append(allFindings, findings...)
		}
	}

	return allFindings, nil
}

func composeFileAsByteReader(filePath string, project *types.Project) (io.Reader, error) {
	composeFile, err := os.ReadFile(filePath)
	if err != nil {
		return nil, fmt.Errorf("failed to open compose file %s: %w", filePath, err)
	}
	base, err := loader.LoadWithContext(context.TODO(), types.ConfigDetails{
		WorkingDir:  project.WorkingDir,
		Environment: project.Environment,
		ConfigFiles: []types.ConfigFile{
Repository: docker/compose
File: cmd/compose/run.go (Go)

Code before:
	*composeOptions
	Service       string
	Command       []string
	environment   []string
	Detach        bool
	Remove        bool
	noTty         bool
	tty           bool
	interactive   bool
	user          string
	workdir       string
	entrypoint    string
	entrypointCmd []string
	capAdd        opts.ListOpts
	capDrop       opts.ListOpts
	labels        []string
	volumes       []string
	publish       []string
	useAliases    bool
	servicePorts  bool
	name          string
	noDeps        bool
	ignoreOrphans bool
	removeOrphans bool
	quietPull     bool
	EnvFiles      []string
}

func (options runOptions) apply(project *types.Project) (*types.Project, error) {
	if options.noDeps {
		var err error
		project, err = project.WithSelectedServices([]string{options.Service}, types.IgnoreDependencies)
		if err != nil {
			return nil, err
		}
	}

	target, err := project.GetService(options.Service)
	if err != nil {
		return nil, err
	}

	target.Tty = !options.noTty
	target.StdinOpen = options.interactive

	// --service-ports and --publish are incompatible
	if !options.servicePorts {
		if len(target.Ports) > 0 {
			logrus.Debug("Running service without ports exposed as --service-ports=false")
		}
		target.Ports = []types.ServicePortConfig{}

Review comment (suggestion):
doest it need to be access somewhere else?
```suggestion
	envFiles      []string
```

Code after:
	*composeOptions
	Service       string
	Command       []string
	environment   []string
	envFiles      []string
	Detach        bool
	Remove        bool
	noTty         bool
	tty           bool
	interactive   bool
	user          string
	workdir       string
	entrypoint    string
	entrypointCmd []string
	capAdd        opts.ListOpts
	capDrop       opts.ListOpts
	labels        []string
	volumes       []string
	publish       []string
	useAliases    bool
	servicePorts  bool
	name          string
	noDeps        bool
	ignoreOrphans bool
	removeOrphans bool
	quietPull     bool
}

func (options runOptions) apply(project *types.Project) (*types.Project, error) {
	if options.noDeps {
		var err error
		project, err = project.WithSelectedServices([]string{options.Service}, types.IgnoreDependencies)
		if err != nil {
			return nil, err
		}
	}

	target, err := project.GetService(options.Service)
	if err != nil {
		return nil, err
	}

	target.Tty = !options.noTty
	target.StdinOpen = options.interactive

	// --service-ports and --publish are incompatible
	if !options.servicePorts {
		if len(target.Ports) > 0 {
			logrus.Debug("Running service without ports exposed as --service-ports=false")
		}
		target.Ports = []types.ServicePortConfig{}
Repository: docker/compose
File: pkg/compose/create.go (Go)

Code before:
		}
		if m.Type == mount.TypeBind {
			// `Mount` is preferred but does not offer option to created host path if missing
			// so `Bind` API is used here with raw volume string
			// see https://github.com/moby/moby/issues/43483
			v := findVolumeByTarget(service.Volumes, m.Target)
			if v == nil {
				continue MOUNTS
			}
			switch {
			case v.Type != types.VolumeTypeBind:
				v.Source = m.Source
				fallthrough
			case !requireMountAPI(v.Bind):
				vol := findVolumeByName(p.Volumes, m.Source)
				binds = append(binds, toBindString(vol.Name, v))
				continue MOUNTS
			}
		}
		if m.Type == mount.TypeVolume {
			v := findVolumeByTarget(service.Volumes, m.Target)
			if v == nil {
				continue MOUNTS
			}
			vol := findVolumeByName(p.Volumes, m.Source)
			if vol.Driver == "local" && vol.DriverOpts["o"] == "bind" {
				// Looks like a volume, but actually a bind mount which requires the bind API
				binds = append(binds, toBindString(vol.Name, v))
				continue MOUNTS
			}
		}
		mounts = append(mounts, m)
	}
	return binds, mounts, nil
}

func toBindString(name string, v *types.ServiceVolumeConfig) string {
	access := "rw"
	if v.ReadOnly {
		access = "ro"
	}
	options := []string{access}
	if v.Bind != nil && v.Bind.SELinux != "" {
		options = append(options, v.Bind.SELinux)
	}
	if v.Bind != nil && v.Bind.Propagation != "" {
		options = append(options, v.Bind.Propagation)
	}
	if v.Volume != nil && v.Volume.NoCopy {
		options = append(options, "nocopy")
	}

Review comment (suggestion):
I think the trick also requires `--opt type=none` 

Code after:
			// so `Bind` API is used here with raw volume string
			// see https://github.com/moby/moby/issues/43483
			v := findVolumeByTarget(service.Volumes, m.Target)
			if v != nil {
				switch {
				case v.Type != types.VolumeTypeBind:
					v.Source = m.Source
					fallthrough
				case !requireMountAPI(v.Bind):
					vol := findVolumeByName(p.Volumes, m.Source)
					if vol != nil {
						binds = append(binds, toBindString(vol.Name, v))
						continue
					}
				}
			}
		case mount.TypeVolume:
			v := findVolumeByTarget(service.Volumes, m.Target)
			vol := findVolumeByName(p.Volumes, m.Source)
			if v != nil && vol != nil {
				if _, ok := vol.DriverOpts["device"]; ok && vol.Driver == "local" && vol.DriverOpts["o"] == "bind" {
					// Looks like a volume, but actually a bind mount which requires the bind API
					binds = append(binds, toBindString(vol.Name, v))
					continue
				}
			}
		case mount.TypeImage:
			version, err := s.RuntimeVersion(ctx)
			if err != nil {
				return nil, nil, err
			}
			if versions.LessThan(version, "1.48") {
				return nil, nil, fmt.Errorf("volume with type=image require Docker Engine v28 or later")
			}
		}
		mounts = append(mounts, m)
	}
	return binds, mounts, nil
}

func toBindString(name string, v *types.ServiceVolumeConfig) string {
	access := "rw"
	if v.ReadOnly {
		access = "ro"
	}
	options := []string{access}
	if v.Bind != nil && v.Bind.SELinux != "" {
		options = append(options, v.Bind.SELinux)
	}
	if v.Bind != nil && v.Bind.Propagation != "" {
		options = append(options, v.Bind.Propagation)
Repository: docker/compose
File: pkg/compose/create.go (Go)

Code before:
}

func (s *composeService) buildContainerVolumes(
	ctx context.Context,
	p types.Project,
	service types.ServiceConfig,
	inherit *container.Summary,
) ([]string, []mount.Mount, error) {
	var mounts []mount.Mount
	var binds []string

	img := api.GetImageNameOrDefault(service, p.Name)
	imgInspect, err := s.apiClient().ImageInspect(ctx, img)
	if err != nil {
		return nil, nil, err
	}

	mountOptions, err := buildContainerMountOptions(p, service, imgInspect, inherit)
	if err != nil {
		return nil, nil, err
	}

MOUNTS:
	for _, m := range mountOptions {
		if m.Type == mount.TypeNamedPipe {
			mounts = append(mounts, m)
			continue
		}
		if m.Type == mount.TypeBind {
			// `Mount` is preferred but does not offer option to created host path if missing
			// so `Bind` API is used here with raw volume string
			// see https://github.com/moby/moby/issues/43483
			for _, v := range service.Volumes {
				if v.Target == m.Target {
					switch {
					case string(m.Type) != v.Type:
						v.Source = m.Source
						fallthrough
					case !requireMountAPI(v.Bind):
						binds = append(binds, v.String())
						continue MOUNTS
					}
				}
			}
		}
		if m.Type == mount.TypeImage {
			version, err := s.RuntimeVersion(ctx)
			if err != nil {
				return nil, nil, err
			}
			if versions.LessThan(version, "1.48") {

Review comment (suggestion):
Don't we loose the check of `TypeNamedPipe` here?
```suggestion
		switch m.Type {
		case mount.TypeNamedPipe:
		    mounts = append(mounts, m)
		    continue 
		case mount.TypeBind:
```

Code after:
}

//nolint:gocyclo
func (s *composeService) buildContainerVolumes(
	ctx context.Context,
	p types.Project,
	service types.ServiceConfig,
	inherit *container.Summary,
) ([]string, []mount.Mount, error) {
	var mounts []mount.Mount
	var binds []string

	img := api.GetImageNameOrDefault(service, p.Name)
	imgInspect, err := s.apiClient().ImageInspect(ctx, img)
	if err != nil {
		return nil, nil, err
	}

	mountOptions, err := buildContainerMountOptions(p, service, imgInspect, inherit)
	if err != nil {
		return nil, nil, err
	}

	for _, m := range mountOptions {
		switch m.Type {
		case mount.TypeBind:
			// `Mount` is preferred but does not offer option to created host path if missing
			// so `Bind` API is used here with raw volume string
			// see https://github.com/moby/moby/issues/43483
			v := findVolumeByTarget(service.Volumes, m.Target)
			if v != nil {
				switch {
				case v.Type != types.VolumeTypeBind:
					v.Source = m.Source
					fallthrough
				case !requireMountAPI(v.Bind):
					vol := findVolumeByName(p.Volumes, m.Source)
					if vol != nil {
						binds = append(binds, toBindString(vol.Name, v))
						continue
					}
				}
			}
		case mount.TypeVolume:
			v := findVolumeByTarget(service.Volumes, m.Target)
			vol := findVolumeByName(p.Volumes, m.Source)
			if v != nil && vol != nil {
				if _, ok := vol.DriverOpts["device"]; ok && vol.Driver == "local" && vol.DriverOpts["o"] == "bind" {
					// Looks like a volume, but actually a bind mount which requires the bind API
					binds = append(binds, toBindString(vol.Name, v))
					continue