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

syscall: use Docker container for making auto-generated files #15282

Open
bradfitz opened this issue Apr 13, 2016 · 26 comments
Open

syscall: use Docker container for making auto-generated files #15282

bradfitz opened this issue Apr 13, 2016 · 26 comments
Labels
compiler/runtime Issues related to the Go compiler and/or runtime.
Milestone

Comments

@bradfitz
Copy link
Contributor

The auto-generation of the syscall/z*.go files is a mess.

Their generation depends on the headers installed on the host system, so nobody can reliably generate them without making a bunch of accidental unrelated changes.

This applies equally to pkg syscall and to golang.org/x/sys/unix.

Let's use a Dockerfile in there and then run mkwhatever.{sh,pl,go} file in Docker, and then docker cp the auto-generated files out of the container back to the host.

Recent chaos:

It's less clear what to do about other Unix, like OpenBSD (https://go-review.googlesource.com/#/c/21797/, etc). @mdempsky? Jails? But openbsd is tricky for other reasons: #15227

Windows seems to be fine.

/cc @ianlancetaylor

@bradfitz bradfitz self-assigned this Apr 13, 2016
@bradfitz bradfitz added this to the Unplanned milestone Apr 13, 2016
@mdempsky
Copy link
Member

mdempsky commented Apr 14, 2016

I don't think there's anything fundamental to generating OpenBSD's z*.go files that requires running on an OpenBSD host. It looks like we just need access to the kernel headers, which we could get by either downloading from OpenBSD's cvsweb server or grabbing an appropriate install or source tarball.

The same looks to be true of the other mksysnum_*.pl scripts.

@hirochachacha
Copy link
Contributor

How do you plan to support non-amd64 architectures?

@bradfitz
Copy link
Contributor Author

bradfitz commented Mar 2, 2017

@hirochachacha, we're supposed to have builders for all Go architectures, and builders are supposed to be hermetic. So really we could/should just be using the builders to generate these. Dmitry generated the race runtime *.so files using the builders for various architectures. We use the builders to generate releases. Seems doable here too.

@hirochachacha
Copy link
Contributor

@bradfitz Thanks. Are those builders are open to individual developers?
I think this is one of the head aching problem who wants to contribute to x/sys/unix.
For example, current x/sys/unix supports 12 architectures for linux.
If they want to add a new syscall, it's easy. they can use mksyscall.pl directly.
Once, they need to add a constant which is depends on kernel headers,
libc headers, per architecture, the problem is occurred.
It's not a trivial work for individual developers to prepare these environments.
That's the reason I created https://go-review.googlesource.com/#/c/20007/ before.

@hirochachacha
Copy link
Contributor

(Sorry, conclusions are missing)

Although 12 (virtual) machines => 12 cross compilers will reduce some costs, it's still painful.
So, I want a promising alternative.

@bradfitz
Copy link
Contributor Author

bradfitz commented Mar 6, 2017

Are those builders are open to individual developers?

gomote access is available to those who ask and we recognize as being regular contributors.

So, I want a promising alternative.

Got a proposal?

@hirochachacha
Copy link
Contributor

gomote access is available to those who ask and we recognize as being regular contributors.

I see.

Got a proposal?

Yeah, I'm convinced, thank you.

@gopherbot
Copy link

CL https://golang.org/cl/37943 mentions this issue.

@bradfitz
Copy link
Contributor Author

bradfitz commented Mar 8, 2017

/cc @josephlr who did https://golang.org/cl/37943

@josephlr josephlr self-assigned this Mar 8, 2017
@josephlr
Copy link
Member

josephlr commented Mar 9, 2017

Updated https://golang.org/cl/37943 with the basic design of how we should go about doing this in general. The steps should look something like:

  1. Download necessary sources from the most recent stable version
  2. Download go, all the necessary tools to build the source, the emulator, and cross compilers.
  3. For each target, set GOARCH and GOOS to the right values. Then, generate the files normally, except:
    a. Use the correct cross compiler for gcc or go build
    b. Use the correct emulator when running any binaries (./myprog or go run)

