[Buildroot] [PATCH 1/3] package/runc: use BR_PATH to find pkg-config

[Buildroot] [PATCH 1/3] package/runc: use BR_PATH to find pkg-config

[Romain Naour]

When BR2_PACKAGE_LIBSECCOMP is set, go will try to find libseccomp
library using pkg-config. But pkg-config is not in the PATH when go
is called during the runc build.

Set the correct PATH in RUNC_MAKE_ENV.

Fixes:
http://autobuild.buildroot.net/results/fea/fea069fec671b32bffd3d133898d71d45abab552

Signed-off-by: Romain Naour <romain.naour@gmail.com>
Cc: Christian Stewart <christian@paral.in>
---
 package/runc/runc.mk | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/package/runc/runc.mk b/package/runc/runc.mk
index c4e75dd..1396e0c 100644
--- a/package/runc/runc.mk
+++ b/package/runc/runc.mk
@@ -15,7 +15,8 @@ RUNC_GOPATH = "$(@D)/Godeps/_workspace"
 RUNC_MAKE_ENV = $(HOST_GO_TARGET_ENV) \
 	CGO_ENABLED=1 \
 	GOBIN="$(@D)/bin" \
-	GOPATH="$(RUNC_GOPATH)"
+	GOPATH="$(RUNC_GOPATH)" \
+	PATH=$(BR_PATH)
 
 RUNC_GLDFLAGS = \
 	-X main.gitCommit=$(RUNC_VERSION) \
-- 
2.5.5

[Buildroot] [PATCH 2/3] package/runc: fix shared only build when libseccomp is enabled.

[Romain Naour]

When BR2_PACKAGE_LIBSECCOMP is set, the go build system will try to
find static library libseccomp.a since "static_build" and
"extldflags '-static'" are used.

Link runc statically only for static only build and prefer dynamic
linking otherwise.

Signed-off-by: Romain Naour <romain.naour@gmail.com>
Cc: Christian Stewart <christian@paral.in>
---
 package/runc/runc.mk | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/package/runc/runc.mk b/package/runc/runc.mk
index 1396e0c..66f234f 100644
--- a/package/runc/runc.mk
+++ b/package/runc/runc.mk
@@ -19,10 +19,14 @@ RUNC_MAKE_ENV = $(HOST_GO_TARGET_ENV) \
 	PATH=$(BR_PATH)
 
 RUNC_GLDFLAGS = \
-	-X main.gitCommit=$(RUNC_VERSION) \
-	-extldflags '-static'
+	-X main.gitCommit=$(RUNC_VERSION)
 
-RUNC_GOTAGS = cgo static_build
+RUNC_GOTAGS = cgo
+
+ifeq ($(BR2_STATIC_LIBS),y)
+RUNC_GOTAGS += static_build
+RUNC_GLDFLAGS += -extldflags '-static'
+endif
 
 ifeq ($(BR2_PACKAGE_LIBSECCOMP),y)
 RUNC_GOTAGS += seccomp
-- 
2.5.5

[Buildroot] [PATCH 2/3] package/runc: fix shared only build when libseccomp is enabled.

[Thomas Petazzoni]

Hello,

Adding Geoff in Cc.

On Thu,  7 Jul 2016 20:39:36 +0200, Romain Naour wrote:
> When BR2_PACKAGE_LIBSECCOMP is set, the go build system will try to
> find static library libseccomp.a since "static_build" and
> "extldflags '-static'" are used.

Indeed, in BR2_SHARED_LIBS=y builds, libseccomp.a will not be produced.

> Link runc statically only for static only build and prefer dynamic
> linking otherwise.

That's interesting, I thought Go was making static linking *mandatory*.
If that's not the case, then I would indeed prefer if we handled this
like all other packages: static linking only when BR2_STATIC_LIBS.

However, this should be done consistently for all packages: flannel and
docker-containerd also pass -extldflags '-static' unconditionally.

Geoff, Christian, do you see a problem with Romain's proposal?

Thanks,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[Buildroot] [PATCH 2/3] package/runc: fix shared only build when libseccomp is enabled.

[Christian Stewart]

Thomas,

