ronantakizawa/codereview-bench huggingface dataset:
Repository: docker/compose
File: pkg/compose/create.go (Go)

Code before:
			if v != nil {
				if v.Type != types.VolumeTypeBind {
					v.Source = m.Source
				}
				if !bindRequiresMountAPI(v.Bind) {
					source := m.Source
					if vol := findVolumeByName(p.Volumes, m.Source); vol != nil {
						source = m.Source
					}
					binds = append(binds, toBindString(source, v))
					continue
				}
			}
		case mount.TypeVolume:
			v := findVolumeByTarget(service.Volumes, m.Target)
			vol := findVolumeByName(p.Volumes, m.Source)
			if v != nil && vol != nil {
				// Prefer the bind API if no advanced option is used, to preserve backward compatibility
				if !volumeRequiresMountAPI(v.Volume) {
					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)
	}
	if v.Volume != nil && v.Volume.NoCopy {
		options = append(options, "nocopy")

Review comment (suggestion):
removed this earlier fix, as with `volumeRequiresMountAPI` we will prefer `bind` for all volumes without advanced mount options, not just those created with `o=bind`

Code after:
				if v.Type != types.VolumeTypeBind {
					v.Source = m.Source
				}
				if !bindRequiresMountAPI(v.Bind) {
					source := m.Source
					if vol := findVolumeByName(p.Volumes, m.Source); vol != nil {
						source = m.Source
					}
					binds = append(binds, toBindString(source, v))
					continue
				}
			}
		case mount.TypeVolume:
			v := findVolumeByTarget(service.Volumes, m.Target)
			vol := findVolumeByName(p.Volumes, m.Source)
			if v != nil && vol != nil {
				// Prefer the bind API if no advanced option is used, to preserve backward compatibility
				if !volumeRequiresMountAPI(v.Volume) {
					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)
	}
	if v.Volume != nil && v.Volume.NoCopy {
		options = append(options, "nocopy")
	}
Repository: docker/compose
File: pkg/compose/create.go (Go)

Code before:
	}
	return bindings
}

func getDependentServiceFromMode(mode string) string {
	if strings.HasPrefix(
		mode,
		types.NetworkModeServicePrefix,
	) {
		return mode[len(types.NetworkModeServicePrefix):]
	}
	return ""
}

//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

	mountOptions, err := s.buildContainerMountOptions(ctx, p, service, 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 {
				if v.Type != types.VolumeTypeBind {
					v.Source = m.Source
				}
				if !bindRequiresMountAPI(v.Bind) {
					source := m.Source
					if vol := findVolumeByName(p.Volumes, m.Source); vol != nil {
						source = m.Source
					}
					binds = append(binds, toBindString(source, v))
					continue
				}
			}
		case mount.TypeVolume:
			v := findVolumeByTarget(service.Volumes, m.Target)

Review comment (suggestion):
moved to `buildContainerMountOptions` as this is only used when we manage anonymous volumes inheritance (allows to write test without a useless API call/mock)

Code after:
	}
	return bindings
}

func getDependentServiceFromMode(mode string) string {
	if strings.HasPrefix(
		mode,
		types.NetworkModeServicePrefix,
	) {
		return mode[len(types.NetworkModeServicePrefix):]
	}
	return ""
}

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

	mountOptions, err := s.buildContainerMountOptions(ctx, p, service, 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 {
				if v.Type != types.VolumeTypeBind {
					v.Source = m.Source
				}
				if !bindRequiresMountAPI(v.Bind) {
					source := m.Source
					if vol := findVolumeByName(p.Volumes, m.Source); vol != nil {
						source = m.Source
					}
					binds = append(binds, toBindString(source, v))
					continue
				}
			}
		case mount.TypeVolume:
			v := findVolumeByTarget(service.Volumes, m.Target)
			vol := findVolumeByName(p.Volumes, m.Source)
Repository: docker/compose
File: pkg/progress/writer.go (Go)

Code before:
}

const (
	// ModeAuto detect console capabilities
	ModeAuto = "auto"
	// ModeTTY use terminal capability for advanced rendering
	ModeTTY = "tty"
	// ModePlain dump raw events to output
	ModePlain = "plain"
	// ModeQuiet don't display events
	ModeQuiet = "quiet"
	// ModeJSON outputs a machine-readable JSON stream
	ModeJSON = "json"
)

// Mode define how progress should be rendered, either as ModePlain or ModeTTY
var Mode = ModeAuto

