| Message ID | 62cf6f9a00fe67cf999e45f7cbf4bf2e295a0fd5.1463010771.git.geoff@infradead.org |
|---|---|
| State | Changes Requested |
| Headers | show |
Hello, On Thu, 12 May 2016 00:08:46 +0000, Geoff Levand wrote: > CGO needs thread support. Fixes build errors like these: > > warning requested reentrant code, but thread support was disabled > > Signed-off-by: Geoff Levand <geoff@infradead.org> When a commit fixes some build issues reported by the autobuilders, please add something like: Fixes: http://autobuild.buildroot.net/results/0f0f5892e474bdf9145242cee836b8acc383734f/ in the commit log. Thanks! Thomas
Hello, On Thu, 12 May 2016 00:08:46 +0000, Geoff Levand wrote: > CGO needs thread support. Fixes build errors like these: > > warning requested reentrant code, but thread support was disabled > > Signed-off-by: Geoff Levand <geoff@infradead.org> > --- > package/go/Config.in.host | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/package/go/Config.in.host b/package/go/Config.in.host > index 094e402..e988483 100644 > --- a/package/go/Config.in.host > +++ b/package/go/Config.in.host > @@ -1,5 +1,6 @@ > config BR2_PACKAGE_HOST_GO_ARCH_SUPPORTS > bool > default y > + depends on BR2_TOOLCHAIN_HAS_THREADS This is not an architecture dependency, it's a toolchain dependency. We normally show a comment about such a dependency so that users know they need to enable thread support in their toolchain. When you say "CGO needs thread support", is CGO the go runtime on the target? Does this means that any Go program needs threads? Thomas
Hi, On Thu, 2016-05-12 at 16:12 +0200, Thomas Petazzoni wrote: > On Thu, 12 May 2016 00:08:46 +0000, Geoff Levand wrote: > > CGO needs thread support. Fixes build errors like these: > > > > warning requested reentrant code, but thread support was disabled > > > > Signed-off-by: Geoff Levand <geoff@infradead.org> > > --- > > package/go/Config.in.host | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/package/go/Config.in.host b/package/go/Config.in.host > > index 094e402..e988483 100644 > > --- a/package/go/Config.in.host > > +++ b/package/go/Config.in.host > > @@ -1,5 +1,6 @@ > > config BR2_PACKAGE_HOST_GO_ARCH_SUPPORTS > > > > > > bool > > > > > > default y > > +> > > > depends on BR2_TOOLCHAIN_HAS_THREADS > > This is not an architecture dependency, it's a toolchain dependency. We > normally show a comment about such a dependency so that users know they > need to enable thread support in their toolchain. OK, so a Config.host.in like this? config BR2_PACKAGE_HOST_GO_ARCH_SUPPORTS bool default y depends on BR2_arm || BR2_aarch64 || BR2_i386 || BR2_x86_64 || BR2_powerpc64 || BR2_powerpc64le || BR2_mips64 || BR2_mips64el depends on !BR2_ARM_CPU_ARMV4 comment "go needs a toolchain w/ threads" depends on !BR2_TOOLCHAIN_HAS_THREADS depends on BR2_PACKAGE_HOST_GO_ARCH_SUPPORTS > When you say "CGO needs thread support", is CGO the go runtime on the > target? Does this means that any Go program needs threads? cgo is the C language call support and has a runtime component (the cgo package). The host go compiler and some host tools use pthreads, and so are linked with libpthread. If a target go program uses the cgo package, it will need to be linked with libpthread. -Geoff
Hello, On Thu, 12 May 2016 11:32:04 -0700, Geoff Levand wrote: > OK, so a Config.host.in like this? > > config BR2_PACKAGE_HOST_GO_ARCH_SUPPORTS > bool > default y > depends on BR2_arm || BR2_aarch64 || BR2_i386 || BR2_x86_64 || BR2_powerpc64 || BR2_powerpc64le || BR2_mips64 || BR2_mips64el > depends on !BR2_ARM_CPU_ARMV4 > > comment "go needs a toolchain w/ threads" > depends on !BR2_TOOLCHAIN_HAS_THREADS > depends on BR2_PACKAGE_HOST_GO_ARCH_SUPPORTS No, because we don't show comments for host packages (since most of them are not selectable in menuconfig). Unfortunately, the only solution that I see now is that *target* go packages (such as flannel) should have this dependency on BR2_TOOLCHAIN_HAS_THREADS + the comment. All in all, I am in fact not super happy with how we're handling this, but I don't really see how to do it better. One option would be to make the go package a target package rather than a host package. But it's also weird because it basically installs nothing to the target. > > When you say "CGO needs thread support", is CGO the go runtime on the > > target? Does this means that any Go program needs threads? > > cgo is the C language call support and has a runtime component (the > cgo package). Are all Go packages going to use "cgo" ? Or are pure Go software potentially not using cgo ? > The host go compiler and some host tools use pthreads, and so are > linked with libpthread. The fact that the host go compiler uses threads is irrelevant. The BR2_TOOLCHAIN_HAS_THREADS boolean indicate whether there is thread support or not on the *target*. You can basically assume that thread support is always available on the host. > If a target go program uses the cgo package, it will need to be > linked with libpthread. So if I understand correctly, it's only a subset of target Go programs that need thread support? Thomas
On Fri, 2016-05-13 at 15:35 +0200, Thomas Petazzoni wrote: > Hello, > > On Thu, 12 May 2016 11:32:04 -0700, Geoff Levand wrote: > > > OK, so a Config.host.in like this? > > > > config BR2_PACKAGE_HOST_GO_ARCH_SUPPORTS > > bool > > default y > > depends on BR2_arm || BR2_aarch64 || BR2_i386 || BR2_x86_64 || > > BR2_powerpc64 || BR2_powerpc64le || BR2_mips64 || BR2_mips64el > > depends on !BR2_ARM_CPU_ARMV4 > > > > comment "go needs a toolchain w/ threads" > > depends on !BR2_TOOLCHAIN_HAS_THREADS > > depends on BR2_PACKAGE_HOST_GO_ARCH_SUPPORTS > > No, because we don't show comments for host packages (since most of > them are not selectable in menuconfig). OK. > Unfortunately, the only solution that I see now is that *target* go > packages (such as flannel) should have this dependency on > BR2_TOOLCHAIN_HAS_THREADS + the comment. That seems like it will work. > All in all, I am in fact not super happy with how we're handling > this, > but I don't really see how to do it better. One option would be to > make > the go package a target package rather than a host package. But it's > also weird because it basically installs nothing to the target. > > > > When you say "CGO needs thread support", is CGO the go runtime on > > > the > > > target? Does this means that any Go program needs threads? > > > > cgo is the C language call support and has a runtime component (the > > cgo package). > > Are all Go packages going to use "cgo" ? Or are pure Go software > potentially not using cgo ? No, see below. > The host go compiler and some host tools use pthreads, and so are > > linked with libpthread. > > The fact that the host go compiler uses threads is irrelevant. The > BR2_TOOLCHAIN_HAS_THREADS boolean indicate whether there is thread > support or not on the *target*. You can basically assume that thread > support is always available on the host. > > > If a target go program uses the cgo package, it will need to be > > linked with libpthread. > > So if I understand correctly, it's only a subset of target Go > programs > that need thread support? Yes, if they don't use the cgo package, they shouldn't need thread support, so the solution seems to be to just leave it to the target package to specify thread support is needed. I'll make a non-cgo test package to verify. -Geoff
diff --git a/package/go/Config.in.host b/package/go/Config.in.host index 094e402..e988483 100644 --- a/package/go/Config.in.host +++ b/package/go/Config.in.host @@ -1,5 +1,6 @@ config BR2_PACKAGE_HOST_GO_ARCH_SUPPORTS bool default y + depends on BR2_TOOLCHAIN_HAS_THREADS depends on BR2_arm || BR2_aarch64 || BR2_i386 || BR2_x86_64 || BR2_powerpc depends on !BR2_ARM_CPU_ARMV4
CGO needs thread support. Fixes build errors like these: warning requested reentrant code, but thread support was disabled Signed-off-by: Geoff Levand <geoff@infradead.org> --- package/go/Config.in.host | 1 + 1 file changed, 1 insertion(+)