-
Notifications
You must be signed in to change notification settings - Fork 1.5k
sync: fix go1.26 build constraints #12397
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
base: master
Are you sure you want to change the base?
Conversation
|
I think there's also a nonsensical constraint here: gvisor/pkg/sync/runtime_spinning_amd64.s Line 17 in 466fd4e
since go1.26 is true iff go1.25 is true, so logical AND there is pointless. |
yep, I'd overlooked. I'll fix as well later. |
14bb657 to
049a70d
Compare
049a70d to
ccb762a
Compare
|
gentle ping |
Signed-off-by: Koichi Shiraishi <[email protected]>
ccb762a to
02d0f03
Compare
ayushr2
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for the back and forth regarding comments. Was having trouble importing this change. It seems that we have a Copybara transformation which requires that go1.25 be accompanied with // https://go.dev/cl/669235 (1.25) adds a new schedt field prior to nmspinning. and go1.26 be accompanied with // https://go.dev/cl/714800 (1.26) changes size of the schedt.midle field.
Even when using !go1.26, we need the 1.26 comment. If you look, this is the rule that the current codebase is following. I don't agree with it, but defining complex copybara transformations is tough. The transformation currently doesn't understand "don't add comment for !go1.xx".
Could you update your PR with this rule? I will also leave in line comments to clarify more. Without this, the copybara transformations are failing.
|
okay and np! I'm not Googler but I know Copybara is difficult to manage (I heard this from the author of the CUE language), and I'll fix it soon. |
Signed-off-by: Koichi Shiraishi <[email protected]>
|
@ayushr2 is thiss changes are correct? If correctly and need to squash, I'll do that. |
| // limitations under the License. | ||
|
|
||
| // https://go.dev/cl/670497 (1.25) adds a new wait reason, adjusting the value of waitReasonSemacquire. | ||
| // https://go.dev/cl/688335 (1.26) reorders waitreason runtime constants, adjusting the values of waitReasonSemacquire et al. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this comment is wrong, should be // https://go.dev/cl/714800 (1.26) changes size of the schedt.midle field.
|
Could you also squash your commits. |
Fix compile error on
go1.26rc1.There is no error in
pkg/sync/runtime_spinning_go126_amd64.s, butpkg/sync/runtime_spinning_go125_amd64.sis blockinggo1.26, so fix it as well.gvisor/pkg/sync/runtime_spinning_go125_amd64.s
Line 17 in 9f2192b
go envoutput