Skip to contentSkip to conversation

golang: fix build with oldish codesourcery amd64 toolchain (branch yem/go-west)

PATCH New Yann E. MORIN 3 participants 10 messages Nov 25, 2018 – Dec 3, 2018

lore
Share & tools
lore

Yann E. MORIN

author
#0 lore raw

v2 cover golang: fix build with oldish codesourcery amd64 toolchain (branch yem/go-west)

Hello All!

That toolchain seems to suffer from bug #20006

    https://sourceware.org/bugzilla/show_bug.cgi?id=20006

So, like bug 19615, introduce a config option for 20006, and mask away all golang packages.

Additionally, make golang packages abide by the number of CPUs to use.

Changes v1 -> v2:
- drop per-package dependency, move it to BR2_PACKAGE_HOST_GO_ARCH_SUPPORTS (Thomas)

Regards,
Yann E. MORIN.

The following changes since commit ea5280b889782e28b6bdf43e28b0e6a3a610f921

package/samba4: fix install of systemd files (2018-11-25 09:37:07 +0100)

are available in the git repository at:

git://git.buildroot.org/~ymorin/git/buildroot.git

for you to fetch changes up to d64c03d07584a8f48ab309fff66d2ee5661d5ee3

package: hide golang packages for toolchains with binutils bug 20006 (2018-11-25 09:54:23 +0100)

----------------------------------------------------------------
Yann E. MORIN (3):
infra/pkg-golang: enforce number of parallel jobs
toolchain: CodeSourcery AMD64 affected by PR20006
package: hide golang packages for toolchains with binutils bug 20006

 package/go/Config.in.host                                           | 1 +
 package/pkg-golang.mk                                               | 6 ++++--
 toolchain/Config.in                                                 | 5 +++++
 .../toolchain-external-codesourcery-amd64/Config.in                 | 1 +
 4 files changed, 11 insertions(+), 2 deletions(-)
Signature · 6 lines
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'
author
#1 lore raw

PATCH v2 1/3 infra/pkg-golang: enforce number of parallel jobs

By default, the go compiler will spawn as many jobs as there are CPUs available, thus possibily over-shooting the limits set by the user.

Make it abide by the user's wish, and specify the number of jobs allowed to run.

We can do so without fear of a package failing to build in parallel, because they were already all building in parallel, as that is the default for the go compiler.

Signed-off-by "Yann E. MORIN" <[email protected]>
---
 package/pkg-golang.mk | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)
package/pkg-golang.mk +4 -2
index 6eacd14180..4f2c7e77e1 100644
@@ -55,8 +55,10 @@ ifeq ($(BR2_STATIC_LIBS),y)
 $(2)_LDFLAGS += -extldflags '-static'
 endif
 
-$(2)_BUILD_OPTS += -ldflags "$$($(2)_LDFLAGS)"
-$(2)_BUILD_OPTS += -tags "$$($(2)_TAGS)"
+$(2)_BUILD_OPTS += \
+	-ldflags "$$($(2)_LDFLAGS)" \
+	-tags "$$($(2)_TAGS)" \
+	-p $(PARALLEL_JOBS)
 
 # Target packages need the Go compiler on the host.
 $(2)_DEPENDENCIES += host-go
Signature · 1 line
2.14.1
#2 lore raw

PATCH v2 1/3 infra/pkg-golang: enforce number of parallel jobs

Quoted 20 lines · Yann E. MORIN

By default, the go compiler will spawn as many jobs as there are CPUs available, thus possibily over-shooting the limits set by the user.

Make it abide by the user's wish, and specify the number of jobs allowed to run.

We can do so without fear of a package failing to build in parallel, because they were already all building in parallel, as that is the default for the go compiler.

Signed-off-by "Yann E. MORIN" <[email protected]>
---
 package/pkg-golang.mk | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)
package/pkg-golang.mk +0 -0
index 6eacd14180..4f2c7e77e1 100644

package/pkg-golang.mk ifeq ($(BR2_STATIC_LIBS),y) · line 61

Quoted 9 lines
package/pkg-golang.mk +4 -2
@@ -55,8 +55,10 @@ ifeq ($(BR2_STATIC_LIBS),y)
 $(2)_LDFLAGS += -extldflags '-static'
 endif
 
-$(2)_BUILD_OPTS += -ldflags "$$($(2)_LDFLAGS)"
-$(2)_BUILD_OPTS += -tags "$$($(2)_TAGS)"
+$(2)_BUILD_OPTS += \
+	-ldflags "$$($(2)_LDFLAGS)" \
+	-tags "$$($(2)_TAGS)" \
+	-p $(PARALLEL_JOBS)

I wonder, if BR2_LEVEL == 0, if it wouldn't be more appropriate to not pass any
option. That said, it probably needlessly complicates things.

[I also wonder but don't want to think about: what will happen in top-level
parallel build...]

Regards,
Arnout