// NewWriter returns a new multi-progress writer
func NewWriter(ctx context.Context, out *streams.Out, progressTitle string) (Writer, error) {
	isTerminal := out.IsTerminal()
	dryRun, ok := ctx.Value(api.DryRunKey{}).(bool)
	if !ok {
		dryRun = false
	}
	if v, ok := os.LookupEnv("COMPOSE_PROGRESS"); ok && Mode == ModeAuto {
		Mode = v
	}
	if Mode == ModeQuiet {
		return quiet{}, nil
	}

	tty := Mode == ModeTTY
	if Mode == ModeAuto && isTerminal {
		tty = true
	}
	if tty {
		return newTTYWriter(out, dryRun, progressTitle)
	}
	if Mode == ModeJSON {
		return &jsonWriter{
			out:    out,
			done:   make(chan bool),
			dryRun: dryRun,
		}, nil
	}
	return &plainWriter{
		out:    out,
		done:   make(chan bool),
		dryRun: dryRun,
	}, nil

Review comment (suggestion):
a better place to manage this is `addProjectFlags` in cmd/compose.go, as you can set flag default value from env value (the same way we do for `--env-file`)

Code after:

const (
	// ModeAuto detect console capabilities
	ModeAuto = "auto"
	// ModeTTY use terminal capability for advanced rendering
	ModeTTY = "tty"
	// ModePlain dump raw events to output
	ModePlain = "plain"
	// ModeQuiet don't display events
	ModeQuiet = "quiet"
	// ModeJSON outputs a machine-readable JSON stream
	ModeJSON = "json"
)

// Mode define how progress should be rendered, either as ModePlain or ModeTTY
var Mode = ModeAuto

// NewWriter returns a new multi-progress writer
func NewWriter(ctx context.Context, out *streams.Out, progressTitle string) (Writer, error) {
	isTerminal := out.IsTerminal()
	dryRun, ok := ctx.Value(api.DryRunKey{}).(bool)
	if !ok {
		dryRun = false
	}
	if Mode == ModeQuiet {
		return quiet{}, nil
	}

	tty := Mode == ModeTTY
	if Mode == ModeAuto && isTerminal {
		tty = true
	}
	if tty {
		return newTTYWriter(out, dryRun, progressTitle)
	}
	if Mode == ModeJSON {
		return &jsonWriter{
			out:    out,
			done:   make(chan bool),
			dryRun: dryRun,
		}, nil
	}
	return &plainWriter{
		out:    out,
		done:   make(chan bool),
		dryRun: dryRun,
	}, nil
}

func newTTYWriter(out io.Writer, dryRun bool, progressTitle string) (Writer, error) {
	return &ttyWriter{
Repository: docker/compose
File: pkg/compose/plugins.go (Go)

Code before:
	SetEnvType = "setenv"
)

func (s *composeService) runPlugin(ctx context.Context, project *types.Project, service types.ServiceConfig, command string) error {
	provider := *service.Provider

	plugin, err := s.getPluginBinaryPath(provider.Type)
	if err != nil {
		return err
	}

	if err := s.checkPluginEnabledInDD(ctx, plugin); err != nil {
		return err
	}

	cmd := s.setupPluginCommand(ctx, project, provider, plugin.Path, command)

	variables, err := s.executePlugin(ctx, cmd, command, service)
	if err != nil {
		return err
	}

	prefix := strings.ToUpper(service.Name) + "_"
	for name, s := range project.Services {
		if _, ok := s.DependsOn[service.Name]; ok {
			for key, val := range variables {
				s.Environment[prefix+key] = &val
			}
			project.Services[name] = s
		}
	}
	return nil
}

func (s *composeService) executePlugin(ctx context.Context, cmd *exec.Cmd, command string, service types.ServiceConfig) (types.Mapping, error) {
	eg := errgroup.Group{}
	stdout, err := cmd.StdoutPipe()
	if err != nil {
		return nil, err
	}

	err = cmd.Start()
	if err != nil {
		return nil, err
	}
	eg.Go(cmd.Wait)

	decoder := json.NewDecoder(stdout)
	defer func() { _ = stdout.Close() }()

	variables := types.Mapping{}

Review comment (suggestion):
micro-nit 😅 (definitely not blocking); looks like the `prefix` is only used when `depends_on` matches, so could be put inside that branch; 

```suggestion
	for name, s := range project.Services {
		if _, ok := s.DependsOn[service.Name]; ok {
			prefix := strings.ToUpper(service.Name) + "_"
			for key, val := range variables {
```


Code after:
	SetEnvType = "setenv"
)

func (s *composeService) runPlugin(ctx context.Context, project *types.Project, service types.ServiceConfig, command string) error {
	provider := *service.Provider

	plugin, err := s.getPluginBinaryPath(provider.Type)
	if err != nil {
		return err
	}

	if err := s.checkPluginEnabledInDD(ctx, plugin); err != nil {
		return err
	}

	cmd := s.setupPluginCommand(ctx, project, provider, plugin.Path, command)

	variables, err := s.executePlugin(ctx, cmd, command, service)
	if err != nil {
		return err
	}

	for name, s := range project.Services {
		if _, ok := s.DependsOn[service.Name]; ok {
			prefix := strings.ToUpper(service.Name) + "_"
			for key, val := range variables {
				s.Environment[prefix+key] = &val
			}
			project.Services[name] = s
		}
	}
	return nil
}

func (s *composeService) executePlugin(ctx context.Context, cmd *exec.Cmd, command string, service types.ServiceConfig) (types.Mapping, error) {
	eg := errgroup.Group{}
	stdout, err := cmd.StdoutPipe()
	if err != nil {
		return nil, err
	}

	err = cmd.Start()
	if err != nil {
		return nil, err
	}
	eg.Go(cmd.Wait)

	decoder := json.NewDecoder(stdout)
	defer func() { _ = stdout.Close() }()

	variables := types.Mapping{}
Repository: docker/compose
File: pkg/compose/plugins.go (Go)

Code before:
		return nil, err
	}

	err = cmd.Start()
	if err != nil {
		return nil, err
	}
	eg.Go(cmd.Wait)

	decoder := json.NewDecoder(stdout)
	defer func() { _ = stdout.Close() }()

	variables := types.Mapping{}

	pw := progress.ContextWriter(ctx)
	var action string
	switch command {
	case "up":
		pw.Event(progress.CreatingEvent(service.Name))
		action = "create"
	case "down":
		pw.Event(progress.RemovingEvent(service.Name))
		action = "remove"
	}
	for {
		var msg JsonMessage
		err = decoder.Decode(&msg)
		if errors.Is(err, io.EOF) {
			break
		}
		if err != nil {
			return nil, err
		}
		switch msg.Type {
		case ErrorType:
			pw.Event(progress.ErrorMessageEvent(service.Name, "error"))
			return nil, errors.New(msg.Message)
		case InfoType:
			pw.Event(progress.ErrorMessageEvent(service.Name, msg.Message))
		case SetEnvType:
			key, val, found := strings.Cut(msg.Message, "=")
			if !found {
				return nil, fmt.Errorf("invalid response from plugin: %s", msg.Message)
			}
			variables[key] = val
		default:
			return nil, fmt.Errorf("invalid response from plugin: %s", msg.Type)
		}
	}

	err = eg.Wait()

Review comment (suggestion):
Should this switch also have a default? Looks like if `command` is not `up` or `down`, then the switch at the end of the function returns an error (but after a bunch of work is done). Maybe that's intentional, but I just noticed it.

If it's intentional, it would be good to add a `default:` with a comment that explains that it is intentional (but will produce an error further down).

Code after:
		return nil, err
	}

	err = cmd.Start()
	if err != nil {
		return nil, err
	}
	eg.Go(cmd.Wait)

	decoder := json.NewDecoder(stdout)
	defer func() { _ = stdout.Close() }()

	variables := types.Mapping{}

	pw := progress.ContextWriter(ctx)
	var action string
	switch command {
	case "up":
		pw.Event(progress.CreatingEvent(service.Name))
		action = "create"
	case "down":
		pw.Event(progress.RemovingEvent(service.Name))
		action = "remove"
	default:
		return nil, fmt.Errorf("unsupported plugin command: %s", command)
	}
	for {
		var msg JsonMessage
		err = decoder.Decode(&msg)
		if errors.Is(err, io.EOF) {
			break
		}
		if err != nil {
			return nil, err
		}
		switch msg.Type {
		case ErrorType:
			pw.Event(progress.ErrorMessageEvent(service.Name, "error"))
			return nil, errors.New(msg.Message)
		case InfoType:
			pw.Event(progress.ErrorMessageEvent(service.Name, msg.Message))
		case SetEnvType:
			key, val, found := strings.Cut(msg.Message, "=")
			if !found {
				return nil, fmt.Errorf("invalid response from plugin: %s", msg.Message)
			}
			variables[key] = val
		default:
			return nil, fmt.Errorf("invalid response from plugin: %s", msg.Type)
		}
	}
Repository: docker/compose
File: docs/examples/provider.go (Go)

Code before:
func composeCommand() *cobra.Command {
	c := &cobra.Command{
		Use:              "compose EVENT",
		TraverseChildren: true,
	}
	c.PersistentFlags().String("project-name", "", "compose project name") // unused
	c.AddCommand(&cobra.Command{
		Use:  "up",
		Run:  up,
		Args: cobra.ExactArgs(1),
	})
	c.AddCommand(&cobra.Command{
		Use:  "down",
		Run:  down,
		Args: cobra.ExactArgs(1),
	})
	return c
}

func up(_ *cobra.Command, args []string) {
	servicename := args[0]
	for i := 0; i < 100; i += 10 {
		time.Sleep(1 * time.Second)
		fmt.Printf(`{ "type": "info", "message": "Processing ... %d%%" }\n`, i)
	}
	fmt.Printf(`{ "type": "setenv", "message": "URL=https://magic.cloud/%s" }\n`, servicename)
}

func down(_ *cobra.Command, _ []string) {
	fmt.Printf(`{ "type": "error", "message": "Permission error" }\n`)
}


Review comment (bug):
What do you think of adding a `debug` message?
```suggestion
        fmt.Printf(`{ "type": "debug", "message": "Starting %s" }`, servicename)
	for i := 0; i < 100; i += 10 {
```

Code after:
func composeCommand() *cobra.Command {
	c := &cobra.Command{
		Use:              "compose EVENT",
		TraverseChildren: true,
	}
	c.PersistentFlags().String("project-name", "", "compose project name") // unused
	c.AddCommand(&cobra.Command{
		Use:  "up",
		Run:  up,
		Args: cobra.ExactArgs(1),
	})
	c.AddCommand(&cobra.Command{
		Use:  "down",
		Run:  down,
		Args: cobra.ExactArgs(1),
	})
	return c
}

const lineSeparator = "\n"

func up(_ *cobra.Command, args []string) {
	servicename := args[0]
	fmt.Printf(`{ "type": "debug", "message": "Starting %s" }%s`, servicename, lineSeparator)

	for i := 0; i < 100; i += 10 {
		time.Sleep(1 * time.Second)
		fmt.Printf(`{ "type": "info", "message": "Processing ... %d%%" }%s`, i, lineSeparator)
	}
	fmt.Printf(`{ "type": "setenv", "message": "URL=https://magic.cloud/%s" }%s`, servicename, lineSeparator)
}

func down(_ *cobra.Command, _ []string) {
	fmt.Printf(`{ "type": "error", "message": "Permission error" }%s`, lineSeparator)
}
Repository: docker/compose
File: docs/examples/provider.go (Go)

Code before:
	}
	c.PersistentFlags().String("project-name", "", "compose project name") // unused
	c.AddCommand(&cobra.Command{
		Use:  "up",
		Run:  up,
		Args: cobra.ExactArgs(1),
	})
	c.AddCommand(&cobra.Command{
		Use:  "down",
		Run:  down,
		Args: cobra.ExactArgs(1),
	})
	return c
}

