ACK: [PATCH 1/1] libfwtsiasl: fix parallel build with GNU Make >= 4.4

[ivanhu]

Thanks for the fix.

Acked-by: Ivan Hu <ivan.hu at canonical.com>

On 5/20/24 17:27, Julien Olivain wrote:
> When a build host has a large number of cores (like 20+) and GNU Make
> version is >= 4.4, fwts randomly fail to build in parallel, with a
> "make -j$(nproc)" command, with error:
> 
>      mv: cannot stat 'dtcompilerparser.tab.c': No such file or directory
>      mv: cannot stat 'prparser.tab.c': No such file or directory
> 
> This issue has been reported here:
> https://github.com/fwts/fwts/issues/7
> 
> The Makefile.am of libfwtsiasl is using the GNU Make ".NOTPARALLEL"
> special target with prerequisites to handle commands generating
> multiple outputs (like lex/yacc invocations). See:
> https://github.com/fwts/fwts/blob/V24.03.00/src/libfwtsiasl/Makefile.am#L61
> 
> First, the .NOTPARALLEL special target _with_ prerequisites is a
> feature added in GNU Make 4.4. See:
> https://git.savannah.gnu.org/cgit/make.git/commit/?id=f6ea899d83bf00fe9201fde0ca9cf7af8e443677
> https://lists.gnu.org/archive/html/help-make/2022-10/msg00020.html
> 
> GNU Make version < 4.4 will interpret it as if it was written without
> prerequisite (as a standalone ".NOTPARALLEL:"). The effect is that the
> parallel compilation is disabled for the whole libfwtsiasl. The
> standalone .NOTPARALLEL special target was introduced in GNU Make 3.79
> in 2000. This is why parallel builds are working with Make older than
> version 4.4.
> 
> Secondly, the reason why the build is failing on GNU Make >= 4.4 is
> because the usage of .NOTPARALLEL in incorrect.
> 
> Quoting the Make manual:
> https://www.gnu.org/software/make/manual/html_node/Parallel-Disable.html
> """
> If the .NOTPARALLEL special target has prerequisites, then each of those
> prerequisites will be considered a target and all prerequisites of these
> targets will be run serially.
> """
> 
> Note the serialization will happen on the prerequisites of the targets
> set as prerequisites of .NOTPARALLEL.
> 
> The targets will not be correctly marked to disable parallel
> execution.
> 
> Thirdly, the use of multiple targets in a rule is incorrect here. See
> Make manual:
> https://www.gnu.org/software/make/manual/html_node/Multiple-Targets.html
> The construct used in Makefile.am of libfwtsiasl for lex/yacc parsers
> assumes they are independant targets (so they can be executed in
> parallel). Finally, the "mv" command is failing, because there will be
> one parallel execution per target, the first mv will suceed and the
> other ones will fail. Multiple independant targets are often used in
> Makefiles for lex/yacc, they are working because they are not using
> "mv". Even in multiple execution, files are just overwritten.
> 
> Fixing this .NOTPARALLEL usage with prerequisites would require Make
> version 4.4 or greater. This is a strong requirement, as there is
> still many Linux distros with older Make version (as an example Ubuntu
> 22.04 LTS has Make 4.3).
> 
> The .WAIT special target could be used, but was also introduced in
> Make version 4.4. See:
> https://git.savannah.gnu.org/cgit/make.git/commit/?id=f6ea899d83bf00fe9201fde0ca9cf7af8e443677
> 
> GNU Make 4.3 also introduced "Grouped Targets" for that purpose. See:
> https://www.gnu.org/software/make/manual/html_node/Multiple-Targets.html
> But this would add a requirement on a recent Make version.
> 
> This commit fixes the issue by declaring the first generated file as a
> dependency of the other extra generated files. This has the effect of
> completely solving the parallel build for all GNU Make versions. Also,
> this enables parallel build for libfwtsiasl (except for the parser
> generation) and makes the whole build faster.
> 
> Signed-off-by: Julien Olivain <ju.o at free.fr>
> ---
>   src/libfwtsiasl/Makefile.am | 16 ++++++++--------
>   1 file changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/src/libfwtsiasl/Makefile.am b/src/libfwtsiasl/Makefile.am
> index cb10bc58..ac54f621 100644
> --- a/src/libfwtsiasl/Makefile.am
> +++ b/src/libfwtsiasl/Makefile.am
> @@ -58,32 +58,32 @@ aslcompiler.y: $(ASL_PARSER)
>   aslcompilerlex.c: $(ASL_LEXER)
>   	${LEX} ${AM_LFLAGS} -PAslCompiler -o$@ $(top_srcdir)/src/acpica/source/compiler/aslcompiler.l
>   
> -.NOTPARALLEL: aslcompiler.c
> -aslcompiler.c aslcompiler.y.h: aslcompiler.y
> +aslcompiler.c: aslcompiler.y
>   	${YACC} ${AM_YFLAGS} -d -baslcompiler -pAslCompiler $^
>   	mv aslcompiler.tab.c aslcompiler.c
>   	cp aslcompiler.tab.h aslcompiler.y.h
> +aslcompiler.y.h: aslcompiler.c
>   
> -.NOTPARALLEL: dtcompilerparserlex.c
> -dtcompilerparserlex.c dtcompilerparser.c dtcompilerparser.y.h: $(top_srcdir)/src/acpica/source/compiler/dtcompilerparser.l $(top_srcdir)/src/acpica/source/compiler/dtcompilerparser.y
> +dtcompilerparserlex.c: $(top_srcdir)/src/acpica/source/compiler/dtcompilerparser.l $(top_srcdir)/src/acpica/source/compiler/dtcompilerparser.y
>   	${LEX} ${AM_LFLAGS} -PDtCompilerParser -odtcompilerparserlex.c $<
>   	${YACC} ${AM_YFLAGS} -bdtcompilerparser -pDtCompilerParser $(top_srcdir)/src/acpica/source/compiler/dtcompilerparser.y
>   	mv dtcompilerparser.tab.c dtcompilerparser.c
>   	cp dtcompilerparser.tab.h dtcompilerparser.y.h
> +dtcompilerparser.c dtcompilerparser.y.h: dtcompilerparserlex.c
>   
> -.NOTPARALLEL: dtparserlex.c
> -dtparserlex.c dtparser.c dtparser.y.h: $(top_srcdir)/src/acpica/source/compiler/dtparser.l $(top_srcdir)/src/acpica/source/compiler/dtparser.y
> +dtparserlex.c: $(top_srcdir)/src/acpica/source/compiler/dtparser.l $(top_srcdir)/src/acpica/source/compiler/dtparser.y
>   	${LEX} ${AM_LFLAGS} -PDtParser -odtparserlex.c $<
>   	${YACC} ${AM_YFLAGS} -bdtparser -pDtParser $(top_srcdir)/src/acpica/source/compiler/dtparser.y
>   	mv dtparser.tab.c dtparser.c
>   	cp dtparser.tab.h dtparser.y.h
> +dtparser.c dtparser.y.h: dtparserlex.c
>   
> -.NOTPARALLEL: prparserlex.c
> -prparserlex.c prparser.c prparser.y.h: $(top_srcdir)/src/acpica/source/compiler/prparser.l $(top_srcdir)/src/acpica/source/compiler/prparser.y
> +prparserlex.c: $(top_srcdir)/src/acpica/source/compiler/prparser.l $(top_srcdir)/src/acpica/source/compiler/prparser.y
>   	${LEX} ${AM_LFLAGS} -PPrParser -oprparserlex.c $<
>   	${YACC} ${AM_YFLAGS} -bprparser -pPrParser $(top_srcdir)/src/acpica/source/compiler/prparser.y
>   	mv prparser.tab.c prparser.c
>   	cp prparser.tab.h prparser.y.h
> +prparser.c prparser.y.h: prparserlex.c
>   
>   pkglib_LTLIBRARIES = libfwtsiasl.la
>