On Fri, Jul 8, 2016 at 12:27 PM Thomas Petazzoni
<thomas.petazzoni@free-electrons.com> wrote: > That's interesting, I thought Go was making static linking *mandatory*.
> If that's not the case, then I would indeed prefer if we handled this
> like all other packages: static linking only when BR2_STATIC_LIBS.
> However, this should be done consistently for all packages: flannel and
docker-containerd also pass -extldflags '-static' unconditionally.
> Geoff, Christian, do you see a problem with Romain's proposal?
In my latest revision of docker-engine I removed the -static flag for CGO
because btrfs would not link for the same reason.
I think it's fine to remove the ?-static? extldflag on everything for Go. I have
had no trouble doing so with docker-engine and I don't see why there would be a
problem with the other packages.
We want to statically link the Go code to other go code, yes, but linking the Go
code to C libraries should be fine dynamically.
Best, Christian
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20160708/d4aec296/attachment.html>

[Buildroot] [PATCH 2/3] package/runc: fix shared only build when libseccomp is enabled.

[Thomas Petazzoni]

Hello,


On Fri, 08 Jul 2016 19:32:41 +0000, Christian Stewart wrote:

> In my latest revision of docker-engine I removed the -static flag for CGO
> because btrfs would not link for the same reason.
> I think it's fine to remove the ?-static? extldflag on everything for Go. I have
> had no trouble doing so with docker-engine and I don't see why there would be a
> problem with the other packages.
> We want to statically link the Go code to other go code, yes, but linking the Go
> code to C libraries should be fine dynamically.

OK. Can we remove unconditionally, or should we still keep it for
BR2_STATIC_LIBS=y builds?

Perhaps we should pass -extldflags '$(TARGET_LDFLAGS)', since
TARGET_LDFLAGS will automatically contain -static when
BR2_STATIC_LIBS=y.

For the specific case of runc, is "RUNC_GOTAGS = static_build" needed
when BR2_STATIC_LIBS=y ?

Thanks,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[Buildroot] [PATCH 2/3] package/runc: fix shared only build when libseccomp is enabled.

[Christian Stewart]

Thomas,


On Fri, Jul 8, 2016 12:50 PM, Thomas Petazzoni thomas.petazzoni at free-electrons.com wrote:OK. Can we remove unconditionally, or should we still keep it for

BR2_STATIC_LIBS=y builds?




Perhaps we should pass -extldflags '$(TARGET_LDFLAGS)', since

TARGET_LDFLAGS will automatically contain -static when

BR2_STATIC_LIBS=y.




That should be fine since extldflags is, I believe, only passed to C related
linkers.
For the specific case of runc, is ?RUNC_GOTAGS = static_build? needed

when BR2_STATIC_LIBS=y ?



That go tag is related to linking Go related code? In the runc code with a quick
grep I see only one inconsequential place it's actually referenced:

Godeps/_workspace/src/github.com/godbus/dbus/homedir_dynamic.go:// +build
!static_build Godeps/_workspace/src/github.com/godbus/dbus/homedir_static.go:// +build
static_build
And in that code I cannot even see the logic between the differences in the two
files, as far as I can tell when doing a static build it just looks up the
current home dir of the user, when doing a dynamic build it does some complex
/etc/passwd checking or the like. Absolutely nonsensical.
I'd just leave the static_build tag in there unconditionally, as it seems to
have little to no effect anyway, and turning it off will actually result in some
more complex and unnecessary home directory detection code being activated.
Best, Christian
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20160708/70ff0c4e/attachment.html>

[Buildroot] [PATCH 2/3] package/runc: fix shared only build when libseccomp is enabled.

[Thomas Petazzoni]

Hello,

Could you fix your e-mail client to quote things properly? In the
below, it is impossible to distinguish the text you are quoting from
the text you have actually written. It makes replying to your e-mails
really complicated.

Thanks,

Thomas