func up(_ *cobra.Command, args []string) {
	servicename := args[0]
	for i := 0; i < 100; i += 10 {
		time.Sleep(1 * time.Second)
		fmt.Printf(`{ "type": "info", "message": "Processing ... %d%%" }\n`, i)
	}
	fmt.Printf(`{ "type": "setenv", "message": "URL=https://magic.cloud/%s" }\n`, servicename)
}

func down(_ *cobra.Command, _ []string) {
	fmt.Printf(`{ "type": "error", "message": "Permission error" }\n`)
}


Review comment (bug):
Removing `\n`, the parsing fails on the Compose side with the following error `invalid character '\\' looking for beginning of object key string`

```suggestion
		fmt.Printf(`{ "type": "info", "message": "Processing ... %d%%" }`, i)
	}
	fmt.Printf(`{ "type": "setenv", "message": "URL=https://magic.cloud/%s" }`, servicename)
}

func down(_ *cobra.Command, _ []string) {
	fmt.Printf(`{ "type": "error", "message": "Permission error" }`)
```

Code after:
	}
	c.PersistentFlags().String("project-name", "", "compose project name") // unused
	c.AddCommand(&cobra.Command{
		Use:  "up",
		Run:  up,
		Args: cobra.ExactArgs(1),
	})
	c.AddCommand(&cobra.Command{
		Use:  "down",
		Run:  down,
		Args: cobra.ExactArgs(1),
	})
	return c
}