Also, @bradfitz suggested docker cp to get the files out of the Docker container. This CL instead uses a shared directory (docker run -d) to both put the files in and take them out. The shared approach seemed simpler, but I'd like any feedback.

@bradfitz
Copy link
Contributor Author

bradfitz commented Mar 9, 2017

Also, @bradfitz suggested docker cp to get the files out of the Docker container. This CL instead uses a shared directory (docker run -d) to both put the files in and take them out.

The advantage of docker cp is that it is more portable. The shared directory bind mounts only work if your Docker client & server are on the same machine (and both Linux), precluding updating this from Mac/Windows running Docker there.

At least, that's how it used to be. I haven't really kept up with latest Docker. If you want to use -d, please verify it all works on macOS too.

@mdlayher
Copy link
Member

mdlayher commented Mar 9, 2017

Just gave it a run for the first time on my linux/amd64 host. It works great!! I glanced at the diff and it appears that there are quite a few constants that are currently outdated or incorrect in master for various architectures.

Well done. I'll review the code now.

@josephlr
Copy link
Member

josephlr commented Mar 10, 2017

@bradfitz, it seems like the shared directories work on macOS and Windows, so it should be possible to now generate all the Linux go files from a macOS or Windows system. It seems like -d is at least portable enough, and it allows for hacking on the go files without any rebuilding (not that big of a deal though).

@josephlr
Copy link
Member

Most of the changes introduced to the z*_linux*.go files will be purely additive with the following exceptions:

  • I changed mkpost.go to be consistent across architectures, so now fields marked as __unused by glibc or pad fields generated by cgo -godefs are no longer exported.
  • cgo -godefs has changed to no longer have zero-length fields like foo byte[0].
  • Various constants that refer to the number of some quantity are incremented (e.g. AF_MAX or RTM_NR_FAMILIES).
  • Some mask constants have had additional higher significance bits added (e.g. MS_RMT_MASK going from 0x800051 to 0x2800051).
  • Some constants are private and shouldn't have been exposed to userspace (e.g. IFF_802_1Q_VLAN).
  • Some contants were just removed by the kernel interface (e.g. PTRACE_SEIZE_DEVEL)
  • Some constants either had incorrect values or never existed at all (e.g. EPOLL_NONBLOCK).

These are a lot of changes, would be easier to submit diffs on a per-architecture basis, or by the type of change (i.e. one diff fixes all the IFF_* constants, one diff removes all bad constants, etc...), or with one enormous change?

@josephlr
Copy link
Member

Also, what should we do about sparc64? The go compiler doesn't even support that archetecture, so I don't even know how the z*_linux_sparc64.go files were even generated.

@ianlancetaylor
Copy link
Contributor

Changes existing exported pad fields to no longer be exported is likely to break existing programs. While clearly those programs should not be referring directly to padding fields, in fact they do.

@ianlancetaylor
Copy link
Contributor

In general while for x/sys/unix we can make things right, for syscall we need to preserve existing names even if they seem wrong to us.

@ianlancetaylor
Copy link
Contributor

I think that diffs by architecture will be simplest.

I assume the sparc64 information in x/sys/unix was built using gccgo.

@josephlr
Copy link
Member

That seems reasonable: change them in x/sys/unix but not in syscall. Would we even want this sort of process for syscall as it is "locked down"?

@bradfitz
Copy link
Contributor Author

That seems reasonable: change them in x/sys/unix but not in syscall. Would we even want this sort of process for syscall as it is "locked down"?

It would still be nice for syscall also. We do occasionally still update it for Go's needs. By "locked down" we primarily mean that we don't add to it by user request just to add new specific symbols. But if something in the Go std library needs something, we sometimes do update it.

