Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

breaking changes from cobra v1.9.0 #5827

Closed
tassa-yoniso-manasi-karoto opened this issue Feb 16, 2025 · 7 comments
Closed

breaking changes from cobra v1.9.0 #5827

tassa-yoniso-manasi-karoto opened this issue Feb 16, 2025 · 7 comments

Comments

@tassa-yoniso-manasi-karoto

Description

cobra v1.9.0 was just released.
I have a program that requires Docker CLI and Cobra. I have just upgraded my dependencies and and I get a compilation error:

Wails CLI v2.10

Executing: go mod tidy
  • Generating bindings: 
  ERROR   
          # github.com/docker/cli/cli/command/completion
          ../../pkg/mod/github.com/docker/cli@v27.5.1+incompatible/cli/command/completion/functions.go:135:9: cannot use cobra.FixedCompletions(options, cobra.ShellCompDirectiveNoFileComp) (value of type cobra.CompletionFunc) as ValidArgsFn value in return statement
          
          exit status 1

There seems to be some breaking changes associated with Cobra.

Reproduce

  1. go get -u
  2. try to compile (in my case with wails dev)

Expected behavior

should compile

docker version

Client:
 Version:           27.5.1
 API version:       1.47
 Go version:        go1.23.4
 Git commit:        tag v27.5.1
 Built:             Mon Jan 27 09:32:48 2025
 OS/Arch:           linux/amd64
 Context:           default

Server:
 Engine:
  Version:          27.5.1
  API version:      1.47 (minimum version 1.24)
  Go version:       go1.23.4
  Git commit:       tag v27.5.1
  Built:            Mon Jan 27 09:32:48 2025
  OS/Arch:          linux/amd64
  Experimental:     false
 containerd:
  Version:          2.0.1
  GitCommit:        UNSET
 runc:
  Version:          1.1.13
  GitCommit:        
 docker-init:
  Version:          0.19.0
  GitCommit:

docker info

Client:
 Version:    27.5.1
 Context:    default
 Debug Mode: false
 Plugins:
  compose: Docker Compose (Docker Inc.)
    Version:  v2.29.1
    Path:     /usr/libexec/docker/cli-plugins/docker-compose

Server:
 Containers: 25
  Running: 0
  Paused: 0
  Stopped: 25
 Images: 22
 Server Version: 27.5.1
 Storage Driver: overlay2
  Backing Filesystem: xfs
  Supports d_type: true
  Using metacopy: false
  Native Overlay Diff: true
  userxattr: false
 Logging Driver: json-file
 Cgroup Driver: cgroupfs
 Cgroup Version: 1
 Plugins:
  Volume: local
  Network: bridge host ipvlan macvlan null overlay
  Log: awslogs fluentd gcplogs gelf journald json-file local splunk syslog
 Swarm: inactive
 Runtimes: io.containerd.runc.v2 runc
 Default Runtime: runc
 Init Binary: docker-init
 containerd version: UNSET
 runc version: 
 init version: 
 Security Options:
  seccomp
   Profile: builtin
 Kernel Version: 6.12.11_1
 Operating System: Void Linux
 OSType: linux
 Architecture: x86_64
 CPUs: 4
 Total Memory: 7.569GiB
 Name: machine
 ID: b84fa389-c9ff-42b0-91af-758a74b846f0
 Docker Root Dir: /var/lib/docker
 Debug Mode: false
 Experimental: false
 Insecure Registries:
  127.0.0.0/8
 Live Restore Enabled: false

Additional Info

No response

@thaJeztah
Copy link
Member

Ugh; thanks for reporting; it looks like Cobra introduced a Completion type, which is an alias for a string, but I suspect Go's way of matching signatures does not account for aliases, so a string (even though it's an alias for Completion) does not satisfy the interface?

// ValidArgsFn a function to be used by cobra command as `ValidArgsFunction` to offer command line completion
type ValidArgsFn func(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective)

https://github.com/spf13/cobra/blob/5f9c40898e795a9fb0fd5ca83b6e05c3720523d1/completions.go#L128-L132

// Completion is a string that can be used for completions
//
// two formats are supported:
//   - the completion choice
//   - the completion choice with a textual description (separated by a TAB).
//
// [CompletionWithDesc] can be used to create a completion string with a textual description.
//
// Note: Go type alias is used to provide a more descriptive name in the documentation, but any string can be used.
type Completion = string

// CompletionFunc is a function that provides completion results.
type CompletionFunc func(cmd *Command, args []string, toComplete string) ([]Completion, ShellCompDirective)

Looks to be introduced in spf13/cobra@8cb30f9

@thaJeztah
Copy link
Member

@tassa-yoniso-manasi-karoto I'm discussing with the Cobra maintainers, but also looking at workarounds on our side; would you be able to test if the changes from this PR fix the build?

It's a draft PR and opened from my fork, so not merged yet (and the branch will go away eventually), but for testing if the fix work, you add a replace rule in your go.mod to use the code from that branch;

// FIXME: testing https://github.com/docker/cli/pull/5828
replace github.com/docker/cli => github.com/thaJeztah/cli v0.0.0-20250216163510-ae3d4db9f658

We likely will have to do that alias either way as there's now a type defined in upstream Cobra, but I wanted to verify if that approach works, so would be great if you could test it at least fixes the issue.

@ccoVeille
Copy link

Follow up, I provided a fix

Thanks again @tassa-yoniso-manasi-karoto for reporting it, and sorry for the inconvenience

@jpmcb
Copy link

jpmcb commented Feb 16, 2025

@thaJeztah apologies for the breaking change: we'll cut v1.9.1 today.

Deeper integration testing is a missing piece of cobras broader test/maintenance strategy. I'd love to explore how we can prevent this type of problem for consumers (and consumers of consumers) in the future.

@thaJeztah
Copy link
Member

Thanks everyone!

Really, no stress: no harm done. Things break at times, and that's a reality of life.

As to integration testing; in general we've not seen problematic issues with Cobra upgrades. Our (docker/cli, moby/moby) repositories are still in a really awkward split not being able (yet) to be an actual go module (we're getting closer, but the situation is ... complex). I definitely wouldn't want to put the burden of testing against those on the Cobra maintainers.

One thing that could help is to do rc's before releases and ask projects that are interested to help verifying to run their CI against those RCs. That said, with Cobra generally not having caused major issues with upgrades, perhaps the current situation isn't too problematic; turnaround time from discovery to fix was really short, and anyone updating dependencies should look at changes before updating (to learn what changes they're accepting with the update), and test in CI.

(The only thing I could see becoming useful at some point is drafting ideas for a "v2"; having looked at some parts of the code it's starting to show its age, and I have a feeling some parts start to stretch the original design, or resulted in some bloat to preserve backward compatibility).

@tassa-yoniso-manasi-karoto
Copy link
Author

I can confirm this can be resolved by either using #5828 or cobra's current main branch.

Thank you for the quick resolution, feel free to close the issue.

@thaJeztah
Copy link
Member

cobra v1.9.1 was tagged, which includes the fix; let me indeed go ahead and close this one. Thanks again for reporting!

# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

No branches or pull requests

4 participants