diff mbox series

package/exim: fix parallel build

Message ID 20200502181504.28690-1-luca@lucaceresoli.net
State Accepted
Headers show
Series package/exim: fix parallel build | expand

Commit Message

Luca Ceresoli May 2, 2020, 6:15 p.m. UTC
exim does build in parallel correctly, but has a concurrency bug in
generating version info files which happens either in the build step or in
the install step.

Add a patch to fix the bug.

Fixes:
  http://autobuild.buildroot.net/results/ebf/ebfccad007e216564889645a07f5487747116331//
  http://autobuild.buildroot.net/results/56a/56a8457efcb32579ad6da99a769b6438dd0db267//
  http://autobuild.buildroot.net/results/6a1/6a1f8a352649baf767b094cb6bbe2a7397fa7fac//
  http://autobuild.buildroot.net/results/5ed/5ed1c42b3d33198f32d1267e5cc2b1fa1211495a//
  http://autobuild.buildroot.net/results/b30/b304569948fd481ce33ecd052a1036153c5d459e//
  http://autobuild.buildroot.net/results/d2c/d2c7abfe08672e53ff890127f787f8d2e84860f4//

Signed-off-by: Luca Ceresoli <luca@lucaceresoli.net>

---

I cannot test that this is really a fix since I never was able to reproduce
the problem locally. However I could easily check that the reversion script
is actually executed twice. The timing of these executions can cause errors
like the one found by the autobuilders. Thus I'm reasonably sure this
commit fixes a bug.
---
 ...x-version.-h-sh-generation-with-para.patch | 84 +++++++++++++++++++
 1 file changed, 84 insertions(+)
 create mode 100644 package/exim/0007-Makefile-Base-fix-version.-h-sh-generation-with-para.patch

Comments

Yann E. MORIN May 2, 2020, 7:31 p.m. UTC | #1
Luca, All,

On 2020-05-02 20:15 +0200, Luca Ceresoli spake thusly:
> exim does build in parallel correctly, but has a concurrency bug in
> generating version info files which happens either in the build step or in
> the install step.
> 
> Add a patch to fix the bug.
> 
> Fixes:
>   http://autobuild.buildroot.net/results/ebf/ebfccad007e216564889645a07f5487747116331//
>   http://autobuild.buildroot.net/results/56a/56a8457efcb32579ad6da99a769b6438dd0db267//
>   http://autobuild.buildroot.net/results/6a1/6a1f8a352649baf767b094cb6bbe2a7397fa7fac//
>   http://autobuild.buildroot.net/results/5ed/5ed1c42b3d33198f32d1267e5cc2b1fa1211495a//
>   http://autobuild.buildroot.net/results/b30/b304569948fd481ce33ecd052a1036153c5d459e//
>   http://autobuild.buildroot.net/results/d2c/d2c7abfe08672e53ff890127f787f8d2e84860f4//
> 
> Signed-off-by: Luca Ceresoli <luca@lucaceresoli.net>

Applied to master, thanks.

Regards,
Yann E. MORIN.