@josephlr
Copy link
Member

josephlr commented Mar 28, 2017

The main issue I could see is that switching over the building of the z*_linux_*.go files will introduce a one-time large change that could potentially break users. Moving x/sys/unix to the docker build system was mostly additive, but still had deletions (see above). I guess we could try to find the versions of the Linux kernel and glibc that introduce the fewest changes, but we would probably still have some breakage. I guess it just depends if removing things like IFF_802_1Q_VLAN is a "bug fix" or a change to the syscall package interface.

@laboger
Copy link
Contributor

laboger commented Mar 30, 2017

Since this is going to regenerate all the z* files in x/sys/unix, could this include the change that was suggested by @ianlancetaylor in issue #19560 to define Signal and Errno within x/sys/unix and eliminate the need to continue to use syscall.Signal and syscall.Errno? That will allow Go users to (almost) migrate completely from syscall to x/sys/unix.

I don't understand how this proposal works for non-amd64 architectures. The values that are generated and put into the z files are pulled from the host system's header files, which aren't the same for all architectures. So if you are going to use cross compilers you are still reading the header files from the amd64 host.

@ianlancetaylor
Copy link
Contributor

I think we should worry about Signal and Errno separately from this effort.

gopherbot pushed a commit to golang/sys that referenced this issue Apr 21, 2017
Right now the process for adding in new constants, errors, or syscalls
for Linux is a pain and unreliable. The scripts are designed to be run
on the target architecture and use the header files installed on the
user's system. This makes it hard to generate files for all the
architectures or to have consistency between users. See golang/go#15282.

This CL fixes this issue by making all of the files for the 11 supported
architectures directly from source checkouts of Linux, glibc, and bluez.
This is done using Docker, the gcc cross-compilers, and qemu emulation.
Previously discussed here:
    https://go-review.googlesource.com/c/37589/

A README.md file is also added to explain how all the parts of the build
system work.

In order to get the build working for all the architectures, I made
some changes to the other scripts called from mkall_linux.go:
  - Files only used for generating linux code, moved to linux/
  - linux/mksysnum.pl supports a specified CC compiler.
  - The generated C code in mkerrors.sh changed to avoid a warning
  - mkerrors.sh headers changed to fix powerpc64 bug in sys/ioctl.h
  - linux/types.go no longer needs to export Ptrace structs in lowercase

Build instructions:
  - Host system needs to be x86-64 Linux
  - Install Docker (https://docs.docker.com/engine/installation/)
  - ./mkall.sh (That's it!!!)

Change-Id: I87067c14442ba12f8d51991349a43a9d73f38ae0
Reviewed-on: https://go-review.googlesource.com/37943
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
@gopherbot
Copy link

Change https://golang.org/cl/102655 mentions this issue: unix: generate linux/sparc64 go files using Docker

gopherbot pushed a commit to golang/sys that referenced this issue Dec 21, 2018
With cgo supporting sparc64 as of CL 102555 and CL 132155, the Docker
based build system can be used to generate file for linux/sparc64 as
well.

Updates golang/go#15282

Change-Id: I109e55f39d3229b24b0029c42074e439c6c54dc8
Reviewed-on: https://go-review.googlesource.com/c/102655
Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Jul 7, 2022
@bcmills
Copy link
Contributor

bcmills commented Nov 7, 2023

Since syscall is no longer deprecated (#60797), it seems like maybe we should reprioritize this technical debt in the package — it makes me uncomfortable to review CLs with manual changes in files that are clearly marked DO NOT EDIT. 😅

@bcmills
Copy link
Contributor

bcmills commented Nov 7, 2023

For example, it appears that a few constants were added to types_linux.go in https://go.dev/cl/407694, but as far as I can tell were not propagated to the ztypes files for arches other than loong64.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler/runtime Issues related to the Go compiler and/or runtime.
Projects
None yet
Development

No branches or pull requests

9 participants