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.

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

Jump to bottom

cmd/cgo: request to backport loong64 cmd/cgo CL 342305 to 1.18 branch #52419

Closed
ianlancetaylor opened this issue Apr 18, 2022 · 6 comments
Closed

cmd/cgo: request to backport loong64 cmd/cgo CL 342305 to 1.18 branch #52419

ianlancetaylor opened this issue Apr 18, 2022 · 6 comments
Labels
FrozenDueToAge NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. release-blocker
Milestone
Go1.18.2

Comments

@ianlancetaylor
Copy link
Contributor

The x/sys repo uses a script to generate various files. That script requires that cmd/cgo support the GOARCH value being generated. The script normally runs with a released version of Go.

The loong64 port requires adding loong64 support files to the x/sys repo. These files are then vendored into cmd/vendor/golang.org/x/sys (yes, it's unfortunate that we vendor all of x/sys/unix into the main repo, but that is a problem for another day).

So we have a chicken vs. egg problem with the loong64 port: we have to get cgo to support loong64 in order to build x/sys/unix, and we need x/sys/unix in order to complete the loong64 port.

The cgo fix is small and harmless: https://go.dev/cl/342305.

I would like to request that we submit that CL to the main branch, and backport it to the 1.18 branch, so that it will be in the 1.18.2 release. That will get us back to a reasonably clean state.

In the meantime https://go.dev/cl/399336 generates the x/sys loong64 files with a patched version of Go. We can do that temporarily but we would like that to be as temporary as possible.

CC @dr2chase @cherrymui @golang/release
@ianlancetaylor ianlancetaylor added this to the Go1.18.2 milestone Apr 18, 2022
@thanm thanm added the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Apr 19, 2022
@cherrymui
Copy link
Member

I think it is okay to submit to the main branch. I'm not sure why it needs to be backported to 1.18. Does the build script require to use the release branch? Can we make it use tip?

It looks like the CL also patches the kernel, libc, etc. So removing a patch to Go doesn't seem to reduce the temporariness much.

@ianlancetaylor
Copy link
Contributor Author

The x/sys/unix build normally uses a Go release. It's GOLANG_VERSION in x/sys/unix/linux/Dockerfile. It would be nice to be able to keep doing that.

The kernel patch will also go away when the loong64 support is merged into the Linux kernel, which will supposedly happen with the 5.19 kernel release. The x/sys/unix script currently uses kernel version 5.16.

I agree that none of this is essential, it's just a little bit easier to work with and faster to run the script.

@cherrymui
Copy link
Member

Okay, thanks. I think backporting is probably fine. The CL is safe and minimal. (Although this is a bit unusual for backporting)

@ianlancetaylor
Copy link
Contributor Author

Thanks.

@limeidan
Copy link
Contributor

Hi all,

I did a test of backporting the CL 342305 to go1.18, but it doesn't work

----- GENERATING: loong64 -----
configure: WARNING: minimum kernel version reset to 5.15.0
header files generated
zsysnum file generated
zsyscall file generated
cgo: cannot load DWARF output from _obj/_cgo_.o: applyRelocations: not implemented
could not make ztypes file: exit status 1
***** FAILURE:    loong64 *****

Tested and found that it needs another CL https://go-review.googlesource.com/c/go/+/396735/3

@ianlancetaylor
Copy link
Contributor Author

Thanks for trying it out. This has gotten complicated enough that I know think that we should just wait for the 1.19 release to update the x/sys/unix rebuild scripts (in the hopes that 1.19 does include loong64 support.) I'm withdrawing this suggestion.

@golang golang locked and limited conversation to collaborators Apr 20, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. release-blocker
Projects
None yet
Milestone
Go1.18.2
Development

No branches or pull requests
5 participants
@ianlancetaylor @gopherbot @thanm @cherrymui @limeidan