On Fri, 08 Jul 2016 19:55:59 +0000, Christian Stewart wrote:
> Thomas,
> 
> 
> On Fri, Jul 8, 2016 12:50 PM, Thomas Petazzoni thomas.petazzoni at free-electrons.com wrote:OK. Can we remove unconditionally, or should we still keep it for
> 
> BR2_STATIC_LIBS=y builds?
> 
> 
> 
> 
> Perhaps we should pass -extldflags '$(TARGET_LDFLAGS)', since
> 
> TARGET_LDFLAGS will automatically contain -static when
> 
> BR2_STATIC_LIBS=y.
> 
> 
> 
> 
> That should be fine since extldflags is, I believe, only passed to C related
> linkers.
> For the specific case of runc, is ?RUNC_GOTAGS = static_build? needed
> 
> when BR2_STATIC_LIBS=y ?
> 
> 
> 
> That go tag is related to linking Go related code? In the runc code with a quick
> grep I see only one inconsequential place it's actually referenced:
> 
> Godeps/_workspace/src/github.com/godbus/dbus/homedir_dynamic.go:// +build
> !static_build Godeps/_workspace/src/github.com/godbus/dbus/homedir_static.go:// +build
> static_build
> And in that code I cannot even see the logic between the differences in the two
> files, as far as I can tell when doing a static build it just looks up the
> current home dir of the user, when doing a dynamic build it does some complex
> /etc/passwd checking or the like. Absolutely nonsensical.
> I'd just leave the static_build tag in there unconditionally, as it seems to
> have little to no effect anyway, and turning it off will actually result in some
> more complex and unnecessary home directory detection code being activated.
> Best, Christian



-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[Buildroot] [PATCH 2/3] package/runc: fix shared only build when libseccomp is enabled.

[Romain Naour]

Hi Christian, Thomas,

Le 08/07/2016 ? 21:55, Christian Stewart a ?crit :
> Thomas,
> 
> 
> On Fri, Jul 8, 2016 12:50 PM, Thomas Petazzoni
> thomas.petazzoni at free-electrons.com <mailto:thomas.petazzoni@free-electrons.com>
> wrote:
> 
>     OK. Can we remove unconditionally, or should we still keep it for
> 
>     BR2_STATIC_LIBS=y builds?
> 
> 
>     Perhaps we should pass -extldflags '$(TARGET_LDFLAGS)', since
> 
>     TARGET_LDFLAGS will automatically contain -static when
> 
>     BR2_STATIC_LIBS=y.
> 
> 
> That should be fine since extldflags is, I believe, only passed to C related
> linkers.
> 
>     For the specific case of runc, is ?RUNC_GOTAGS = static_build? needed
> 
>     when BR2_STATIC_LIBS=y ?
> 
> 
> That go tag is related to linking Go related code? In the runc code with a quick
> grep I see only one inconsequential place it's actually referenced:
> 
> Godeps/_workspace/src/github.com/godbus/dbus/homedir_dynamic.go:// +build
> !static_build
> Godeps/_workspace/src/github.com/godbus/dbus/homedir_static.go:// +build
> static_build
> 
> And in that code I cannot even see the logic between the differences in the two
> files, as far as I can tell when doing a static build it just looks up the
> current home dir of the user, when doing a dynamic build it does some
> complex /etc/passwd checking or the like. Absolutely nonsensical.
> 
> I'd just leave the static_build tag in there unconditionally, as it seems to
> have little to no effect anyway, and turning it off will actually result in some
> more complex and unnecessary home directory detection code being activated.

Ok, thanks for your explanation.

Actually this patch is a try to fix the libseccomp build issue... I wasn't sure
about using "static_build" only for static builds.
I don't know much the Go build system but it seems possible to link dynamically
with libseccomp.so thanks to cgo being enabled (CGO_ENABLED=1).
Can you test Thomas's proposal (pass -extldflags '$(TARGET_LDFLAGS)') ?

Feel free to respin these patches and continue the work on docker-engine ;)

Best regards,
Romain


> 
> Best,
> Christian
> 

[Buildroot] [PATCH 2/3] package/runc: fix shared only build when libseccomp is enabled.

[Thomas Petazzoni]

Hello,

On Thu,  7 Jul 2016 20:39:36 +0200, Romain Naour wrote:
> When BR2_PACKAGE_LIBSECCOMP is set, the go build system will try to
> find static library libseccomp.a since "static_build" and
> "extldflags '-static'" are used.
> 
> Link runc statically only for static only build and prefer dynamic
> linking otherwise.
> 
> Signed-off-by: Romain Naour <romain.naour@gmail.com>
> Cc: Christian Stewart <christian@paral.in>

I have just sent a patch series that passes -static only when
BR2_STATIC_LIBS=y:

http://patchwork.ozlabs.org/patch/652100/
http://patchwork.ozlabs.org/patch/652101/
http://patchwork.ozlabs.org/patch/652102/

