Skip to content

chore: inline needlessly extracted single-use helpers #14048

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.

Sign up for GitHub

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
ndeloof merged 2 commits into from
Aug 14, 2026
Merged

chore: inline needlessly extracted single-use helpers #14048

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b6c9584
chore: inline needless single-use helpers
ndeloof Aug 14, 2026
a5992f6
chore: honest name for navigation-menu resolution, share setServiceScale
ndeloof Aug 14, 2026
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
8 changes: 1 addition & 7 deletions cmd/compose/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ func runConfig(ctx context.Context, dockerCli command.Cli, opts configOptions, s
}

if !opts.noInterpolate {
content = escapeDollarSign(content)
content = bytes.ReplaceAll(content, []byte{'$'}, []byte{'$', '$'})
}

if opts.quiet {
Expand Down Expand Up @@ -696,9 +696,3 @@ func runEnvironment(ctx context.Context, dockerCli command.Cli, opts configOptio
}
return nil
}

func escapeDollarSign(marshal []byte) []byte {
dollar := []byte{'$'}
escDollar := []byte{'$', '$'}
return bytes.ReplaceAll(marshal, dollar, escDollar)
}
11 changes: 4 additions & 7 deletions cmd/compose/create.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ func (opts createOptions) GetTimeout() *time.Duration {

func (opts createOptions) Apply(project *types.Project) error {
if opts.pullChanged {
if !opts.isPullPolicyValid() {
if !slices.Contains(validPullPolicies, opts.Pull) {
return fmt.Errorf("invalid --pull option %q", opts.Pull)
}
for i, service := range project.Services {
Expand Down Expand Up @@ -214,10 +214,7 @@ func applyScaleOpts(project *types.Project, opts []string) error {
return nil
}

func (opts createOptions) isPullPolicyValid() bool {
pullPolicies := []string{
types.PullPolicyAlways, types.PullPolicyNever, types.PullPolicyBuild,
types.PullPolicyMissing, types.PullPolicyIfNotPresent,
}
return slices.Contains(pullPolicies, opts.Pull)
var validPullPolicies = []string{
types.PullPolicyAlways, types.PullPolicyNever, types.PullPolicyBuild,
types.PullPolicyMissing, types.PullPolicyIfNotPresent,
}
21 changes: 8 additions & 13 deletions cmd/compose/list.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,14 @@ func runList(ctx context.Context, dockerCli command.Cli, backendOptions *Backend
return nil
}

view := viewFromStackList(stackList)
view := make([]stackView, len(stackList))
for i, s := range stackList {
view[i] = stackView{
Name: s.Name,
Status: strings.TrimSpace(s.Status + " " + s.Reason),
ConfigFiles: s.ConfigFiles,
}
}
return formatter.Print(view, lsOpts.Format, dockerCli.Out(), func(w io.Writer) {
for _, stack := range view {
_, _ = fmt.Fprintf(w, "%s\t%s\t%s\n", stack.Name, stack.Status, stack.ConfigFiles)
Expand All @@ -131,15 +138,3 @@ type stackView struct {
Status string
ConfigFiles string
}

func viewFromStackList(stackList []api.Stack) []stackView {
retList := make([]stackView, len(stackList))
for i, s := range stackList {
retList[i] = stackView{
Name: s.Name,
Status: strings.TrimSpace(fmt.Sprintf("%s %s", s.Status, s.Reason)),
ConfigFiles: s.ConfigFiles,
}
}
return retList
}
15 changes: 1 addition & 14 deletions cmd/compose/options.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ import (
"os"
"slices"
"sort"
"strings"
"text/tabwriter"

"github.com/compose-spec/compose-go/v2/cli"
Expand Down Expand Up @@ -177,7 +176,7 @@ func promptForInterpolatedVariables(ctx context.Context, dockerCli command.Cli,
}

func extractInterpolationVariablesFromModel(ctx context.Context, dockerCli command.Cli, projectOptions *ProjectOptions, cmdEnvs []string) ([]varInfo, bool, error) {
cmdEnvMap := extractEnvCLIDefined(cmdEnvs)
cmdEnvMap := types.NewMappingWithEquals(cmdEnvs).ToMapping()

// Create a model without interpolation to extract variables
opts := configOptions{
Expand Down Expand Up @@ -229,18 +228,6 @@ func extractInterpolationVariablesFromModel(ctx context.Context, dockerCli comma
return varsInfo, false, nil
}

func extractEnvCLIDefined(cmdEnvs []string) map[string]string {
// Parse command-line environment variables
cmdEnvMap := make(map[string]string)
for _, env := range cmdEnvs {
key, val, ok := strings.Cut(env, "=")
if ok {
cmdEnvMap[key] = val
}
}
return cmdEnvMap
}

func displayInterpolationVariables(writer io.Writer, varsInfo []varInfo) {
// Display all variables in a table format
_, _ = fmt.Fprintln(writer, "\nFound the following variables in configuration:")
Expand Down
15 changes: 11 additions & 4 deletions cmd/compose/scale.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,17 +93,24 @@ func runScale(ctx context.Context, dockerCli command.Cli, backendOptions *Backen
}

for key, value := range serviceReplicaTuples {
service, err := project.GetService(key)
if err != nil {
if err := setServiceScale(project, key, value); err != nil {
return err
}
service.SetScale(value)
project.Services[key] = service
}

return backend.Scale(ctx, project, api.ScaleOptions{Services: services})
}

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
}
Comment on lines +104 to +112

@thaJeztah thaJeztah Aug 14, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks like this is used in two places, but possibly even worth considering inlining it in both places;

@ndeloof ndeloof Aug 14, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I would rather keep this one: it has two real callers (applyScaleOpts in create.go and runScale), and the value-semantics dance (GetService → SetScale → write back into project.Services[name]) is easy to get wrong — forgetting the write-back is a silent no-op. Before this PR runScale had exactly that duplication, drifting from the helper. Real reuse is the boundary this PR tries to preserve; happy to inline both if you feel strongly about it.

@thaJeztah thaJeztah Aug 14, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Ah, right yeah, the project.Services[name] = service may be easily overlooked. Yup, that's fair, no problem!

@thaJeztah thaJeztah Aug 14, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

(I was actually looking; couldn't we just iterate over project.Services, but there was also "disabled services" etc to take into account).


func parseServicesReplicasArgs(args []string) (map[string]int, error) {
serviceReplicaTuples := map[string]int{}
for _, arg := range args {
Expand Down
14 changes: 2 additions & 12 deletions cmd/compose/up.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ func (opts upOptions) apply(project *types.Project, services []string) (*types.P
return project, nil
}

func (opts *upOptions) validateNavigationMenu(dockerCli command.Cli) {
func (opts *upOptions) resolveNavigationMenu(dockerCli command.Cli) {
if !dockerCli.Out().IsTerminal() {
opts.navigationMenu = false
return
Expand Down Expand Up @@ -135,7 +135,7 @@ func upCommand(p *ProjectOptions, dockerCli command.Cli, backendOptions *Backend
return errors.New("cannot combine --attach and --attach-dependencies")
}

up.validateNavigationMenu(dockerCli)
up.resolveNavigationMenu(dockerCli)

if !p.All && len(project.Services) == 0 {
return fmt.Errorf("no service selected")
Expand Down Expand Up @@ -351,13 +351,3 @@ func runUp(
},
})
}

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
}
6 changes: 1 addition & 5 deletions cmd/display/tty.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ func (t *task) update(e api.Resource) {
t.stop()
}
case api.Working:
t.hasMore()
t.spinner.Restart()
}
t.status = e.Status
t.text = e.Text
Expand All @@ -142,10 +142,6 @@ func (t *task) stop() {
t.spinner.Stop()
}

func (t *task) hasMore() {
t.spinner.Restart()
}

func (t *task) Completed() bool {
switch t.status {
case api.Done, api.Error, api.Warning:
Expand Down
10 changes: 2 additions & 8 deletions pkg/compose/attach.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ import (

"github.com/compose-spec/compose-go/v2/types"
"github.com/moby/moby/api/pkg/stdcopy"
containerType "github.com/moby/moby/api/types/container"
"github.com/moby/moby/client"
"github.com/sirupsen/logrus"

Expand Down Expand Up @@ -55,20 +54,15 @@ func (s *composeService) attach(ctx context.Context, project *types.Project, lis
}

for _, ctr := range containers {
err := s.attachContainer(ctx, ctr, listener)
service := ctr.Labels[api.ServiceLabel]
err := s.doAttachContainer(ctx, service, ctr.ID, getContainerNameWithoutProject(ctr), listener)
if err != nil {
return nil, err
}
}
return containers, nil
}

func (s *composeService) attachContainer(ctx context.Context, container containerType.Summary, listener api.ContainerEventListener) error {
service := container.Labels[api.ServiceLabel]
name := getContainerNameWithoutProject(container)
return s.doAttachContainer(ctx, service, container.ID, name, listener)
}

func (s *composeService) doAttachContainer(ctx context.Context, service, id, name string, listener api.ContainerEventListener) error {
inspect, err := s.apiClient().ContainerInspect(ctx, id, client.ContainerInspectOptions{})
if err != nil {
Expand Down
24 changes: 10 additions & 14 deletions pkg/compose/build_bake.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -586,24 +586,20 @@ func (s *composeService) dryRunBake(cfg bakeConfig) map[string]string {
bakeResponse := map[string]string{}
for name, target := range cfg.Targets {
dryRunUUID := fmt.Sprintf("dryRun-%x", sha1.Sum([]byte(name)))
s.displayDryRunBuildEvent(name, dryRunUUID, target.Tags[0])
s.events.On(api.Resource{
ID: name + " ==>",
Status: api.Done,
Text: fmt.Sprintf("==> writing image %s", dryRunUUID),
})
s.events.On(api.Resource{
ID: name + " ==> ==>",
Status: api.Done,
Text: fmt.Sprintf(`naming to %s`, target.Tags[0]),
})
bakeResponse[name] = dryRunUUID
}
for name := range bakeResponse {
s.events.On(builtEvent(name))
}
return bakeResponse
}

func (s *composeService) displayDryRunBuildEvent(name, dryRunUUID, tag string) {
s.events.On(api.Resource{
ID: name + " ==>",
Status: api.Done,
Text: fmt.Sprintf("==> writing image %s", dryRunUUID),
})
s.events.On(api.Resource{
ID: name + " ==> ==>",
Status: api.Done,
Text: fmt.Sprintf(`naming to %s`, tag),
})
}
87 changes: 32 additions & 55 deletions pkg/compose/create.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -257,10 +257,21 @@ func (s *composeService) getCreateConfigs(ctx context.Context,
inherit *container.Summary,
opts createOptions,
) (createConfigs, error) {
labels, err := s.prepareLabels(opts.Labels, service, number)
labels := opts.Labels
hash, err := ServiceHash(service)
if err != nil {
return createConfigs{}, err
}
labels[api.ConfigHashLabel] = hash
if number > 0 {
// One-off containers are not indexed
labels[api.ContainerNumberLabel] = strconv.Itoa(number)
}
var dependencies []string
for dep, d := range service.DependsOn {
dependencies = append(dependencies, fmt.Sprintf("%s:%s:%t", dep, d.Condition, d.Restart))
}
labels[api.DependenciesLabel] = strings.Join(dependencies, ",")

var runCmd, entrypoint []string
if service.Command != nil {
Expand Down Expand Up @@ -578,26 +589,6 @@ func parseSecurityOpts(p *types.Project, securityOpts []string) ([]string, bool,
return parsed, unconfined, nil
}

func (s *composeService) prepareLabels(labels types.Labels, service types.ServiceConfig, number int) (map[string]string, error) {
hash, err := ServiceHash(service)
if err != nil {
return nil, err
}
labels[api.ConfigHashLabel] = hash

if number > 0 {
// One-off containers are not indexed
labels[api.ContainerNumberLabel] = strconv.Itoa(number)
}

var dependencies []string
for s, d := range service.DependsOn {
dependencies = append(dependencies, fmt.Sprintf("%s:%s:%t", s, d.Condition, d.Restart))
}
labels[api.DependenciesLabel] = strings.Join(dependencies, ",")
return labels, nil
}

// defaultNetworkSettings determines the container.NetworkMode and corresponding network.NetworkingConfig (nil if not applicable).
func defaultNetworkSettings(project *types.Project,
service types.ServiceConfig, serviceIndex int,
Expand Down Expand Up @@ -1338,11 +1329,28 @@ func buildMountOptions(volume types.ServiceVolumeConfig) (*mount.BindOptions, *m
case "bind":
return buildBindOption(volume.Bind), nil, nil, nil
case "volume":
return nil, buildVolumeOptions(volume.Volume), nil, nil
if volume.Volume == nil {
return nil, nil, nil, nil
}
return nil, &mount.VolumeOptions{
NoCopy: volume.Volume.NoCopy,
Subpath: volume.Volume.Subpath,
Labels: volume.Volume.Labels,
// DriverConfig: , // FIXME missing from model ?
}, nil, nil
case "tmpfs":
return nil, nil, buildTmpfsOptions(volume.Tmpfs), nil
if volume.Tmpfs == nil {
return nil, nil, nil, nil
}
return nil, nil, &mount.TmpfsOptions{
SizeBytes: int64(volume.Tmpfs.Size),
Mode: os.FileMode(volume.Tmpfs.Mode),
}, nil
case "image":
return nil, nil, nil, buildImageOptions(volume.Image)
if volume.Image == nil {
return nil, nil, nil, nil
}
return nil, nil, nil, &mount.ImageOptions{Subpath: volume.Image.SubPath}
}
return nil, nil, nil, nil
}
Expand All @@ -1366,37 +1374,6 @@ func buildBindOption(bind *types.ServiceVolumeBind) *mount.BindOptions {
return opts
}

func buildVolumeOptions(vol *types.ServiceVolumeVolume) *mount.VolumeOptions {
if vol == nil {
return nil
}
return &mount.VolumeOptions{
NoCopy: vol.NoCopy,
Subpath: vol.Subpath,
Labels: vol.Labels,
// DriverConfig: , // FIXME missing from model ?
}
}

func buildTmpfsOptions(tmpfs *types.ServiceVolumeTmpfs) *mount.TmpfsOptions {
if tmpfs == nil {
return nil
}
return &mount.TmpfsOptions{
SizeBytes: int64(tmpfs.Size),
Mode: os.FileMode(tmpfs.Mode),
}
}

func buildImageOptions(image *types.ServiceVolumeImage) *mount.ImageOptions {
if image == nil {
return nil
}
return &mount.ImageOptions{
Subpath: image.SubPath,
}
}

// createNetwork creates the given (managed) network with its compose labels and
// config-hash. It is executed as a plan operation (OpCreateNetwork); resolution
// of external networks lives in resolveExternalNetwork, and reuse of legacy
Expand Down
Loading
Loading