| Message ID | 20220621101128.50543-1-anthony.perard@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [XEN,v2.1,1/4] build,include: rework shell script for headers++.chk | expand |
Hi Anthony, On 21.06.2022 12:11, Anthony PERARD wrote: > The command line generated for headers++.chk by make is quite long, > and in some environment it is too long. This issue have been seen in > Yocto build environment. > > Error messages: > make[9]: execvp: /bin/sh: Argument list too long > make[9]: *** [include/Makefile:181: include/headers++.chk] Error 127 > > Rework so that we do the foreach loop in shell rather that make to > reduce the command line size by a lot. We also need a way to get > headers prerequisite for some public headers so we use a switch "case" > in shell to be able to do some simple pattern matching. Variables > alone in POSIX shell don't allow to work with associative array or > variables with "/". > > Also rework headers99.chk as it has a similar implementation, even if > with only two headers to check the command line isn't too long at the > moment. > > Reported-by: Bertrand Marquis <Bertrand.Marquis@arm.com> > Fixes: 28e13c7f43 ("build: xen/include: use if_changed") > Signed-off-by: Anthony PERARD <anthony.perard@citrix.com> > Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com> Reviewed-by: Michal Orzel <michal.orzel@arm.com> Tested-by: Michal Orzel <michal.orzel@arm.com> Cheers, Michal
On 22.06.2022 14:35, Michal Orzel wrote: > Hi Anthony, > > On 21.06.2022 12:11, Anthony PERARD wrote: >> The command line generated for headers++.chk by make is quite long, >> and in some environment it is too long. This issue have been seen in >> Yocto build environment. >> >> Error messages: >> make[9]: execvp: /bin/sh: Argument list too long >> make[9]: *** [include/Makefile:181: include/headers++.chk] Error 127 >> >> Rework so that we do the foreach loop in shell rather that make to >> reduce the command line size by a lot. We also need a way to get >> headers prerequisite for some public headers so we use a switch "case" >> in shell to be able to do some simple pattern matching. Variables >> alone in POSIX shell don't allow to work with associative array or >> variables with "/". >> >> Also rework headers99.chk as it has a similar implementation, even if >> with only two headers to check the command line isn't too long at the >> moment. >> >> Reported-by: Bertrand Marquis <Bertrand.Marquis@arm.com> >> Fixes: 28e13c7f43 ("build: xen/include: use if_changed") >> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com> >> Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com> > > Reviewed-by: Michal Orzel <michal.orzel@arm.com> Acked-by: Jan Beulich <jbeulich@suse.com> > Tested-by: Michal Orzel <michal.orzel@arm.com> > > Cheers, > Michal
On 21.06.2022 12:11, Anthony PERARD wrote: > The command line generated for headers++.chk by make is quite long, > and in some environment it is too long. This issue have been seen in > Yocto build environment. > > Error messages: > make[9]: execvp: /bin/sh: Argument list too long > make[9]: *** [include/Makefile:181: include/headers++.chk] Error 127 > > Rework so that we do the foreach loop in shell rather that make to > reduce the command line size by a lot. We also need a way to get > headers prerequisite for some public headers so we use a switch "case" > in shell to be able to do some simple pattern matching. Variables > alone in POSIX shell don't allow to work with associative array or > variables with "/". > > Also rework headers99.chk as it has a similar implementation, even if > with only two headers to check the command line isn't too long at the > moment. > > Reported-by: Bertrand Marquis <Bertrand.Marquis@arm.com> > Fixes: 28e13c7f43 ("build: xen/include: use if_changed") > Signed-off-by: Anthony PERARD <anthony.perard@citrix.com> > Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com> I have committed this, but strictly speaking imo the R-b should have been dropped because ... > --- > > Notes: > v3: > - add one more pattern to avoid a possible empty body for "case" > - use $() instead of `` to execute get_prereq() > - also convert headers99_chk > - convert some 'tab' to 'space', have only 1 tab at start of line ... at least the added headers99_chk conversion was not a purely mechanical change. Jan > v2: > - fix typo in commit message > - fix out-of-tree build > > v1: > - was sent as a reply to v1 of the series > > xen/include/Makefile | 37 +++++++++++++++++++++++++++++-------- > 1 file changed, 29 insertions(+), 8 deletions(-) > > diff --git a/xen/include/Makefile b/xen/include/Makefile > index 617599df7e..510f65c92a 100644 > --- a/xen/include/Makefile > +++ b/xen/include/Makefile > @@ -141,13 +141,24 @@ cmd_header_chk = \ > quiet_cmd_headers99_chk = CHK $@ > define cmd_headers99_chk > rm -f $@.new; \ > - $(foreach i, $(filter %.h,$^), \ > - echo "#include "\"$(i)\" \ > + get_prereq() { \ > + case $$1 in \ > + $(foreach i, $(filter %.h,$^), \ > + $(if $($(patsubst $(srctree)/%,%,$(i))-prereq), \ > + $(i)$(close) \ > + echo "$(foreach j, $($(patsubst $(srctree)/%,%,$(i))-prereq), \ > + -include $(j).h)";;)) \ > + *) ;; \ > + esac; \ > + }; \ > + for i in $(filter %.h,$^); do \ > + echo "#include "\"$$i\" \ > | $(CC) -x c -std=c99 -Wall -Werror \ > -include stdint.h \ > - $(foreach j, $($(patsubst $(srctree)/%,%,$i)-prereq), -include $(j).h) \ > + $$(get_prereq $$i) \ > -S -o /dev/null - \ > - || exit $$?; echo $(i) >> $@.new;) \ > + || exit $$?; echo $$i >> $@.new; \ > + done; \ > mv $@.new $@ > endef > > @@ -158,13 +169,23 @@ define cmd_headerscxx_chk > touch $@.new; \ > exit 0; \ > fi; \ > - $(foreach i, $(filter %.h,$^), \ > - echo "#include "\"$(i)\" \ > + get_prereq() { \ > + case $$1 in \ > + $(foreach i, $(filter %.h,$^), \ > + $(if $($(patsubst $(srctree)/%,%,$(i))-prereq), \ > + $(i)$(close) \ > + echo "$(foreach j, $($(patsubst $(srctree)/%,%,$(i))-prereq), \ > + -include c$(j))";;)) \ > + *) ;; \ > + esac; \ > + }; \ > + for i in $(filter %.h,$^); do \ > + echo "#include "\"$$i\" \ > | $(CXX) -x c++ -std=gnu++98 -Wall -Werror -D__XEN_TOOLS__ \ > -include stdint.h -include $(srcdir)/public/xen.h \ > - $(foreach j, $($(patsubst $(srctree)/%,%,$i)-prereq), -include c$(j)) \ > + $$(get_prereq $$i) \ > -S -o /dev/null - \ > - || exit $$?; echo $(i) >> $@.new;) \ > + || exit $$?; echo $$i >> $@.new; done; \ > mv $@.new $@ > endef >
diff --git a/xen/include/Makefile b/xen/include/Makefile index 617599df7e..510f65c92a 100644 --- a/xen/include/Makefile +++ b/xen/include/Makefile @@ -141,13 +141,24 @@ cmd_header_chk = \ quiet_cmd_headers99_chk = CHK $@ define cmd_headers99_chk rm -f $@.new; \ - $(foreach i, $(filter %.h,$^), \ - echo "#include "\"$(i)\" \ + get_prereq() { \ + case $$1 in \ + $(foreach i, $(filter %.h,$^), \ + $(if $($(patsubst $(srctree)/%,%,$(i))-prereq), \ + $(i)$(close) \ + echo "$(foreach j, $($(patsubst $(srctree)/%,%,$(i))-prereq), \ + -include $(j).h)";;)) \ + *) ;; \ + esac; \ + }; \ + for i in $(filter %.h,$^); do \ + echo "#include "\"$$i\" \ | $(CC) -x c -std=c99 -Wall -Werror \ -include stdint.h \ - $(foreach j, $($(patsubst $(srctree)/%,%,$i)-prereq), -include $(j).h) \ + $$(get_prereq $$i) \ -S -o /dev/null - \ - || exit $$?; echo $(i) >> $@.new;) \ + || exit $$?; echo $$i >> $@.new; \ + done; \ mv $@.new $@ endef @@ -158,13 +169,23 @@ define cmd_headerscxx_chk touch $@.new; \ exit 0; \ fi; \ - $(foreach i, $(filter %.h,$^), \ - echo "#include "\"$(i)\" \ + get_prereq() { \ + case $$1 in \ + $(foreach i, $(filter %.h,$^), \ + $(if $($(patsubst $(srctree)/%,%,$(i))-prereq), \ + $(i)$(close) \ + echo "$(foreach j, $($(patsubst $(srctree)/%,%,$(i))-prereq), \ + -include c$(j))";;)) \ + *) ;; \ + esac; \ + }; \ + for i in $(filter %.h,$^); do \ + echo "#include "\"$$i\" \ | $(CXX) -x c++ -std=gnu++98 -Wall -Werror -D__XEN_TOOLS__ \ -include stdint.h -include $(srcdir)/public/xen.h \ - $(foreach j, $($(patsubst $(srctree)/%,%,$i)-prereq), -include c$(j)) \ + $$(get_prereq $$i) \ -S -o /dev/null - \ - || exit $$?; echo $(i) >> $@.new;) \ + || exit $$?; echo $$i >> $@.new; done; \ mv $@.new $@ endef
The command line generated for headers++.chk by make is quite long, and in some environment it is too long. This issue have been seen in Yocto build environment. Error messages: make[9]: execvp: /bin/sh: Argument list too long make[9]: *** [include/Makefile:181: include/headers++.chk] Error 127 Rework so that we do the foreach loop in shell rather that make to reduce the command line size by a lot. We also need a way to get headers prerequisite for some public headers so we use a switch "case" in shell to be able to do some simple pattern matching. Variables alone in POSIX shell don't allow to work with associative array or variables with "/". Also rework headers99.chk as it has a similar implementation, even if with only two headers to check the command line isn't too long at the moment. Reported-by: Bertrand Marquis <Bertrand.Marquis@arm.com> Fixes: 28e13c7f43 ("build: xen/include: use if_changed") Signed-off-by: Anthony PERARD <anthony.perard@citrix.com> Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com> --- Notes: v3: - add one more pattern to avoid a possible empty body for "case" - use $() instead of `` to execute get_prereq() - also convert headers99_chk - convert some 'tab' to 'space', have only 1 tab at start of line v2: - fix typo in commit message - fix out-of-tree build v1: - was sent as a reply to v1 of the series xen/include/Makefile | 37 +++++++++++++++++++++++++++++-------- 1 file changed, 29 insertions(+), 8 deletions(-)