> ---
> 
> I cannot test that this is really a fix since I never was able to reproduce
> the problem locally. However I could easily check that the reversion script
> is actually executed twice. The timing of these executions can cause errors
> like the one found by the autobuilders. Thus I'm reasonably sure this
> commit fixes a bug.
> ---
>  ...x-version.-h-sh-generation-with-para.patch | 84 +++++++++++++++++++
>  1 file changed, 84 insertions(+)
>  create mode 100644 package/exim/0007-Makefile-Base-fix-version.-h-sh-generation-with-para.patch
> 
> diff --git a/package/exim/0007-Makefile-Base-fix-version.-h-sh-generation-with-para.patch b/package/exim/0007-Makefile-Base-fix-version.-h-sh-generation-with-para.patch
> new file mode 100644
> index 000000000000..e97bd78a6ae1
> --- /dev/null
> +++ b/package/exim/0007-Makefile-Base-fix-version.-h-sh-generation-with-para.patch
> @@ -0,0 +1,84 @@
> +From 19f6e36d3473ddba1a211e7af9352a10febb7270 Mon Sep 17 00:00:00 2001
> +From: Luca Ceresoli <luca@lucaceresoli.net>
> +Date: Fri, 1 May 2020 16:27:48 +0200
> +Subject: [PATCH] Makefile-Base: fix version.{h,sh} generation with parallel
> + build
> +
> +When using parallel make (make -j<N>) the build sometimes fails either
> +during 'make' or during 'make install'.
> +
> +Error messages look either like:
> +
> +  make[1]: Entering directory '/home/buildroot/autobuild/instance-2/output-1/build/exim-4.93.0.4'
> +  /bin/sh scripts/source_checks
> +  `Makefile' is up to date.
> +
> +  make[2]: Entering directory '/home/buildroot/autobuild/instance-2/output-1/build/exim-4.93.0.4/build-br'
> +  /bin/sh ../scripts/Configure-os.c
> +  ../scripts/reversion: Your copy of Exim lacks any version information.
> +  Makefile:785: recipe for target 'version.sh' failed
> +
> +or like:
> +
> +  DESTDIR=/home/buildroot/autobuild/instance-2/output-1/target INSTALL_ARG="-no_chown -no_symlink" build=br /usr/bin/make -j4 -C /home/buildroot/autobuild/instance-2/output-1/build/exim-4.93.0.4  CFLAGS="-std=c99 -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64  -Os  " install
> +  make[1]: Entering directory '/home/buildroot/autobuild/instance-2/output-1/build/exim-4.93.0.4'
> +  /bin/sh scripts/source_checks
> +  `Makefile' is up to date.
> +
> +  make[2]: Entering directory '/home/buildroot/autobuild/instance-2/output-1/build/exim-4.93.0.4/build-br'
> +  /home/buildroot/autobuild/instance-2/output-1/host/bin/i586-linux-gcc version.c
> +  version.c: In function 'version_init':
> +  version.c:32:1: error: expected expression before ';' token
> +   ;
> +   ^
> +
> +This is due to the rule:
> +
> +  version.h version.sh::
> + 	@../scripts/reversion
> +
> +that executes reversion twice, once to satisfy the version.h target and
> +once for version.sh. This is unnecessary because reversion generates both
> +files anyway, but harmless without parallel build. When using parallel make
> +however reversion is sporadically run in a time sequence such that the
> +generated files are being used by other rules while they are being
> +rewritten by the second reversion instance.
> +
> +Fix by making only one of the two targets run reversion, and the other one
> +depend on it.
> +
> +Fixes builds found by the Buildroot autobuilders:
> +  http://autobuild.buildroot.net/results/ebf/ebfccad007e216564889645a07f5487747116331//
> +  http://autobuild.buildroot.net/results/56a/56a8457efcb32579ad6da99a769b6438dd0db267//
> +  http://autobuild.buildroot.net/results/6a1/6a1f8a352649baf767b094cb6bbe2a7397fa7fac//
> +  http://autobuild.buildroot.net/results/5ed/5ed1c42b3d33198f32d1267e5cc2b1fa1211495a//
> +  http://autobuild.buildroot.net/results/b30/b304569948fd481ce33ecd052a1036153c5d459e//
> +  http://autobuild.buildroot.net/results/d2c/d2c7abfe08672e53ff890127f787f8d2e84860f4//
> +
> +Signed-off-by: Luca Ceresoli <luca@lucaceresoli.net>
> +Upstream-status: patch submitted (https://bugs.exim.org/show_bug.cgi?id=2566)
> +
> +---
> + OS/Makefile-Base | 4 +++-
> + 1 file changed, 3 insertions(+), 1 deletion(-)
> +
> +diff --git a/OS/Makefile-Base b/OS/Makefile-Base
> +index b66678bee4de..4966c25b5359 100644
> +--- a/OS/Makefile-Base
> ++++ b/OS/Makefile-Base
> +@@ -664,9 +664,11 @@ PHDRS = ../config.h \
> + 
> + # Update Exim's version information and build the version object.
> + 
> +-version.h version.sh::
> ++version.sh::
> + 	@../scripts/reversion
> + 
> ++version.h: version.sh
> ++
> + cnumber.h: version.h
> + 
> + version.o: $(HDRS) cnumber.h version.h version.c
> +-- 
> +2.26.2
> +
> -- 
> 2.26.2
> 
> _______________________________________________
> buildroot mailing list
> buildroot@busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot
Peter Korsgaard May 10, 2020, 7:25 p.m. UTC | #2
>>>>> "Luca" == Luca Ceresoli <luca@lucaceresoli.net> writes:

 > exim does build in parallel correctly, but has a concurrency bug in
 > generating version info files which happens either in the build step or in
 > the install step.

 > Add a patch to fix the bug.

 > Fixes:
 >   http://autobuild.buildroot.net/results/ebf/ebfccad007e216564889645a07f5487747116331//
 >   http://autobuild.buildroot.net/results/56a/56a8457efcb32579ad6da99a769b6438dd0db267//
 >   http://autobuild.buildroot.net/results/6a1/6a1f8a352649baf767b094cb6bbe2a7397fa7fac//
 >   http://autobuild.buildroot.net/results/5ed/5ed1c42b3d33198f32d1267e5cc2b1fa1211495a//
 >   http://autobuild.buildroot.net/results/b30/b304569948fd481ce33ecd052a1036153c5d459e//
 >   http://autobuild.buildroot.net/results/d2c/d2c7abfe08672e53ff890127f787f8d2e84860f4//

 > Signed-off-by: Luca Ceresoli <luca@lucaceresoli.net>

 > ---

 > I cannot test that this is really a fix since I never was able to reproduce
 > the problem locally. However I could easily check that the reversion script
 > is actually executed twice. The timing of these executions can cause errors
 > like the one found by the autobuilders. Thus I'm reasonably sure this
 > commit fixes a bug.

Committed to 2020.02.x, thanks.
diff mbox series

Patch

diff --git a/package/exim/0007-Makefile-Base-fix-version.-h-sh-generation-with-para.patch b/package/exim/0007-Makefile-Base-fix-version.-h-sh-generation-with-para.patch
new file mode 100644
index 000000000000..e97bd78a6ae1
--- /dev/null
+++ b/package/exim/0007-Makefile-Base-fix-version.-h-sh-generation-with-para.patch
@@ -0,0 +1,84 @@ 
+From 19f6e36d3473ddba1a211e7af9352a10febb7270 Mon Sep 17 00:00:00 2001
+From: Luca Ceresoli <luca@lucaceresoli.net>
+Date: Fri, 1 May 2020 16:27:48 +0200
+Subject: [PATCH] Makefile-Base: fix version.{h,sh} generation with parallel
+ build
+
+When using parallel make (make -j<N>) the build sometimes fails either
+during 'make' or during 'make install'.
+
+Error messages look either like:
+
+  make[1]: Entering directory '/home/buildroot/autobuild/instance-2/output-1/build/exim-4.93.0.4'
+  /bin/sh scripts/source_checks
+  `Makefile' is up to date.
+
+  make[2]: Entering directory '/home/buildroot/autobuild/instance-2/output-1/build/exim-4.93.0.4/build-br'
+  /bin/sh ../scripts/Configure-os.c
+  ../scripts/reversion: Your copy of Exim lacks any version information.
+  Makefile:785: recipe for target 'version.sh' failed
+
+or like:
+
+  DESTDIR=/home/buildroot/autobuild/instance-2/output-1/target INSTALL_ARG="-no_chown -no_symlink" build=br /usr/bin/make -j4 -C /home/buildroot/autobuild/instance-2/output-1/build/exim-4.93.0.4  CFLAGS="-std=c99 -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64  -Os  " install
+  make[1]: Entering directory '/home/buildroot/autobuild/instance-2/output-1/build/exim-4.93.0.4'
+  /bin/sh scripts/source_checks
+  `Makefile' is up to date.
+
+  make[2]: Entering directory '/home/buildroot/autobuild/instance-2/output-1/build/exim-4.93.0.4/build-br'
+  /home/buildroot/autobuild/instance-2/output-1/host/bin/i586-linux-gcc version.c
+  version.c: In function 'version_init':
+  version.c:32:1: error: expected expression before ';' token
+   ;
+   ^
+
+This is due to the rule:
+
+  version.h version.sh::
+ 	@../scripts/reversion
+
+that executes reversion twice, once to satisfy the version.h target and
+once for version.sh. This is unnecessary because reversion generates both
+files anyway, but harmless without parallel build. When using parallel make
+however reversion is sporadically run in a time sequence such that the
+generated files are being used by other rules while they are being
+rewritten by the second reversion instance.
+
+Fix by making only one of the two targets run reversion, and the other one
+depend on it.
+
+Fixes builds found by the Buildroot autobuilders:
+  http://autobuild.buildroot.net/results/ebf/ebfccad007e216564889645a07f5487747116331//
+  http://autobuild.buildroot.net/results/56a/56a8457efcb32579ad6da99a769b6438dd0db267//
+  http://autobuild.buildroot.net/results/6a1/6a1f8a352649baf767b094cb6bbe2a7397fa7fac//
+  http://autobuild.buildroot.net/results/5ed/5ed1c42b3d33198f32d1267e5cc2b1fa1211495a//
+  http://autobuild.buildroot.net/results/b30/b304569948fd481ce33ecd052a1036153c5d459e//
+  http://autobuild.buildroot.net/results/d2c/d2c7abfe08672e53ff890127f787f8d2e84860f4//
+
+Signed-off-by: Luca Ceresoli <luca@lucaceresoli.net>
+Upstream-status: patch submitted (https://bugs.exim.org/show_bug.cgi?id=2566)
+
+---
+ OS/Makefile-Base | 4 +++-
+ 1 file changed, 3 insertions(+), 1 deletion(-)
+
+diff --git a/OS/Makefile-Base b/OS/Makefile-Base
+index b66678bee4de..4966c25b5359 100644
+--- a/OS/Makefile-Base
++++ b/OS/Makefile-Base
+@@ -664,9 +664,11 @@ PHDRS = ../config.h \
+ 
+ # Update Exim's version information and build the version object.
+ 
+-version.h version.sh::
++version.sh::
+ 	@../scripts/reversion
+ 
++version.h: version.sh
++
+ cnumber.h: version.h
+ 
+ version.o: $(HDRS) cnumber.h version.h version.c
+-- 
+2.26.2
+