Quoted 4 lines
package/pkg-golang.mk +0 -0
 # Target packages need the Go compiler on the host.
 $(2)_DEPENDENCIES += host-go
author
#3 lore raw

PATCH v2 1/3 infra/pkg-golang: enforce number of parallel jobs

Arnout, All,

On 2018-11-27 18:42 +0100, Arnout Vandecappelle spake thusly:

Quoted 24 lines · Yann E. MORIN
Quoted 23 lines

By default, the go compiler will spawn as many jobs as there are CPUs available, thus possibily over-shooting the limits set by the user.

Make it abide by the user's wish, and specify the number of jobs allowed to run.

We can do so without fear of a package failing to build in parallel, because they were already all building in parallel, as that is the default for the go compiler.

Signed-off-by "Yann E. MORIN" <[email protected]>
---
 package/pkg-golang.mk | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)
package/pkg-golang.mk +0 -0
index 6eacd14180..4f2c7e77e1 100644
@@ -55,8 +55,10 @@ ifeq ($(BR2_STATIC_LIBS),y)
 $(2)_LDFLAGS += -extldflags '-static'
 endif
Quoted 9 lines

package/pkg-golang.mk ifeq ($(BR2_STATIC_LIBS),y) · line 61

Quoted 6 lines
package/pkg-golang.mk +4 -2
-$(2)_BUILD_OPTS += -ldflags "$$($(2)_LDFLAGS)"
-$(2)_BUILD_OPTS += -tags "$$($(2)_TAGS)"
+$(2)_BUILD_OPTS += \
+	-ldflags "$$($(2)_LDFLAGS)" \
+	-tags "$$($(2)_TAGS)" \
+	-p $(PARALLEL_JOBS)

I wonder, if BR2_LEVEL == 0, if it wouldn't be more appropriate to not pass any
option. That said, it probably needlessly complicates things.

Well, I'd rather we be consistent: anything that want to be parallel uses $(PARALLEL_JOBS), which is always the correct value.

Quoted 2 lines

[I also wonder but don't want to think about: what will happen in top-level
parallel build...]

I think it would go smoothly, in fact, given the direction we're going. AFAIR, Thomas suggested we decorelate the top-level parallelism from the package-level parallelism, i.e. two config options: one to specify the number of simultaneous package to build in parallel, and one to specify the number of jobs per package [0].

That does not allow to parallelise as much as it sould be, but allows to not overload the system too much, especially for those buildsystems that do not know how to talk to a make-jobserver, like guess what, go, or ninja [1] or such.

So, in the end, the existig PARALLEL_JOBS will be the package-level paralelism, with the user specifying the top-level parallelism with 'make -j N'

Not that this is ideal, I know, but that's the best we can do...

[0] AFAIUI, that's what is done in The Other buildsystem. Or so I was told...

[1] there is a MR pending for ninja, and they seem to be amenable to merge it sooner or later. But there will always be outliers that don;t know how to (or want to) play by the rules.

Regards,
Yann E. MORIN.

Signature · 6 lines
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'
#4 lore raw

PATCH v2 1/3 infra/pkg-golang: enforce number of parallel jobs

In reply to Yann E. MORIN
Quoted 5 lines
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

"Yann" == Yann E MORIN <[email protected]> writes:

By default, the go compiler will spawn as many jobs as there are CPUs available, thus possibily over-shooting the limits set by the user.

Quoted 8 lines

Make it abide by the user's wish, and specify the number of jobs allowed to run.

We can do so without fear of a package failing to build in parallel, because they were already all building in parallel, as that is the default for the go compiler.

Signed-off-by "Yann E. MORIN" <[email protected]>

Committed to 2018.08.x, thanks.

Signature · 1 line
Bye, Peter Korsgaard
author
#5 lore raw

PATCH v2 2/3 toolchain: CodeSourcery AMD64 affected by PR20006

In reply to Yann E. MORIN
Signed-off-by "Yann E. MORIN" <[email protected]>
Cc Romain Naour <[email protected]>
Cc Thomas Petazzoni <[email protected]>
---
 toolchain/Config.in                                                  | 5 +++++
 .../toolchain-external-codesourcery-amd64/Config.in                  | 1 +
 2 files changed, 6 insertions(+)
toolchain/Config.in +5 -0
index c2192a52b1..196612b8b0 100644
@@ -69,6 +69,11 @@ comment "Toolchain Generic Options"
 config BR2_TOOLCHAIN_HAS_BINUTILS_BUG_19615
 	bool
 
+# https://sourceware.org/bugzilla/show_bug.cgi?id=20006
+# Affect toolchains built with binutils 2.26 (fixed in binutils 2.26.1).
+config BR2_TOOLCHAIN_HAS_BINUTILS_BUG_20006
+	bool
+
 # Atomic types can be:
 #  - never lock-free
 #  - sometimes lock-free