So I've marked this patch as superseded.

Thanks,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[Buildroot] [PATCH 3/3] package/runc: disable for static only build

[Romain Naour]

When build statically, runc try to link against Scrt1.o which is not
provided by a toolchain for static build only.

[...]arm-buildroot-linux-uclibcgnueabi/bin/ld: cannot find Scrt1.o: No such file or directory
collect2: error: ld returned 1 exit status

Add the reverse dependency on docker-containerd.

Remove the static case handling for runc.

Signed-off-by: Romain Naour <romain.naour@gmail.com>
Cc: Christian Stewart <christian@paral.in>
---
 package/docker-containerd/Config.in | 6 ++++--
 package/runc/Config.in              | 5 +++--
 package/runc/runc.mk                | 5 -----
 3 files changed, 7 insertions(+), 9 deletions(-)

diff --git a/package/docker-containerd/Config.in b/package/docker-containerd/Config.in
index 99af917..6615c5f 100644
--- a/package/docker-containerd/Config.in
+++ b/package/docker-containerd/Config.in
@@ -5,6 +5,7 @@ config BR2_PACKAGE_DOCKER_CONTAINERD
 	depends on BR2_TOOLCHAIN_HAS_THREADS
 	depends on BR2_USE_MMU # util-linux
 	depends on BR2_USE_WCHAR # util-linux
+	depends on !BR2_STATIC_LIBS # runc
 	select BR2_PACKAGE_RUNC # runtime dependency
 	select BR2_PACKAGE_UTIL_LINUX # runtime dependency
 	select BR2_PACKAGE_UTIL_LINUX_BINARIES
@@ -16,8 +17,9 @@ config BR2_PACKAGE_DOCKER_CONTAINERD
 
 	  https://github.com/docker/containerd
 
-comment "docker-containerd needs a toolchain w/ threads, wchar"
+comment "docker-containerd needs a toolchain w/ dynamic library, threads, wchar"
 	depends on BR2_PACKAGE_HOST_GO_ARCH_SUPPORTS
 	depends on BR2_PACKAGE_HOST_GO_CGO_LINKING_SUPPORTS
 	depends on BR2_USE_MMU
-	depends on !BR2_TOOLCHAIN_HAS_THREADS || !BR2_USE_WCHAR
+	depends on !BR2_TOOLCHAIN_HAS_THREADS || !BR2_USE_WCHAR \
+		 || BR2_STATIC_LIBS
diff --git a/package/runc/Config.in b/package/runc/Config.in
index fd5dee7..8b6d003 100644
--- a/package/runc/Config.in
+++ b/package/runc/Config.in
@@ -3,13 +3,14 @@ config BR2_PACKAGE_RUNC
 	depends on BR2_PACKAGE_HOST_GO_ARCH_SUPPORTS
 	depends on BR2_PACKAGE_HOST_GO_CGO_LINKING_SUPPORTS
 	depends on BR2_TOOLCHAIN_HAS_THREADS
+	depends on !BR2_STATIC_LIBS
 	help
 	  runC is a CLI tool for spawning and running containers
 	  according to the OCP specification.
 
 	  https://github.com/opencontainers/runc
 
-comment "runc needs a toolchain w/ threads"
+comment "runc needs a toolchain w/ dynamic library, threads"
 	depends on BR2_PACKAGE_HOST_GO_ARCH_SUPPORTS && \
 		BR2_PACKAGE_HOST_GO_CGO_LINKING_SUPPORTS
-	depends on !BR2_TOOLCHAIN_HAS_THREADS
+	depends on !BR2_TOOLCHAIN_HAS_THREADS || BR2_STATIC_LIBS
diff --git a/package/runc/runc.mk b/package/runc/runc.mk
index 66f234f..0032017 100644
--- a/package/runc/runc.mk
+++ b/package/runc/runc.mk
@@ -23,11 +23,6 @@ RUNC_GLDFLAGS = \
 
 RUNC_GOTAGS = cgo
 
-ifeq ($(BR2_STATIC_LIBS),y)
-RUNC_GOTAGS += static_build
-RUNC_GLDFLAGS += -extldflags '-static'
-endif
-
 ifeq ($(BR2_PACKAGE_LIBSECCOMP),y)
 RUNC_GOTAGS += seccomp
 RUNC_DEPENDENCIES += libseccomp host-pkgconf
