GoogleContainerTools / container-debug-support

Language-runtime support files for in-container debugging
Apache License 2.0
93 stars 25 forks source link

Update to delve 1.6.1 #77

Closed 110y closed 2 years ago

110y commented 3 years ago
110y commented 3 years ago

CI job has failed due to dockerhub pull rate limit like below:

unexpected status code https://registry-1.docker.io/v2/library/busybox/manifests/sha256:b5fc1d7b2e4ea86a06b0cf88de915a2c43a99a00b6b3c0af731e5f4c07ae8eff: 429 Too Many Requests - Server message: toomanyrequests: You have reached your pull rate limit. You may increase the limit by authenticating and upgrading: https://www.docker.com/increase-rate-limit
briandealwis commented 3 years ago

Oh dear, Delve 1.6.1 moves the minimum supported version to 1.14.

[go113pod go113app] 2021-07-14T17:42:02Z info layer=debugger launching process with args: [./app]
[go113pod go113app] API server listening at: [::]:56286
[go113pod go113app] Version of Go is too old for this version of Delve (minimum supported version 1.14, suppress this error with --check-go-version=false)
etanshaul commented 3 years ago

Oh dear, Delve 1.6.1 moves the minimum supported version to 1.14.

@briandealwis GoLand 2021.1 (the latest version) allows you to bring in old Go SDKs including 1.13 and earlier.

One option could be to mark 1.14+ as a requirement for debugging with Cloud Code. Do we know how many users out there are still on 1.13 and earlier?

briandealwis commented 3 years ago

Looking into it right now. My priority is supporting the VS Code and GoLand, both of which embed Delve. I think we'll need to hold off until their current releases include Delve 1.6.1.

A possible approach is to include multiple versions of Delve and some configuration options to guide Skaffold to choose an earlier version.

briandealwis commented 3 years ago

We don’t have any data on Go versions in use, and I can’t think of any robust way to find that out (the ago version in a go.mod does not restrict the Go version actually used). We do know that Delve 1.5 raised the minimum to Go 1.13, so our users are at least 1.13.

Another strategy is to support using an existing copy of Delve in the image. Rather than hard- code the path to ˋdlvˋ, we could append the dev location to the PATH and see if another version is found.

briandealwis commented 3 years ago

~Even worse,~ Delve 1.7.0 has been released with support for Go 1.17 and dropping support for Go 1.14.

Well, not worse :-)

hyangah commented 3 years ago

Hi VS Code Go maintainer here.

How much of old version are you talking about? If you need to use go1.14, you can still use delve 1.7, with --check-go-version=false flag. You can shell out dlv to add the extra flag, and place the wrapper in the path. That will probably work for go1.13.

If you need much older versions now I want to understand the use case better - the official Go supports the latest two versions

briandealwis commented 3 years ago

Hi @hyangah — although Go may move on, customers may take more time :-)

Erm, Skaffold itself is still built with Go 1.14 🙄

110y commented 3 years ago

I thought it might be a time to make us be able to specify the exact version of the helper-container image via Skaffold config file...?

hyangah commented 3 years ago

If 1.14 is the oldest version, I think it's most likely ok to just use --check-go-version=false and use the latest delve 1.7.x. That's how goland is handling old go versions now.

If you want to be precise, yes, you upgrade the delve minor version only when you upgrade the go minor version or freeze the container config for old go version. Delve actively maintains only the latest release of Delve, so bugs in the old version of delve won't be fixed.

briandealwis commented 3 years ago

Given that Delve will work with Go 1.14, and most likely support 1.13, I think we could patch our Delve to run with --check-go-version=false similar to https://github.com/go-delve/delve/pull/2684 providing that we emit a warning for unsupported versions. That would avoid breaking existing users but provide a warning, and unblock the newest releases.

briandealwis commented 2 years ago

Closing in favour of #88 to bring in Delve 1.7.2. This uses the strategy in the previous comment, to change Delve's default for --check-go-version to false.