const lineSeparator = "\n"

func up(_ *cobra.Command, args []string) {
	servicename := args[0]
	fmt.Printf(`{ "type": "debug", "message": "Starting %s" }%s`, servicename, lineSeparator)

	for i := 0; i < 100; i += 10 {
		time.Sleep(1 * time.Second)
		fmt.Printf(`{ "type": "info", "message": "Processing ... %d%%" }%s`, i, lineSeparator)
	}
	fmt.Printf(`{ "type": "setenv", "message": "URL=https://magic.cloud/%s" }%s`, servicename, lineSeparator)
}

func down(_ *cobra.Command, _ []string) {
	fmt.Printf(`{ "type": "error", "message": "Permission error" }%s`, lineSeparator)
}
Repository: docker/compose
File: pkg/e2e/compose_run_test.go (Go)

Code before:
	t.Run("--pull", func(t *testing.T) {
		res := c.RunDockerComposeCmd(t, "-f", "./fixtures/run-test/pull.yaml", "down", "--remove-orphans", "--rmi", "all")
		res.Assert(t, icmd.Success)

		res = c.RunDockerComposeCmd(t, "-f", "./fixtures/run-test/pull.yaml", "run", "--pull", "always", "backend")
		assert.Assert(t, strings.Contains(res.Combined(), "backend Pulling"), res.Combined())
		assert.Assert(t, strings.Contains(res.Combined(), "backend Pulled"), res.Combined())
	})

	t.Run("compose run --env-from-file", func(t *testing.T) {
		res := c.RunDockerComposeCmd(t, "-f", "./fixtures/run-test/compose.yaml", "run", "--env-from-file", "./fixtures/run-test/run.env",
			"front", "env")
		res.Assert(t, icmd.Expected{Out: "FOO=BAR"})
	})

	t.Run("compose run -rm with stop signal", func(t *testing.T) {
		projectName := "run-test"
		res := c.RunDockerComposeCmd(t, "--project-name", projectName, "-f", "./fixtures/ps-test/compose.yaml", "run", "--rm", "-d", "nginx")
		res.Assert(t, icmd.Success)

		res = c.RunDockerCmd(t, "ps", "--quiet", "--filter", "name=run-test-nginx")
		containerID := strings.TrimSpace(res.Stdout())

		res = c.RunDockerCmd(t, "stop", containerID)
		res.Assert(t, icmd.Success)
		res = c.RunDockerCmd(t, "ps", "--all", "--filter", "name=run-test-nginx")
		assert.Assert(t, !strings.Contains(res.Stdout(), "run-test-nginx"), res.Stdout())
	})
}