-- 
2.5.5

[Buildroot] [PATCH 3/3] package/runc: disable for static only build

[Thomas Petazzoni]

Hello,

On Thu,  7 Jul 2016 20:39:37 +0200, Romain Naour wrote:
> When build statically, runc try to link against Scrt1.o which is not
> provided by a toolchain for static build only.
> 
> [...]arm-buildroot-linux-uclibcgnueabi/bin/ld: cannot find Scrt1.o: No such file or directory
> collect2: error: ld returned 1 exit status
> 
> Add the reverse dependency on docker-containerd.
> 
> Remove the static case handling for runc.
> 
> Signed-off-by: Romain Naour <romain.naour@gmail.com>
> Cc: Christian Stewart <christian@paral.in>

I believe this only happens on ARM, because on ARM, Go forces the use
of -pie, for some reason:

        pie := (goarch == "arm" && goos == "linux") || goos == "android"
        if pie { // we need to use -pie for Linux/ARM to get accurate imported sym
                cgoLDFLAGS = append(cgoLDFLAGS, "-pie")
        }

And static linking + PIE isn't supported by uClibc. But on
architectures other than ARM, I believe Go doesn't use -pie, so static
linking with uClibc should work (remains to be tested, though).

For this reason, I believe your patch is not correct as it disables Go
packages for static linking on all architectures, while only ARM is
affected by this problem.

This is yet another situation which makes me think we should add a
"BR2_TOOLCHAIN_SUPPORTS_PIE" hidden boolean.

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[Buildroot] [PATCH 3/3] package/runc: disable for static only build

[Thomas Petazzoni]

Hello,

On Fri, 8 Jul 2016 21:47:12 +0200, Thomas Petazzoni wrote:

> I believe this only happens on ARM, because on ARM, Go forces the use
> of -pie, for some reason:
> 
>         pie := (goarch == "arm" && goos == "linux") || goos == "android"
>         if pie { // we need to use -pie for Linux/ARM to get accurate imported sym
>                 cgoLDFLAGS = append(cgoLDFLAGS, "-pie")
>         }
> 
> And static linking + PIE isn't supported by uClibc. But on
> architectures other than ARM, I believe Go doesn't use -pie, so static
> linking with uClibc should work (remains to be tested, though).
> 
> For this reason, I believe your patch is not correct as it disables Go
> packages for static linking on all architectures, while only ARM is
> affected by this problem.
> 
> This is yet another situation which makes me think we should add a
> "BR2_TOOLCHAIN_SUPPORTS_PIE" hidden boolean.

I'll mark your patch as Changes Requested. We instead need a patch at
the BR2_PACKAGE_HOST_GO_ARCH_SUPPORTS level that makes Go packages
unavailable on ARM + uClibc + static linking. To do this, you should
probably use the BR2_TOOLCHAIN_SUPPORTS_PIE option added by
https://patchwork.ozlabs.org/patch/646956/.

Something like:

config BR2_PACKAGE_HOST_GO_ARCH_SUPPORTS
        bool
        default y
-       depends on BR2_arm || BR2_aarch64 || BR2_i386 || BR2_x86_64 || ...
+       depends on (BR2_arm && BR2_TOOLCHAIN_SUPPORTS_PIE) || BR2_aarch64 || BR2_i386 || BR2_x86_64 || ...

Can you send such a patch?

Thanks,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[Buildroot] [PATCH 1/3] package/runc: use BR_PATH to find pkg-config

[Thomas Petazzoni]

Hello,

On Thu,  7 Jul 2016 20:39:35 +0200, Romain Naour wrote:
> When BR2_PACKAGE_LIBSECCOMP is set, go will try to find libseccomp
> library using pkg-config. But pkg-config is not in the PATH when go
> is called during the runc build.
> 
> Set the correct PATH in RUNC_MAKE_ENV.
> 
> Fixes:
> http://autobuild.buildroot.net/results/fea/fea069fec671b32bffd3d133898d71d45abab552
> 
> Signed-off-by: Romain Naour <romain.naour@gmail.com>
> Cc: Christian Stewart <christian@paral.in>
> ---
>  package/runc/runc.mk | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)

Applied to master, thanks.

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com