toolchain/toolchain-external/toolchain-external-codesourcery-amd64/Config.in +1 -0
index 50c5a20019..9187333b95 100644
@@ -13,6 +13,7 @@ config BR2_TOOLCHAIN_EXTERNAL_CODESOURCERY_AMD64
 	select BR2_TOOLCHAIN_HEADERS_AT_LEAST_4_7
 	select BR2_TOOLCHAIN_GCC_AT_LEAST_6
 	select BR2_TOOLCHAIN_HAS_BINUTILS_BUG_19615 # based-on binutils-2.26
+	select BR2_TOOLCHAIN_HAS_BINUTILS_BUG_20006 # based-on binutils-2.26
 	help
 	  Sourcery CodeBench toolchain for the amd64 (x86_64)
 	  architectures, from Mentor Graphics. It uses gcc 6.2,
Signature · 1 line
2.14.1
author
#7 lore raw

PATCH v2 3/3 package: hide golang packages for toolchains with binutils bug 20006

In reply to Yann E. MORIN

Fixes:

    http://autobuild.buildroot.org/results/020/02039969b16534d4020ecd4574bae71b91c1e6b8/ (flannel)
    http://autobuild.buildroot.org/results/e95/e9528b06b350ef84c1e2cb59fba87b4db77b4660/ (docker-engine)
    [...]
Signed-off-by "Yann E. MORIN" <[email protected]>
Cc Thomas Petazzoni <[email protected]>
Cc Romain Naour <[email protected]>

---
Changes v1 -> v2:
- drop per-package dependency, move it to BR2_PACKAGE_HOST_GO_ARCH_SUPPORTS (Thomas)

---
 package/go/Config.in.host | 1 +
 1 file changed, 1 insertion(+)
package/go/Config.in.host +1 -0
index c871ac4196..f619ca0073 100644
@@ -1,6 +1,7 @@
 config BR2_PACKAGE_HOST_GO_ARCH_SUPPORTS
 	bool
 	default y
+	depends on !BR2_TOOLCHAIN_HAS_BINUTILS_BUG_20006
 	depends on BR2_PACKAGE_HOST_GO_BOOTSTRAP_ARCH_SUPPORTS
 	depends on (BR2_arm && BR2_TOOLCHAIN_SUPPORTS_PIE) || BR2_aarch64 \
 		|| BR2_i386 || BR2_x86_64 || BR2_powerpc64le \
Signature · 1 line
2.14.1
#8 lore raw

PATCH v2 3/3 package: hide golang packages for toolchains with binutils bug 20006

Quoted 7 lines
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

"Yann" == Yann E MORIN <[email protected]> writes:

Fixes:

    http://autobuild.buildroot.org/results/020/02039969b16534d4020ecd4574bae71b91c1e6b8/ (flannel)
    http://autobuild.buildroot.org/results/e95/e9528b06b350ef84c1e2cb59fba87b4db77b4660/ (docker-engine)
    [...]
Quoted 8 lines
Signed-off-by "Yann E. MORIN" <[email protected]>
Cc Thomas Petazzoni <[email protected]>
Cc Romain Naour <[email protected]>

---
Changes v1 -> v2:
- drop per-package dependency, move it to BR2_PACKAGE_HOST_GO_ARCH_SUPPORTS (Thomas)

Committed to 2018.08.x, thanks.

Signature · 1 line
Bye, Peter Korsgaard
#9 lore raw

v2 cover golang: fix build with oldish codesourcery amd64 toolchain (branch yem/go-west)

In reply to Yann E. MORIN
Quoted 31 lines
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

"Yann" == Yann E MORIN <[email protected]> writes:

Hello All!
That toolchain seems to suffer from bug #20006

    https://sourceware.org/bugzilla/show_bug.cgi?id=20006

So, like bug 19615, introduce a config option for 20006, and mask away all golang packages.

Additionally, make golang packages abide by the number of CPUs to use.

Changes v1 -> v2:
- drop per-package dependency, move it to BR2_PACKAGE_HOST_GO_ARCH_SUPPORTS (Thomas)

Regards,
Yann E. MORIN.

The following changes since commit ea5280b889782e28b6bdf43e28b0e6a3a610f921

package/samba4: fix install of systemd files (2018-11-25 09:37:07 +0100)

are available in the git repository at:

git://git.buildroot.org/~ymorin/git/buildroot.git

for you to fetch changes up to d64c03d07584a8f48ab309fff66d2ee5661d5ee3

Quoted 8 lines

package: hide golang packages for toolchains with binutils bug 20006 (2018-11-25 09:54:23 +0100)

----------------------------------------------------------------
Yann E. MORIN (3):
infra/pkg-golang: enforce number of parallel jobs
toolchain: CodeSourcery AMD64 affected by PR20006
package: hide golang packages for toolchains with binutils bug 20006

Committed all 3, thanks.

Signature · 1 line
Bye, Peter Korsgaard