Review comment (suggestion):
suggestion:  use `--format '{{.Names}}'` in command so output only shows container(s) name

Code after:
	t.Run("--pull", func(t *testing.T) {
		res := c.RunDockerComposeCmd(t, "-f", "./fixtures/run-test/pull.yaml", "down", "--remove-orphans", "--rmi", "all")
		res.Assert(t, icmd.Success)

		res = c.RunDockerComposeCmd(t, "-f", "./fixtures/run-test/pull.yaml", "run", "--pull", "always", "backend")
		assert.Assert(t, strings.Contains(res.Combined(), "backend Pulling"), res.Combined())
		assert.Assert(t, strings.Contains(res.Combined(), "backend Pulled"), res.Combined())
	})

	t.Run("compose run --env-from-file", func(t *testing.T) {
		res := c.RunDockerComposeCmd(t, "-f", "./fixtures/run-test/compose.yaml", "run", "--env-from-file", "./fixtures/run-test/run.env",
			"front", "env")
		res.Assert(t, icmd.Expected{Out: "FOO=BAR"})
	})

	t.Run("compose run -rm with stop signal", func(t *testing.T) {
		projectName := "run-test"
		res := c.RunDockerComposeCmd(t, "--project-name", projectName, "-f", "./fixtures/ps-test/compose.yaml", "run", "--rm", "-d", "nginx")
		res.Assert(t, icmd.Success)

		res = c.RunDockerCmd(t, "ps", "--quiet", "--filter", "name=run-test-nginx")
		containerID := strings.TrimSpace(res.Stdout())

		res = c.RunDockerCmd(t, "stop", containerID)
		res.Assert(t, icmd.Success)
		res = c.RunDockerCmd(t, "ps", "--all", "--filter", "name=run-test-nginx", "--format", "'{{.Names}}'")
		assert.Assert(t, !strings.Contains(res.Stdout(), "run-test-nginx"), res.Stdout())
	})
}