8000 chore: remove go version specific code by djdongjin · Pull Request #11857 · containerd/containerd · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

chore: remove go version specific code #11857

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.

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

djdongjin
Copy link
Member
@djdongjin djdongjin commented May 14, 2025

Now that we have 1.24.x as go min version, I think we can remove this go code specific to a lower
version.

Depends on

@k8s-ci-robot
Copy link

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@djdongjin djdongjin force-pushed the remove-go-version-specific-code branch from dbc19ad to f194d8c Compare May 28, 2025 02:10
@djdongjin djdongjin marked this pull request as ready for review May 28, 2025 16:51
@dosubot dosubot bot added go Pull requests that update Go code kind/cleanup labels May 28, 2025
@djdongjin djdongjin changed the title chore: remove specific go version code chore: remove go version specific code May 28, 2025
@djdongjin
Copy link
Member Author

/cc @fuweid @thaJeztah since you guys contributed some of these code and may have more context on them. I think we should be good to delete them given we have go 1.24.3 as the min version?

@djdongjin djdongjin requested review from fuweid, thaJeztah and Copilot May 28, 2025 16:53
Copy link
@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR removes Go version–specific workarounds now that Go 1.24 is the minimum supported version, consolidates duplicate logic, and cleans up legacy test files.

  • Drop the Go 1.23 “double close” PIDFD workaround and its runtime import in unshare_linux.go.
  • Standardize on fs.FileInfoToDirEntry and remove Go 1.16/1.17 compatibility tests.
  • Simplify the mount package’s getUsernsFD by removing legacy build tags and updating PIDFD logic.

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.

Show a summary per file
File Description
pkg/sys/unshare_linux.go Removed runtime import and Go 1.23 PIDFD dup-close logic
pkg/oci/utils_unix_test.go Replaced custom fileInfoToDirEntry with fs.FileInfoToDirEntry
pkg/oci/utils_unix_go117_test.go Deleted Go 1.17–specific compatibility shim
pkg/oci/utils_unix_go116_test.go Deleted Go 1.16–specific compatibility shim
core/mount/mount_idmapped_utils_linux_go122.go Deleted Go 1.22–specific mount helpers
core/mount/mount_idmapped_utils_linux.go Updated getUsernsFD to always use PIDFD, removed outdated checks

@github-project-automation github-project-automation bot moved this from Needs Triage to Review In Progress in Pull Request Review Jun 9, 2025
@estesp estesp added this pull request to the merge queue Jun 9, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jun 9, 2025
@fuweid
Copy link
Member
fuweid commented Jun 10, 2025
=== FAIL: integration/client TestContainerPTY (5.99s)
    container_test.go:2741: output `` does not contain \x00, trying again
    container_test.go:2741: output `` does not contain \x00, trying again
    container_test.go:2739: expected \x00 in output

hmm this issue shows up again

@fuweid fuweid added this pull request to the merge queue Jun 10, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jun 10, 2025
Now that we have 1.24.x as go min version, I think
we can remove this go code specific to a lower
version.

Signed-off-by: Jin Dong <djdongjin95@gmail.com>
@djdongjin djdongjin force-pushed the remove-go-version-specific-code branch from f194d8c to 734d52c Compare June 13, 2025 01:35
Copy link
Member
@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

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

LGTM

indeed, as the module no longer allows older Go versions, this will just be dead code.

@estesp estesp added this pull request to the merge queue Jun 13, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jun 13, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
go Pull requests that update Go code kind/cleanup size/L
Projects
Status: Review In Progress
Development

Successfully merging this pull request may close these issues.

5 participants
0