Skip to content
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

feat: golang / dlv 1.23 support #139

Closed
wants to merge 3 commits into from
Closed

feat: golang / dlv 1.23 support #139

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
eea03ea
feat: golang / dlv 1.23 support
zarenner Sep 4, 2024
93691e6
Fix integration tests for local
zarenner Sep 4, 2024
1589ae2
remove unsupported golang version tests
zarenner Sep 4, 2024
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
4 changes: 1 addition & 3 deletions go/helper-image/Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ ARG BUILDPLATFORM
ARG TARGETOS
ARG TARGETARCH

ARG DELVE_VERSION=1.20.1
ARG DELVE_VERSION=1.23.0

# Patch delve to make defaults for --check-go-version and --only-same-user
# to be set at build time. We must install patch(1) to apply the patch.
Expand All @@ -19,8 +19,6 @@ RUN apt-get update && apt-get install -y --no-install-recommends \
RUN curl --location --output delve.tar.gz https://github.com/go-delve/delve/archive/v$DELVE_VERSION.tar.gz \
&& tar xzf delve.tar.gz \
&& mv delve-$DELVE_VERSION delve-source
COPY delve-*.patch .
RUN patch -p0 -d delve-source < delve-as-options.patch
Comment on lines -22 to -23
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm curious why you decided to just delete this rather than update it to apply to Delve 1.23.0. Are the issues Brian mentioned in this PR no longer relevant? From reading that PR, it sounds to me like this will make the image:

  • not work with binaries compiled with Go >= 1.24, even though it may turn out to be fully compatible. (I'm guessing this is why you had to delete all the tests for the older versions?)
  • not work at all because the connecting user is different. (Perhaps the tests in this repo don't really cover this issue? Or maybe there's some other reason it's no longer relevant?)

Obviously I'm somewhat less concerned about the first bullet, but the second one seems worrying. I'm not super familiar with all this, so maybe I'm overlooking something...


# Produce an as-static-as-possible dlv binary to work on musl and glibc
RUN cd delve-source \
Expand Down
41 changes: 0 additions & 41 deletions go/helper-image/delve-as-options.patch
View file
Open in desktop

This file was deleted.

23 changes: 14 additions & 9 deletions go/skaffold.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,37 +37,42 @@ profiles:

# integration: set of `skaffold debug`-like integration tests
- name: integration
build:
local:
useBuildkit: true
push: false
patches:
- op: add
path: /build/artifacts/-
value:
image: go118app
image: go121app
context: test/goapp
docker:
buildArgs:
GOVERSION: 1.18
GOVERSION: '1.21'
- op: add
path: /build/artifacts/-
value:
image: go119app
image: go122app
context: test/goapp
docker:
buildArgs:
GOVERSION: 1.19
GOVERSION: '1.22'
- op: add
path: /build/artifacts/-
value:
image: go120app
image: go123app
context: test/goapp
docker:
buildArgs:
GOVERSION: '1.20'
GOVERSION: '1.23'

deploy:
kubectl:
manifests:
- test/k8s-test-go118.yaml
- test/k8s-test-go119.yaml
- test/k8s-test-go120.yaml
- test/k8s-test-go121.yaml
- test/k8s-test-go122.yaml
- test/k8s-test-go123.yaml

# release: pushes images to production with :latest
- name: release
Expand Down
91 changes: 0 additions & 91 deletions go/test/k8s-test-go115.yaml
View file
Open in desktop

This file was deleted.

91 changes: 0 additions & 91 deletions go/test/k8s-test-go116.yaml
View file
Open in desktop

This file was deleted.

91 changes: 0 additions & 91 deletions go/test/k8s-test-go117.yaml
View file
Open in desktop

This file was deleted.

Loading