-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
Makefile: fixup for Go 1.23 #4288
Conversation
It is no longer possible to compile runc with rlimit hack from commit da68c8e with Go 1.23 (devel at the moment), it errors out: > link: github.com/opencontainers/runc/libcontainer/system: invalid reference to syscall.origRlimitNofile Go 1.23 implements some restrictions wrt accessing internal symbols, and to work around those, we need to add a checklinkname=0 linker flag. Alas, this flag is not supported in older versions, so the logic is added to check go version and add the flag conditionally. This fixes the issue. Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Do we need to discuss with the Go maintainers for a more official workaround for our issue? (or has there been, and it has been rejected?) |
I'm going to file a bug to Go once I have a neat repro. Alas I can't think of a clear fix (otherwise I'd open a CL right away). |
I think golang should fix their own issues before rejecting something. |
In my opinion, golang is a basic code language, it should let these bump actions controlled by user, a language and it’s standard library should provide as many functions as possible to users, but not silently doing something that can’t controlled by users. |
So maybe we can open a PR to golang according to the above thought. |
The history of bumping rlimit_nofile is quite long, and I don't think they will accept a revert, as such revert will accept quite a lot of users. OTOH clearly demonstrating the problem to go devs and asking for a solution might be the way to go. |
@thaJeztah thanks for linking that, I wasn't aware! This runc usafe seems to be affected too: runc/libcontainer/utils/utils_unix.go Line 112 in 68564ee
Oh, it is part of this one already, sorry! https://go-review.googlesource.com/c/go/+/587219/2 |
Kir mentioned those changes in the linked ticket, and I found the ticket from those review comments 😄 And, yes, if there would be a way to do this without the |
Please wait for https://go.dev/cl/587219 instead of adding this to your makefile. If you add this to your makefile then there is nothing stopping runc from depending on more internal details via linkname. We are going to be far less sympathetic to keeping compatibility in the future for programs using that flag. |
Thanks for looking Russ, yes, that was my concern a bit; this option would be a bit of a big hammer; not great indeed. |
Closing in favor of #4290 (a different way will be used in Go 1.23 if https://go-review.googlesource.com/c/go/+/588076 is going to be merged). |
It is no longer possible to compile runc with rlimit hack from commit da68c8e with Go 1.23 (devel at the moment), it errors out:
Go 1.23 implements some restrictions wrt accessing internal symbols, and to work around those, we need to add a checklinkname=0 linker flag.
Alas, this flag is not supported in older versions, so the logic is added to check go version and add the flag conditionally.
Fixes: #4287