* [XEN PATCH 0/4] xen: rework compat headers generation
@ 2022-06-01 16:59 Anthony PERARD
2022-06-01 16:59 ` [XEN PATCH 1/4] build: xen/include: use if_changed Anthony PERARD
` (4 more replies)
0 siblings, 5 replies; 23+ messages in thread
From: Anthony PERARD @ 2022-06-01 16:59 UTC (permalink / raw)
To: xen-devel
Cc: Anthony PERARD, Julien Grall, Jan Beulich, Andrew Cooper,
Wei Liu, George Dunlap, Stefano Stabellini
Patch series available in this git branch:
https://xenbits.xen.org/git-http/people/aperard/xen-unstable.git br.build-system-xen-include-rework-v1
Hi,
This patch series is about 2 improvement. First one is to use $(if_changed, )
in "include/Makefile" to make the generation of the compat headers less verbose
and to have the command line part of the decision to rebuild the headers.
Second one is to replace one slow script by a much faster one, and save time
when generating the headers.
Thanks.
Anthony PERARD (4):
build: xen/include: use if_changed
build: set PERL
build: replace get-fields.sh by a perl script
build: remove auto.conf prerequisite from compat/xlat.h target
xen/Makefile | 1 +
xen/include/Makefile | 106 ++++---
xen/tools/compat-xlat-header | 539 +++++++++++++++++++++++++++++++++++
xen/tools/get-fields.sh | 528 ----------------------------------
4 files changed, 614 insertions(+), 560 deletions(-)
create mode 100755 xen/tools/compat-xlat-header
delete mode 100644 xen/tools/get-fields.sh
--
Anthony PERARD
^ permalink raw reply [flat|nested] 23+ messages in thread
* [XEN PATCH 1/4] build: xen/include: use if_changed
2022-06-01 16:59 [XEN PATCH 0/4] xen: rework compat headers generation Anthony PERARD
@ 2022-06-01 16:59 ` Anthony PERARD
2022-06-02 9:11 ` Jan Beulich
2022-06-09 10:16 ` Bertrand Marquis
2022-06-01 16:59 ` [XEN PATCH 2/4] build: set PERL Anthony PERARD
` (3 subsequent siblings)
4 siblings, 2 replies; 23+ messages in thread
From: Anthony PERARD @ 2022-06-01 16:59 UTC (permalink / raw)
To: xen-devel
Cc: Anthony PERARD, Andrew Cooper, George Dunlap, Jan Beulich,
Julien Grall, Stefano Stabellini, Wei Liu
Use "define" for the headers*_chk commands as otherwise the "#"
is interpreted as a comment and make can't find the end of
$(foreach,).
Adding several .PRECIOUS as without them `make` deletes the
intermediate targets. This is an issue because the macro $(if_changed,)
check if the target exist in order to decide whether to recreate the
target.
Removing the call to `mkdir` from the commands. Those aren't needed
anymore because a rune in Rules.mk creates the directory for each
$(targets).
Remove "export PYTHON" as it is already exported.
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---
xen/include/Makefile | 108 ++++++++++++++++++++++++++++++-------------
1 file changed, 76 insertions(+), 32 deletions(-)
diff --git a/xen/include/Makefile b/xen/include/Makefile
index 03baf10efb..6d9bcc19b0 100644
--- a/xen/include/Makefile
+++ b/xen/include/Makefile
@@ -45,38 +45,65 @@ public-$(CONFIG_ARM) := $(wildcard $(srcdir)/public/arch-arm/*.h $(srcdir)/publi
.PHONY: all
all: $(addprefix $(obj)/,$(headers-y))
-$(obj)/compat/%.h: $(obj)/compat/%.i $(srcdir)/Makefile $(srctree)/tools/compat-build-header.py
- $(PYTHON) $(srctree)/tools/compat-build-header.py <$< $(patsubst $(obj)/%,%,$@) >>$@.new; \
- mv -f $@.new $@
-
-$(obj)/compat/%.i: $(obj)/compat/%.c $(srcdir)/Makefile
- $(CPP) $(filter-out -Wa$(comma)% -include %/include/xen/config.h,$(XEN_CFLAGS)) $(cppflags-y) -o $@ $<
-
-$(obj)/compat/%.c: $(src)/public/%.h $(srcdir)/xlat.lst $(srcdir)/Makefile $(srctree)/tools/compat-build-source.py
- mkdir -p $(@D)
- $(PYTHON) $(srctree)/tools/compat-build-source.py $(srcdir)/xlat.lst <$< >$@.new
- mv -f $@.new $@
-
-$(obj)/compat/.xlat/%.h: $(obj)/compat/%.h $(obj)/compat/.xlat/%.lst $(srctree)/tools/get-fields.sh $(srcdir)/Makefile
- export PYTHON=$(PYTHON); \
- while read what name; do \
- $(SHELL) $(srctree)/tools/get-fields.sh "$$what" compat_$$name $< || exit $$?; \
- done <$(patsubst $(obj)/compat/%,$(obj)/compat/.xlat/%,$(basename $<)).lst >$@.new
- mv -f $@.new $@
+quiet_cmd_compat_h = GEN $@
+cmd_compat_h = \
+ $(PYTHON) $(srctree)/tools/compat-build-header.py <$< $(patsubst $(obj)/%,%,$@) >>$@.new; \
+ mv -f $@.new $@
+
+quiet_cmd_compat_i = CPP $@
+cmd_compat_i = $(CPP) $(filter-out -Wa$(comma)% -include %/include/xen/config.h,$(XEN_CFLAGS)) $(cppflags-y) -o $@ $<
+
+quiet_cmd_compat_c = GEN $@
+cmd_compat_c = \
+ $(PYTHON) $(srctree)/tools/compat-build-source.py $(srcdir)/xlat.lst <$< >$@.new; \
+ mv -f $@.new $@
+
+quiet_cmd_xlat_headers = GEN $@
+cmd_xlat_headers = \
+ while read what name; do \
+ $(SHELL) $(srctree)/tools/get-fields.sh "$$what" compat_$$name $< || exit $$?; \
+ done <$(patsubst $(obj)/compat/%,$(obj)/compat/.xlat/%,$(basename $<)).lst >$@.new; \
+ mv -f $@.new $@
+
+targets += $(headers-y)
+$(obj)/compat/%.h: $(obj)/compat/%.i $(srctree)/tools/compat-build-header.py FORCE
+ $(call if_changed,compat_h)
+
+.PRECIOUS: $(obj)/compat/%.i
+targets += $(patsubst %.h, %.i, $(headers-y))
+$(obj)/compat/%.i: $(obj)/compat/%.c FORCE
+ $(call if_changed,compat_i)
+
+.PRECIOUS: $(obj)/compat/%.c
+targets += $(patsubst %.h, %.c, $(headers-y))
+$(obj)/compat/%.c: $(src)/public/%.h $(srcdir)/xlat.lst $(srctree)/tools/compat-build-source.py FORCE
+ $(call if_changed,compat_c)
+
+targets += $(patsubst compat/%, compat/.xlat/%, $(headers-y))
+$(obj)/compat/.xlat/%.h: $(obj)/compat/%.h $(obj)/compat/.xlat/%.lst $(srctree)/tools/get-fields.sh FORCE
+ $(call if_changed,xlat_headers)
+
+quiet_cmd_xlat_lst = GEN $@
+cmd_xlat_lst = \
+ grep -v '^[[:blank:]]*$(pound)' $< | sed -ne 's,@arch@,$(compat-arch-y),g' -re 's,[[:blank:]]+$*\.h[[:blank:]]*$$,,p' >$@.new; \
+ $(call move-if-changed,$@.new,$@)
.PRECIOUS: $(obj)/compat/.xlat/%.lst
-$(obj)/compat/.xlat/%.lst: $(srcdir)/xlat.lst $(srcdir)/Makefile
- mkdir -p $(@D)
- grep -v '^[[:blank:]]*#' $< | sed -ne 's,@arch@,$(compat-arch-y),g' -re 's,[[:blank:]]+$*\.h[[:blank:]]*$$,,p' >$@.new
- $(call move-if-changed,$@.new,$@)
+targets += $(patsubst compat/%.h, compat/.xlat/%.lst, $(headers-y))
+$(obj)/compat/.xlat/%.lst: $(srcdir)/xlat.lst FORCE
+ $(call if_changed,xlat_lst)
xlat-y := $(shell sed -ne 's,@arch@,$(compat-arch-y),g' -re 's,^[?!][[:blank:]]+[^[:blank:]]+[[:blank:]]+,,p' $(srcdir)/xlat.lst | uniq)
xlat-y := $(filter $(patsubst compat/%,%,$(headers-y)),$(xlat-y))
-$(obj)/compat/xlat.h: $(addprefix $(obj)/compat/.xlat/,$(xlat-y)) $(obj)/config/auto.conf $(srcdir)/Makefile
- cat $(filter %.h,$^) >$@.new
+quiet_cmd_xlat_h = GEN $@
+cmd_xlat_h = \
+ cat $(filter %.h,$^) >$@.new; \
mv -f $@.new $@
+$(obj)/compat/xlat.h: $(addprefix $(obj)/compat/.xlat/,$(xlat-y)) $(obj)/config/auto.conf FORCE
+ $(call if_changed,xlat_h)
+
ifeq ($(XEN_TARGET_ARCH),$(XEN_COMPILE_ARCH))
all: $(obj)/headers.chk $(obj)/headers99.chk $(obj)/headers++.chk
@@ -102,27 +129,31 @@ PUBLIC_C99_HEADERS := $(call public-filter-headers,public-c99-headers)
$(src)/public/io/9pfs.h-prereq := string
$(src)/public/io/pvcalls.h-prereq := string
-$(obj)/headers.chk: $(PUBLIC_ANSI_HEADERS) $(srcdir)/Makefile
+quiet_cmd_header_chk = CHK $@
+cmd_header_chk = \
for i in $(filter %.h,$^); do \
$(CC) -x c -ansi -Wall -Werror -include stdint.h \
-S -o /dev/null $$i || exit 1; \
echo $$i; \
- done >$@.new
+ done >$@.new; \
mv $@.new $@
-$(obj)/headers99.chk: $(PUBLIC_C99_HEADERS) $(srcdir)/Makefile
- rm -f $@.new
+quiet_cmd_headers99_chk = CHK $@
+define cmd_headers99_chk
+ rm -f $@.new; \
$(foreach i, $(filter %.h,$^), \
echo "#include "\"$(i)\" \
| $(CC) -x c -std=c99 -Wall -Werror \
-include stdint.h \
$(foreach j, $($(patsubst $(srctree)/%,%,$i)-prereq), -include $(j).h) \
-S -o /dev/null - \
- || exit $$?; echo $(i) >> $@.new;)
+ || exit $$?; echo $(i) >> $@.new;) \
mv $@.new $@
+endef
-$(obj)/headers++.chk: $(PUBLIC_HEADERS) $(srcdir)/Makefile
- rm -f $@.new
+quiet_cmd_headerscxx_chk = CHK $@
+define cmd_headerscxx_chk
+ rm -f $@.new; \
if ! $(CXX) -v >/dev/null 2>&1; then \
touch $@.new; \
exit 0; \
@@ -133,8 +164,21 @@ $(obj)/headers++.chk: $(PUBLIC_HEADERS) $(srcdir)/Makefile
-include stdint.h -include $(srcdir)/public/xen.h \
$(foreach j, $($(patsubst $(srctree)/%,%,$i)-prereq), -include c$(j)) \
-S -o /dev/null - \
- || exit $$?; echo $(i) >> $@.new;)
+ || exit $$?; echo $(i) >> $@.new;) \
mv $@.new $@
+endef
+
+targets += headers.chk
+$(obj)/headers.chk: $(PUBLIC_ANSI_HEADERS) FORCE
+ $(call if_changed,header_chk)
+
+targets += headers99.chk
+$(obj)/headers99.chk: $(PUBLIC_C99_HEADERS) FORCE
+ $(call if_changed,headers99_chk)
+
+targets += headers++.chk
+$(obj)/headers++.chk: $(PUBLIC_HEADERS) FORCE
+ $(call if_changed,headerscxx_chk)
endif
--
Anthony PERARD
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [XEN PATCH 2/4] build: set PERL
2022-06-01 16:59 [XEN PATCH 0/4] xen: rework compat headers generation Anthony PERARD
2022-06-01 16:59 ` [XEN PATCH 1/4] build: xen/include: use if_changed Anthony PERARD
@ 2022-06-01 16:59 ` Anthony PERARD
2022-06-02 9:01 ` Jan Beulich
2022-06-01 16:59 ` [XEN PATCH 3/4] build: replace get-fields.sh by a perl script Anthony PERARD
` (2 subsequent siblings)
4 siblings, 1 reply; 23+ messages in thread
From: Anthony PERARD @ 2022-06-01 16:59 UTC (permalink / raw)
To: xen-devel
Cc: Anthony PERARD, Andrew Cooper, George Dunlap, Jan Beulich,
Julien Grall, Stefano Stabellini, Wei Liu
We are going to use it in a moment.
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---
xen/Makefile | 1 +
1 file changed, 1 insertion(+)
diff --git a/xen/Makefile b/xen/Makefile
index 82f5310b12..a6650a2acc 100644
--- a/xen/Makefile
+++ b/xen/Makefile
@@ -22,6 +22,7 @@ PYTHON_INTERPRETER := $(word 1,$(shell which python3 python python2 2>/dev/null)
export PYTHON ?= $(PYTHON_INTERPRETER)
export CHECKPOLICY ?= checkpolicy
+export PERL ?= perl
$(if $(filter __%, $(MAKECMDGOALS)), \
$(error targets prefixed with '__' are only for internal use))
--
Anthony PERARD
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [XEN PATCH 3/4] build: replace get-fields.sh by a perl script
2022-06-01 16:59 [XEN PATCH 0/4] xen: rework compat headers generation Anthony PERARD
2022-06-01 16:59 ` [XEN PATCH 1/4] build: xen/include: use if_changed Anthony PERARD
2022-06-01 16:59 ` [XEN PATCH 2/4] build: set PERL Anthony PERARD
@ 2022-06-01 16:59 ` Anthony PERARD
2022-06-01 17:32 ` Elliott Mitchell
2022-06-01 16:59 ` [XEN PATCH 4/4] build: remove auto.conf prerequisite from compat/xlat.h target Anthony PERARD
2022-06-01 17:17 ` [XEN PATCH 0/4] xen: rework compat headers generation Andrew Cooper
4 siblings, 1 reply; 23+ messages in thread
From: Anthony PERARD @ 2022-06-01 16:59 UTC (permalink / raw)
To: xen-devel
Cc: Anthony PERARD, Andrew Cooper, George Dunlap, Jan Beulich,
Julien Grall, Stefano Stabellini, Wei Liu
The get-fields.sh which generate all the include/compat/.xlat/*.h
headers is quite slow. It takes for example nearly 3 seconds to
generate platform.h on a recent machine, or 2.3 seconds for memory.h.
Since it's only text processing, rewriting the mix of shell/sed/python
into a single perl script make the generation of those file a lot
faster.
I tried to keep a similar look for the code, to keep the code similar
between the shell and perl, and to ease review. So some code in perl
might look weird or could be written better.
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---
xen/include/Makefile | 6 +-
| 539 +++++++++++++++++++++++++++++++++++
xen/tools/get-fields.sh | 528 ----------------------------------
3 files changed, 541 insertions(+), 532 deletions(-)
create mode 100755 xen/tools/compat-xlat-header
delete mode 100644 xen/tools/get-fields.sh
diff --git a/xen/include/Makefile b/xen/include/Makefile
index 6d9bcc19b0..b7e7148665 100644
--- a/xen/include/Makefile
+++ b/xen/include/Makefile
@@ -60,9 +60,7 @@ cmd_compat_c = \
quiet_cmd_xlat_headers = GEN $@
cmd_xlat_headers = \
- while read what name; do \
- $(SHELL) $(srctree)/tools/get-fields.sh "$$what" compat_$$name $< || exit $$?; \
- done <$(patsubst $(obj)/compat/%,$(obj)/compat/.xlat/%,$(basename $<)).lst >$@.new; \
+ $(PERL) $(srctree)/tools/compat-xlat-header $< $(patsubst $(obj)/compat/%,$(obj)/compat/.xlat/%,$(basename $<)).lst > $@.new; \
mv -f $@.new $@
targets += $(headers-y)
@@ -80,7 +78,7 @@ $(obj)/compat/%.c: $(src)/public/%.h $(srcdir)/xlat.lst $(srctree)/tools/compat-
$(call if_changed,compat_c)
targets += $(patsubst compat/%, compat/.xlat/%, $(headers-y))
-$(obj)/compat/.xlat/%.h: $(obj)/compat/%.h $(obj)/compat/.xlat/%.lst $(srctree)/tools/get-fields.sh FORCE
+$(obj)/compat/.xlat/%.h: $(obj)/compat/%.h $(obj)/compat/.xlat/%.lst $(srctree)/tools/compat-xlat-header FORCE
$(call if_changed,xlat_headers)
quiet_cmd_xlat_lst = GEN $@
--git a/xen/tools/compat-xlat-header b/xen/tools/compat-xlat-header
new file mode 100755
index 0000000000..f1f42a9dde
--- /dev/null
+++ b/xen/tools/compat-xlat-header
@@ -0,0 +1,539 @@
+#!/usr/bin/perl -w
+
+use strict;
+use warnings;
+
+open COMPAT_LIST, "<$ARGV[1]" or die "can't open $ARGV[1], $!";
+open HEADER, "<$ARGV[0]" or die "can't open $ARGV[0], $!";
+
+my @typedefs;
+
+my @header_tokens;
+while (<HEADER>) {
+ next if m/^\s*#.*/;
+ s/([\]\[,;:{}])/ $1 /g;
+ s/^\s+//;
+ push(@header_tokens, split(/\s+/));
+}
+
+sub get_fields {
+ my ($looking_for) = @_;
+ my $level = 1;
+ my $aggr = 0;
+ my ($name, @fields);
+
+ foreach (@header_tokens) {
+ if (/^(struct|union)$/) {
+ unless ($level != 1) {
+ $aggr = 1;
+ @fields = ();
+ $name = '';
+ }
+ } elsif ($_ eq '{') {
+ $level++;
+ } elsif ($_ eq '}') {
+ $level--;
+ if ($level == 1 and $name eq $looking_for) {
+ push (@fields, $_);
+ return @fields;
+ }
+ } elsif (/^[a-zA-Z_].*/) {
+ unless ($aggr == 0 or $name ne "") {
+ $name = $_;
+ }
+ }
+ unless ($aggr == 0) {
+ push (@fields, $_);
+ }
+ }
+ return ();
+}
+
+sub get_typedefs {
+ my $level = 1;
+ my $state = 0;
+ my @typedefs;
+ foreach (@_) {
+ if ($_ eq 'typedef') {
+ unless ($level != 1) {
+ $state = 1;
+ }
+ } elsif (m/^COMPAT_HANDLE\(.*\)$/) {
+ unless ($level != 1 or $state != 1) {
+ $state = 2;
+ }
+ } elsif (m/^[\[\{]$/) {
+ $level++;
+ } elsif (m/^[\]\}]$/) {
+ $level--;
+ } elsif ($_ eq ';') {
+ unless ($level != 1) {
+ $state = 0;
+ }
+ } elsif (m/^[a-zA-Z_].*$/) {
+ unless ($level != 1 or $state != 2) {
+ push (@typedefs, $_);
+ }
+ }
+ }
+ return @typedefs;
+}
+
+sub build_enums {
+ my ($name, @tokens) = @_;
+
+ my $level = 1;
+ my $kind = '';
+ my $named = '';
+ my (@fields, @members, $id);
+
+ foreach (@tokens) {
+ if (m/^(struct|union)$/) {
+ unless ($level != 2) {
+ @fields = ('');
+ }
+ $kind="$_;$kind";
+ } elsif ($_ eq '{') {
+ $level++;
+ } elsif ($_ eq '}') {
+ $level--;
+ if ($level == 1) {
+ my $subkind = $kind;
+ $subkind =~ s/;.*//;
+ if ($subkind eq 'union') {
+ print "\nenum XLAT_$name {\n";
+ foreach (@members) {
+ print " XLAT_${name}_$_,\n";
+ }
+ print "};\n";
+ }
+ return;
+ } elsif ($level == 2) {
+ $named = '?';
+ }
+ } elsif (/^[a-zA-Z_].*$/) {
+ $id = $_;
+ my $k = $kind;
+ $k =~ s/.*?;//;
+ if ($named ne '' and $k ne '') {
+ shift @fields if @fields > 0 and $fields[0] eq '';
+ build_enums("${name}_$_", @fields);
+ $named = '!';
+ }
+ } elsif ($_ eq ',') {
+ unless ($level != 2) {
+ push (@members, $id);
+ }
+ } elsif ($_ eq ';') {
+ unless ($level != 2) {
+ push (@members, $id);
+ }
+ unless ($named eq '') {
+ $kind =~ s/.*?;//;
+ }
+ $named = '';
+ }
+ unless (@fields == 0) {
+ push (@fields, $_);
+ }
+ }
+}
+
+sub handle_field {
+ my ($prefix, $name, $id, $type, @fields) = @_;
+
+ if (@fields == 0) {
+ print " \\\n";
+ if ($type eq '') {
+ print "$prefix\(_d_)->$id = (_s_)->$id;"
+ } else {
+ my $k = $id;
+ $k =~ s/\./_/g;
+ print "${prefix}XLAT_${name}_HNDL_${k}(_d_, _s_);";
+ }
+ } elsif ("@fields" !~ m/[{}]/) {
+ my $tag = "@fields";
+ $tag =~ s/\s*(struct|union)\s+(compat_)?(\w+)\s.*/$3/;
+ print " \\\n";
+ print "${prefix}XLAT_$tag(&(_d_)->$id, &(_s_)->$id);"
+ } else {
+ my $func_id = $id;
+ my @func_tokens = @fields;
+ my $kind = '';
+ my $array = "";
+ my $level = 1;
+ my $arrlvl = 1;
+ my $array_type = '';
+ my $id = '';
+ my $type = '';
+ @fields = ();
+ foreach (@func_tokens) {
+ if (/^(struct|union)$/) {
+ unless ($level != 2) {
+ @fields = ('');
+ }
+ if ($level == 1) {
+ $kind = $_;
+ if ($kind eq 'union') {
+ my $tmp = $func_id;
+ $tmp =~ s/\./_/g;
+ print " \\\n";
+ print "${prefix}switch ($tmp) {"
+ }
+ }
+ } elsif ($_ eq '{') {
+ $level++;
+ $id = '';
+ } elsif ($_ eq '}') {
+ $level--;
+ $id = '';
+ if ($level == 1 and $kind eq 'union') {
+ print " \\\n";
+ print "$prefix}"
+ }
+ } elsif ($_ eq '[') {
+ if ($level != 2 or $arrlvl != 1) {
+ # skip
+ } elsif ($array eq '') {
+ $array = ' ';
+ } else {
+ $array = "$array;";
+ }
+ $arrlvl++;
+ } elsif ($_ eq ']') {
+ $arrlvl--;
+ } elsif (m/^COMPAT_HANDLE\((.*)\)$/) {
+ if ($level == 2 and $id eq '') {
+ $type = $1;
+ $type =~ s/^compat_//;
+ }
+ } elsif ($_ eq "compat_domain_handle_t") {
+ if ($level == 2 and $id eq '') {
+ $array_type = $_;
+ }
+ } elsif (m/^[a-zA-Z_].*$/) {
+ if ($id eq '' and $type eq '' and $array_type eq '') {
+ foreach $id (@typedefs) {
+ unless ($id ne $_) {
+ $type = $id;
+ }
+ }
+ if ($type eq '') {
+ $id = $_;
+ } else {
+ $id = '';
+ }
+ } else {
+ $id = $_;
+ }
+ } elsif (m/^[,;]$/) {
+ if ($level == 2 and $id !~ /^_pad\d*$/) {
+ if ($kind eq 'union') {
+ my $tmp = "$func_id.$id";
+ $tmp =~ s/\./_/g;
+ print " \\\n";
+ print "${prefix}case XLAT_${name}_$tmp:";
+ shift @fields if @fields > 0 and $fields[0] eq '';
+ handle_field("$prefix ", $name, "$func_id.$id", $type, @fields);
+ } elsif ($array eq '' and $array_type eq '') {
+ shift @fields if @fields > 0 and $fields[0] eq '';
+ handle_field($prefix, $name, "$func_id.$id", $type, @fields);
+ } elsif ($array eq '') {
+ copy_array(" ", "$func_id.$id");
+ } else {
+ $array =~ s/^.*?;//;
+ shift @fields if @fields > 0 and $fields[0] eq '';
+ handle_array($prefix, $name, "$func_id.$id", $array, $type, @fields);
+ }
+ unless ($_ ne ';') {
+ @fields = ();
+ $id = '';
+ $type = '';
+ }
+ $array = '';
+ if ($kind eq 'union') {
+ print " \\\n";
+ print "$prefix break;";
+ }
+ }
+ } else {
+ if ($array ne '') {
+ $array = "$array $_";
+ }
+ }
+ unless (@fields == 0) {
+ push (@fields, $_);
+ }
+ }
+ }
+}
+
+sub copy_array {
+ my ($prefix, $id) = @_;
+
+ print " \\\n";
+ print "${prefix}if ((_d_)->$id != (_s_)->$id) \\\n";
+ print "$prefix memcpy((_d_)->$id, (_s_)->$id, sizeof((_d_)->$id));";
+}
+
+sub handle_array {
+ my ($prefix, $name, $id, $array, $type, @fields) = @_;
+
+ my $i = $array;
+ $i =~ s/[^;]//g;
+ $i = length($i);
+ $i = "i$i";
+
+ print " \\\n";
+ print "$prefix\{ \\\n";
+ print "$prefix unsigned int $i; \\\n";
+ my $tmp = $array;
+ $tmp =~ s/;.*$//;
+ $tmp =~ s/^\s*(.*)\s*$/$1/;
+ print "$prefix for ($i = 0; $i < $tmp; ++$i) {";
+ if ($array !~ m/^.*?;/) {
+ handle_field("$prefix ", $name, "$id\[$i]", $type, @fields);
+ } else {
+ handle_array("$prefix " ,$name, "$id\[$i]", $', $type, @fields);
+ }
+ print " \\\n";
+ print "$prefix } \\\n";
+ print "$prefix\}";
+}
+
+sub build_body {
+ my ($name, @tokens) = @_;
+ my $level = 1;
+ my $id = '';
+ my $array = '';
+ my $arrlvl = 1;
+ my $array_type = '';
+ my $type = '';
+ my @fields;
+
+ printf "\n#define XLAT_$name(_d_, _s_) do {";
+
+ foreach (@tokens) {
+ if (/^(struct|union)$/) {
+ unless ($level != 2) {
+ @fields = ('');
+ }
+ } elsif ($_ eq '{') {
+ $level++;
+ $id = '';
+ } elsif ($_ eq '}') {
+ $level--;
+ $id = '';
+ } elsif ($_ eq '[') {
+ if ($level != 2 or $arrlvl != 1) {
+ # skip
+ } elsif ($array eq '') {
+ $array = ' ';
+ } else {
+ $array = "$array;";
+ }
+ $arrlvl++;
+ } elsif ($_ eq ']') {
+ $arrlvl--;
+ } elsif (m/^COMPAT_HANDLE\((.*)\)$/) {
+ if ($level == 2 and $id eq '') {
+ $type = $1;
+ $type =~ s/^compat_//;
+ }
+ } elsif ($_ eq "compat_domain_handle_t") {
+ if ($level == 2 and $id eq '') {
+ $array_type = $_;
+ }
+ } elsif (m/^[a-zA-Z_].*$/) {
+ if ($array ne '') {
+ $array = "$array $_";
+ } elsif ($id eq '' and $type eq '' and $array_type eq '') {
+ foreach $id (@typedefs) {
+ unless ($id eq $_) {
+ $type = $id;
+ }
+ }
+ if ($type eq '') {
+ $id = $_;
+ } else {
+ $id = '';
+ }
+ } else {
+ $id = $_;
+ }
+ } elsif (m/^[,;]$/) {
+ if ($level == 2 and $id !~ /^_pad\d*$/) {
+ if ($array eq '' and $array_type eq '') {
+ shift @fields if @fields > 0 and $fields[0] eq '';
+ handle_field(" ", $name, $id, $type, @fields);
+ } elsif ($array eq '') {
+ copy_array(" ", $id);
+ } else {
+ my $tmp = $array;
+ $tmp =~ s/^.*?;//;
+ shift @fields if @fields > 0 and $fields[0] eq '';
+ handle_array(" ", $name, $id, $tmp, $type, @fields);
+ }
+ unless ($_ ne ';') {
+ @fields = ();
+ $id = '';
+ $type = '';
+ }
+ $array = '';
+ }
+ } else {
+ if ($array ne '') {
+ $array = "$array $_";
+ }
+ }
+ unless (@fields == 0) {
+ push (@fields, $_);
+ }
+ }
+ print " \\\n} while (0)\n";
+}
+
+sub check_field {
+ my ($kind, $name, $field, @extrafields) = @_;
+
+ if ("@extrafields" !~ m/[{}]/) {
+ print "; \\\n";
+ if (@extrafields != 0) {
+ foreach (@extrafields) {
+ if (m/^(struct|union)$/) {
+ # skip
+ } elsif (m/^[a-zA-Z_].*/) {
+ s/^xen_//;
+ print " CHECK_$_";
+ last;
+ } else {
+ die "Malformed compound declaration: '$_'";
+ }
+ }
+ } elsif ($field !~ m/\./) {
+ print " CHECK_FIELD_($kind, $name, $field)";
+ } else {
+ my $n = $field =~ s/\./, /g;
+ print " CHECK_SUBFIELD_${n}_($kind, $name, $field)";
+ }
+ } else {
+ my $level = 1;
+ my @fields = ();
+ my $id = '';
+
+ foreach (@extrafields) {
+ if (m/^(struct|union)$/) {
+ unless ($level != 2) {
+ @fields = ('');
+ }
+ } elsif ($_ eq '{') {
+ $level++;
+ $id = '';
+ } elsif ($_ eq '}') {
+ $level--;
+ $id = '';
+ } elsif (/^compat_.*_t$/) {
+ if ($level == 2) {
+ @fields = ('');
+ s/_t$//;
+ s/^compat_//;
+ }
+ } elsif (/^evtchn_.*_compat_t$/) {
+ if ($level == 2 and $_ ne "evtchn_port_compat_t") {
+ @fields = ('');
+ s/_compat_t$//;
+ }
+ } elsif (/^[a-zA-Z_].*$/) {
+ $id = $_;
+ } elsif (/^[,;]$/) {
+ if ($level == 2 and $id !~ /^_pad\d*$/) {
+ shift @fields if @fields > 0 and $fields[0] eq '';
+ check_field($kind, $name, "$field.$id", @fields);
+ unless ($_ ne ";") {
+ @fields = ();
+ $id = '';
+ }
+ }
+ }
+ unless (@fields == 0) {
+ push (@fields, $_);
+ }
+ }
+ }
+}
+
+sub build_check {
+ my ($name, @tokens) = @_;
+ my $level = 1;
+ my (@fields, $kind, $id);
+ my $arrlvl = 1;
+
+ print "\n";
+ print "#define CHECK_$name \\\n";
+
+ foreach (@tokens) {
+ if (/^(struct|union)$/) {
+ if ($level == 1) {
+ $kind = $_;
+ print " CHECK_SIZE_($kind, $name)";
+ } elsif ($level == 2) {
+ @fields = ('');
+ }
+ } elsif ($_ eq '{') {
+ $level++;
+ $id = '';
+ } elsif ($_ eq '}') {
+ $level--;
+ $id = '';
+ } elsif ($_ eq '[') {
+ $arrlvl++;
+ } elsif ($_ eq ']') {
+ $arrlvl--;
+ } elsif (/^compat_.*_t$/) {
+ if ($level == 2 and $_ ne "compat_argo_port_t") {
+ @fields = ('');
+ s/_t$//;
+ s/^compat_//;
+ }
+ } elsif (/^[a-zA-Z_].*$/) {
+ unless ($level != 2 or $arrlvl != 1) {
+ $id = $_;
+ }
+ } elsif (/^[,;]$/) {
+ if ($level == 2 and $id !~ /^_pad\d*$/) {
+ shift @fields if @fields > 0 and $fields[0] eq '';
+ check_field($kind, $name, $id, @fields);
+ unless ($_ ne ";") {
+ @fields = ();
+ $id = '';
+ }
+ }
+ }
+
+ unless (@fields == 0) {
+ push (@fields, $_);
+ }
+ }
+ print "\n";
+}
+
+@typedefs = get_typedefs(@header_tokens);
+
+while (<COMPAT_LIST>) {
+ my ($what, $name) = split(/\s+/, $_);
+ $name =~ s/^xen//;
+
+ my @fields = get_fields("compat_$name");
+ if (@fields == 0) {
+ die "Fields of 'compat_$name' not found in '$ARGV[1]'";
+ }
+
+ if ($what eq "!") {
+ build_enums($name, @fields);
+ build_body($name, @fields);
+ } elsif ($what eq "?") {
+ build_check($name, @fields);
+ } else {
+ die "Invalid translation indicator: '$what'";
+ }
+}
diff --git a/xen/tools/get-fields.sh b/xen/tools/get-fields.sh
deleted file mode 100644
index 002db2093f..0000000000
--- a/xen/tools/get-fields.sh
+++ /dev/null
@@ -1,528 +0,0 @@
-test -n "$1" -a -n "$2" -a -n "$3"
-set -ef
-
-SED=sed
-if test -x /usr/xpg4/bin/sed; then
- SED=/usr/xpg4/bin/sed
-fi
-if test -z ${PYTHON}; then
- PYTHON=`/usr/bin/env python`
-fi
-if test -z ${PYTHON}; then
- echo "Python not found"
- exit 1
-fi
-
-get_fields ()
-{
- local level=1 aggr=0 name= fields=
- for token in $2
- do
- case "$token" in
- struct|union)
- test $level != 1 || aggr=1 fields= name=
- ;;
- "{")
- level=$(expr $level + 1)
- ;;
- "}")
- level=$(expr $level - 1)
- if [ $level = 1 -a $name = $1 ]
- then
- echo "$fields }"
- return 0
- fi
- ;;
- [a-zA-Z_]*)
- test $aggr = 0 -o -n "$name" || name="$token"
- ;;
- esac
- test $aggr = 0 || fields="$fields $token"
- done
-}
-
-get_typedefs ()
-{
- local level=1 state=
- for token in $1
- do
- case "$token" in
- typedef)
- test $level != 1 || state=1
- ;;
- COMPAT_HANDLE\(*\))
- test $level != 1 -o "$state" != 1 || state=2
- ;;
- [\{\[])
- level=$(expr $level + 1)
- ;;
- [\}\]])
- level=$(expr $level - 1)
- ;;
- ";")
- test $level != 1 || state=
- ;;
- [a-zA-Z_]*)
- test $level != 1 -o "$state" != 2 || echo "$token"
- ;;
- esac
- done
-}
-
-build_enums ()
-{
- local level=1 kind= fields= members= named= id= token
- for token in $2
- do
- case "$token" in
- struct|union)
- test $level != 2 || fields=" "
- kind="$token;$kind"
- ;;
- "{")
- level=$(expr $level + 1)
- ;;
- "}")
- level=$(expr $level - 1)
- if [ $level = 1 ]
- then
- if [ "${kind%%;*}" = union ]
- then
- echo
- echo "enum XLAT_$1 {"
- for m in $members
- do
- echo " XLAT_${1}_$m,"
- done
- echo "};"
- fi
- return 0
- elif [ $level = 2 ]
- then
- named='?'
- fi
- ;;
- [a-zA-Z]*)
- id=$token
- if [ -n "$named" -a -n "${kind#*;}" ]
- then
- build_enums ${1}_$token "$fields"
- named='!'
- fi
- ;;
- ",")
- test $level != 2 || members="$members $id"
- ;;
- ";")
- test $level != 2 || members="$members $id"
- test -z "$named" || kind=${kind#*;}
- named=
- ;;
- esac
- test -z "$fields" || fields="$fields $token"
- done
-}
-
-handle_field ()
-{
- if [ -z "$5" ]
- then
- echo " \\"
- if [ -z "$4" ]
- then
- printf %s "$1(_d_)->$3 = (_s_)->$3;"
- else
- printf %s "$1XLAT_${2}_HNDL_$(echo $3 | $SED 's,\.,_,g')(_d_, _s_);"
- fi
- elif [ -z "$(echo "$5" | $SED 's,[^{}],,g')" ]
- then
- local tag=$(echo "$5" | ${PYTHON} -c '
-import re,sys
-for line in sys.stdin.readlines():
- sys.stdout.write(re.subn(r"\s*(struct|union)\s+(compat_)?(\w+)\s.*", r"\3", line)[0].rstrip() + "\n")
-')
- echo " \\"
- printf %s "${1}XLAT_$tag(&(_d_)->$3, &(_s_)->$3);"
- else
- local level=1 kind= fields= id= array= arrlvl=1 array_type= type= token
- for token in $5
- do
- case "$token" in
- struct|union)
- test $level != 2 || fields=" "
- if [ $level = 1 ]
- then
- kind=$token
- if [ $kind = union ]
- then
- echo " \\"
- printf %s "${1}switch ($(echo $3 | $SED 's,\.,_,g')) {"
- fi
- fi
- ;;
- "{")
- level=$(expr $level + 1) id=
- ;;
- "}")
- level=$(expr $level - 1) id=
- if [ $level = 1 -a $kind = union ]
- then
- echo " \\"
- printf %s "$1}"
- fi
- ;;
- "[")
- if [ $level != 2 -o $arrlvl != 1 ]
- then
- :
- elif [ -z "$array" ]
- then
- array=" "
- else
- array="$array;"
- fi
- arrlvl=$(expr $arrlvl + 1)
- ;;
- "]")
- arrlvl=$(expr $arrlvl - 1)
- ;;
- COMPAT_HANDLE\(*\))
- if [ $level = 2 -a -z "$id" ]
- then
- type=${token#COMPAT_HANDLE?}
- type=${type%?}
- type=${type#compat_}
- fi
- ;;
- compat_domain_handle_t)
- if [ $level = 2 -a -z "$id" ]
- then
- array_type=$token
- fi
- ;;
- [a-zA-Z]*)
- if [ -z "$id" -a -z "$type" -a -z "$array_type" ]
- then
- for id in $typedefs
- do
- test $id != "$token" || type=$id
- done
- if [ -z "$type" ]
- then
- id=$token
- else
- id=
- fi
- else
- id=$token
- fi
- ;;
- [\,\;])
- if [ $level = 2 -a -n "$(echo $id | $SED 's,^_pad[[:digit:]]*,,')" ]
- then
- if [ $kind = union ]
- then
- echo " \\"
- printf %s "${1}case XLAT_${2}_$(echo $3.$id | $SED 's,\.,_,g'):"
- handle_field "$1 " $2 $3.$id "$type" "$fields"
- elif [ -z "$array" -a -z "$array_type" ]
- then
- handle_field "$1" $2 $3.$id "$type" "$fields"
- elif [ -z "$array" ]
- then
- copy_array " " $3.$id
- else
- handle_array "$1" $2 $3.$id "${array#*;}" "$type" "$fields"
- fi
- test "$token" != ";" || fields= id= type=
- array=
- if [ $kind = union ]
- then
- echo " \\"
- printf %s "$1 break;"
- fi
- fi
- ;;
- *)
- if [ -n "$array" ]
- then
- array="$array $token"
- fi
- ;;
- esac
- test -z "$fields" || fields="$fields $token"
- done
- fi
-}
-
-copy_array ()
-{
- echo " \\"
- echo "${1}if ((_d_)->$2 != (_s_)->$2) \\"
- printf %s "$1 memcpy((_d_)->$2, (_s_)->$2, sizeof((_d_)->$2));"
-}
-
-handle_array ()
-{
- local i="i$(echo $4 | $SED 's,[^;], ,g' | wc -w | $SED 's,[[:space:]]*,,g')"
- echo " \\"
- echo "$1{ \\"
- echo "$1 unsigned int $i; \\"
- printf %s "$1 for ($i = 0; $i < "${4%%;*}"; ++$i) {"
- if [ "$4" = "${4#*;}" ]
- then
- handle_field "$1 " $2 $3[$i] "$5" "$6"
- else
- handle_array "$1 " $2 $3[$i] "${4#*;}" "$5" "$6"
- fi
- echo " \\"
- echo "$1 } \\"
- printf %s "$1}"
-}
-
-build_body ()
-{
- echo
- printf %s "#define XLAT_$1(_d_, _s_) do {"
- local level=1 fields= id= array= arrlvl=1 array_type= type= token
- for token in $2
- do
- case "$token" in
- struct|union)
- test $level != 2 || fields=" "
- ;;
- "{")
- level=$(expr $level + 1) id=
- ;;
- "}")
- level=$(expr $level - 1) id=
- ;;
- "[")
- if [ $level != 2 -o $arrlvl != 1 ]
- then
- :
- elif [ -z "$array" ]
- then
- array=" "
- else
- array="$array;"
- fi
- arrlvl=$(expr $arrlvl + 1)
- ;;
- "]")
- arrlvl=$(expr $arrlvl - 1)
- ;;
- COMPAT_HANDLE\(*\))
- if [ $level = 2 -a -z "$id" ]
- then
- type=${token#COMPAT_HANDLE?}
- type=${type%?}
- type=${type#compat_}
- fi
- ;;
- compat_domain_handle_t)
- if [ $level = 2 -a -z "$id" ]
- then
- array_type=$token
- fi
- ;;
- [a-zA-Z_]*)
- if [ -n "$array" ]
- then
- array="$array $token"
- elif [ -z "$id" -a -z "$type" -a -z "$array_type" ]
- then
- for id in $typedefs
- do
- test $id != "$token" || type=$id
- done
- if [ -z "$type" ]
- then
- id=$token
- else
- id=
- fi
- else
- id=$token
- fi
- ;;
- [\,\;])
- if [ $level = 2 -a -n "$(echo $id | $SED 's,^_pad[[:digit:]]*,,')" ]
- then
- if [ -z "$array" -a -z "$array_type" ]
- then
- handle_field " " $1 $id "$type" "$fields"
- elif [ -z "$array" ]
- then
- copy_array " " $id
- else
- handle_array " " $1 $id "${array#*;}" "$type" "$fields"
- fi
- test "$token" != ";" || fields= id= type=
- array=
- fi
- ;;
- *)
- if [ -n "$array" ]
- then
- array="$array $token"
- fi
- ;;
- esac
- test -z "$fields" || fields="$fields $token"
- done
- echo " \\"
- echo "} while (0)"
-}
-
-check_field ()
-{
- if [ -z "$(echo "$4" | $SED 's,[^{}],,g')" ]
- then
- echo "; \\"
- local n=$(echo $3 | $SED 's,[^.], ,g' | wc -w | $SED 's,[[:space:]]*,,g')
- if [ -n "$4" ]
- then
- for n in $4
- do
- case $n in
- struct|union)
- ;;
- [a-zA-Z_]*)
- printf %s " CHECK_${n#xen_}"
- break
- ;;
- *)
- echo "Malformed compound declaration: '$n'" >&2
- exit 1
- ;;
- esac
- done
- elif [ $n = 0 ]
- then
- printf %s " CHECK_FIELD_($1, $2, $3)"
- else
- printf %s " CHECK_SUBFIELD_${n}_($1, $2, $(echo $3 | $SED 's!\.!, !g'))"
- fi
- else
- local level=1 fields= id= token
- for token in $4
- do
- case "$token" in
- struct|union)
- test $level != 2 || fields=" "
- ;;
- "{")
- level=$(expr $level + 1) id=
- ;;
- "}")
- level=$(expr $level - 1) id=
- ;;
- compat_*_t)
- if [ $level = 2 ]
- then
- fields=" "
- token="${token%_t}"
- token="${token#compat_}"
- fi
- ;;
- evtchn_*_compat_t)
- if [ $level = 2 -a $token != evtchn_port_compat_t ]
- then
- fields=" "
- token="${token%_compat_t}"
- fi
- ;;
- [a-zA-Z]*)
- id=$token
- ;;
- [\,\;])
- if [ $level = 2 -a -n "$(echo $id | $SED 's,^_pad[[:digit:]]*,,')" ]
- then
- check_field $1 $2 $3.$id "$fields"
- test "$token" != ";" || fields= id=
- fi
- ;;
- esac
- test -z "$fields" || fields="$fields $token"
- done
- fi
-}
-
-build_check ()
-{
- echo
- echo "#define CHECK_$1 \\"
- local level=1 fields= kind= id= arrlvl=1 token
- for token in $2
- do
- case "$token" in
- struct|union)
- if [ $level = 1 ]
- then
- kind=$token
- printf %s " CHECK_SIZE_($kind, $1)"
- elif [ $level = 2 ]
- then
- fields=" "
- fi
- ;;
- "{")
- level=$(expr $level + 1) id=
- ;;
- "}")
- level=$(expr $level - 1) id=
- ;;
- "[")
- arrlvl=$(expr $arrlvl + 1)
- ;;
- "]")
- arrlvl=$(expr $arrlvl - 1)
- ;;
- compat_*_t)
- if [ $level = 2 -a $token != compat_argo_port_t ]
- then
- fields=" "
- token="${token%_t}"
- token="${token#compat_}"
- fi
- ;;
- [a-zA-Z_]*)
- test $level != 2 -o $arrlvl != 1 || id=$token
- ;;
- [\,\;])
- if [ $level = 2 -a -n "$(echo $id | $SED 's,^_pad[[:digit:]]*,,')" ]
- then
- check_field $kind $1 $id "$fields"
- test "$token" != ";" || fields= id=
- fi
- ;;
- esac
- test -z "$fields" || fields="$fields $token"
- done
- echo ""
-}
-
-list="$($SED -e 's,^[[:space:]]#.*,,' -e 's!\([]\[,;:{}]\)! \1 !g' $3)"
-fields="$(get_fields $(echo $2 | $SED 's,^compat_xen,compat_,') "$list")"
-if [ -z "$fields" ]
-then
- echo "Fields of '$2' not found in '$3'" >&2
- exit 1
-fi
-name=${2#compat_}
-name=${name#xen}
-case "$1" in
-"!")
- typedefs="$(get_typedefs "$list")"
- build_enums $name "$fields"
- build_body $name "$fields"
- ;;
-"?")
- build_check $name "$fields"
- ;;
-*)
- echo "Invalid translation indicator: '$1'" >&2
- exit 1
- ;;
-esac
--
Anthony PERARD
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [XEN PATCH 4/4] build: remove auto.conf prerequisite from compat/xlat.h target
2022-06-01 16:59 [XEN PATCH 0/4] xen: rework compat headers generation Anthony PERARD
` (2 preceding siblings ...)
2022-06-01 16:59 ` [XEN PATCH 3/4] build: replace get-fields.sh by a perl script Anthony PERARD
@ 2022-06-01 16:59 ` Anthony PERARD
2022-06-02 9:16 ` Jan Beulich
2022-06-01 17:17 ` [XEN PATCH 0/4] xen: rework compat headers generation Andrew Cooper
4 siblings, 1 reply; 23+ messages in thread
From: Anthony PERARD @ 2022-06-01 16:59 UTC (permalink / raw)
To: xen-devel
Cc: Anthony PERARD, Andrew Cooper, George Dunlap, Jan Beulich,
Julien Grall, Stefano Stabellini, Wei Liu
Now that the command line generating "xlat.h" is check on rebuild, the
header will be regenerated whenever the list of xlat headers changes
due to change in ".config". We don't need to force a regeneration for
every changes in ".config".
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---
xen/include/Makefile | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/xen/include/Makefile b/xen/include/Makefile
index b7e7148665..937a8bc884 100644
--- a/xen/include/Makefile
+++ b/xen/include/Makefile
@@ -99,7 +99,7 @@ cmd_xlat_h = \
cat $(filter %.h,$^) >$@.new; \
mv -f $@.new $@
-$(obj)/compat/xlat.h: $(addprefix $(obj)/compat/.xlat/,$(xlat-y)) $(obj)/config/auto.conf FORCE
+$(obj)/compat/xlat.h: $(addprefix $(obj)/compat/.xlat/,$(xlat-y)) FORCE
$(call if_changed,xlat_h)
ifeq ($(XEN_TARGET_ARCH),$(XEN_COMPILE_ARCH))
--
Anthony PERARD
^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [XEN PATCH 0/4] xen: rework compat headers generation
2022-06-01 16:59 [XEN PATCH 0/4] xen: rework compat headers generation Anthony PERARD
` (3 preceding siblings ...)
2022-06-01 16:59 ` [XEN PATCH 4/4] build: remove auto.conf prerequisite from compat/xlat.h target Anthony PERARD
@ 2022-06-01 17:17 ` Andrew Cooper
2022-06-06 13:24 ` Anthony PERARD
4 siblings, 1 reply; 23+ messages in thread
From: Andrew Cooper @ 2022-06-01 17:17 UTC (permalink / raw)
To: Anthony Perard, xen-devel
Cc: Julien Grall, Jan Beulich, Wei Liu, George Dunlap, Stefano Stabellini
On 01/06/2022 17:59, Anthony PERARD wrote:
> Patch series available in this git branch:
> https://xenbits.xen.org/git-http/people/aperard/xen-unstable.git br.build-system-xen-include-rework-v1
>
> Hi,
>
> This patch series is about 2 improvement. First one is to use $(if_changed, )
> in "include/Makefile" to make the generation of the compat headers less verbose
> and to have the command line part of the decision to rebuild the headers.
> Second one is to replace one slow script by a much faster one, and save time
> when generating the headers.
>
> Thanks.
>
> Anthony PERARD (4):
> build: xen/include: use if_changed
> build: set PERL
> build: replace get-fields.sh by a perl script
> build: remove auto.conf prerequisite from compat/xlat.h target
>
> xen/Makefile | 1 +
> xen/include/Makefile | 106 ++++---
> xen/tools/compat-xlat-header | 539 +++++++++++++++++++++++++++++++++++
> xen/tools/get-fields.sh | 528 ----------------------------------
Excellent. I was planning to ask you about this. (I also need to
refreshing my half series cleaning up other bits of the build.)
One trivial observation is that it would probably be nicer to name the
script with a .pl extension.
Any numbers on what the speedup in patch 3 is?
Are the generated compat headers identical before and after this
series? If yes, I'm very tempted to ack and commit it straight away.
~Andrew
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [XEN PATCH 3/4] build: replace get-fields.sh by a perl script
2022-06-01 16:59 ` [XEN PATCH 3/4] build: replace get-fields.sh by a perl script Anthony PERARD
@ 2022-06-01 17:32 ` Elliott Mitchell
2022-06-06 13:51 ` Anthony PERARD
0 siblings, 1 reply; 23+ messages in thread
From: Elliott Mitchell @ 2022-06-01 17:32 UTC (permalink / raw)
To: Anthony PERARD
Cc: xen-devel, Andrew Cooper, George Dunlap, Jan Beulich,
Julien Grall, Stefano Stabellini, Wei Liu
On Wed, Jun 01, 2022 at 05:59:08PM +0100, Anthony PERARD wrote:
> diff --git a/xen/tools/compat-xlat-header b/xen/tools/compat-xlat-header
> new file mode 100755
> index 0000000000..f1f42a9dde
> --- /dev/null
> +++ b/xen/tools/compat-xlat-header
> @@ -0,0 +1,539 @@
> +#!/usr/bin/perl -w
> +
> +use strict;
> +use warnings;
I hope to take more of a look at this, but one thing I immediately
notice: -w is redundant with "use warnings;". I strongly prefer
"use warnings;", but others may have different preferences.
--
(\___(\___(\______ --=> 8-) EHM <=-- ______/)___/)___/)
\BS ( | ehem+sigmsg@m5p.com PGP 87145445 | ) /
\_CS\ | _____ -O #include <stddisclaimer.h> O- _____ | / _/
8A19\___\_|_/58D2 7E3D DDF4 7BA6 <-PGP-> 41D1 B375 37D0 8714\_|_/___/5445
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [XEN PATCH 2/4] build: set PERL
2022-06-01 16:59 ` [XEN PATCH 2/4] build: set PERL Anthony PERARD
@ 2022-06-02 9:01 ` Jan Beulich
2022-06-06 13:47 ` Anthony PERARD
0 siblings, 1 reply; 23+ messages in thread
From: Jan Beulich @ 2022-06-02 9:01 UTC (permalink / raw)
To: Anthony PERARD
Cc: Andrew Cooper, George Dunlap, Julien Grall, Stefano Stabellini,
Wei Liu, xen-devel
On 01.06.2022 18:59, Anthony PERARD wrote:
> --- a/xen/Makefile
> +++ b/xen/Makefile
> @@ -22,6 +22,7 @@ PYTHON_INTERPRETER := $(word 1,$(shell which python3 python python2 2>/dev/null)
> export PYTHON ?= $(PYTHON_INTERPRETER)
>
> export CHECKPOLICY ?= checkpolicy
> +export PERL ?= perl
For the intended use, is there a minimum version requirement? If so,
it needs documenting in ./README (and it preferably wouldn't be any
newer than from around the times our other dependencies are). And
even when the uses are fully backwards compatible, I think the need
for the tool wants mentioning there.
Jan
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [XEN PATCH 1/4] build: xen/include: use if_changed
2022-06-01 16:59 ` [XEN PATCH 1/4] build: xen/include: use if_changed Anthony PERARD
@ 2022-06-02 9:11 ` Jan Beulich
2022-06-06 13:39 ` Anthony PERARD
2022-06-09 10:16 ` Bertrand Marquis
1 sibling, 1 reply; 23+ messages in thread
From: Jan Beulich @ 2022-06-02 9:11 UTC (permalink / raw)
To: Anthony PERARD
Cc: Andrew Cooper, George Dunlap, Julien Grall, Stefano Stabellini,
Wei Liu, xen-devel
On 01.06.2022 18:59, Anthony PERARD wrote:
> Use "define" for the headers*_chk commands as otherwise the "#"
> is interpreted as a comment and make can't find the end of
> $(foreach,).
In cmd_xlat_lst you use $(pound) - any reason this doesn't work in
these rules? Note that I don't mind the use of "define", just that I'm
puzzled by the justification.
Jan
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [XEN PATCH 4/4] build: remove auto.conf prerequisite from compat/xlat.h target
2022-06-01 16:59 ` [XEN PATCH 4/4] build: remove auto.conf prerequisite from compat/xlat.h target Anthony PERARD
@ 2022-06-02 9:16 ` Jan Beulich
0 siblings, 0 replies; 23+ messages in thread
From: Jan Beulich @ 2022-06-02 9:16 UTC (permalink / raw)
To: Anthony PERARD
Cc: Andrew Cooper, George Dunlap, Julien Grall, Stefano Stabellini,
Wei Liu, xen-devel
On 01.06.2022 18:59, Anthony PERARD wrote:
> Now that the command line generating "xlat.h" is check on rebuild, the
> header will be regenerated whenever the list of xlat headers changes
> due to change in ".config". We don't need to force a regeneration for
> every changes in ".config".
This looks to be dependent on only patch 1; I wonder if it shouldn't be
viewed as an integral part of that adjustment. Anyway - if you want to
keep it on its own:
Acked-by: Jan Beulich <jbeulich@suse.com>
Jan
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [XEN PATCH 0/4] xen: rework compat headers generation
2022-06-01 17:17 ` [XEN PATCH 0/4] xen: rework compat headers generation Andrew Cooper
@ 2022-06-06 13:24 ` Anthony PERARD
0 siblings, 0 replies; 23+ messages in thread
From: Anthony PERARD @ 2022-06-06 13:24 UTC (permalink / raw)
To: Andrew Cooper
Cc: xen-devel, Julien Grall, Jan Beulich, Wei Liu, George Dunlap,
Stefano Stabellini
On Wed, Jun 01, 2022 at 05:17:36PM +0000, Andrew Cooper wrote:
> On 01/06/2022 17:59, Anthony PERARD wrote:
> > Patch series available in this git branch:
> > https://xenbits.xen.org/git-http/people/aperard/xen-unstable.git br.build-system-xen-include-rework-v1
> >
> > Hi,
> >
> > This patch series is about 2 improvement. First one is to use $(if_changed, )
> > in "include/Makefile" to make the generation of the compat headers less verbose
> > and to have the command line part of the decision to rebuild the headers.
> > Second one is to replace one slow script by a much faster one, and save time
> > when generating the headers.
> >
> > Thanks.
> >
> > Anthony PERARD (4):
> > build: xen/include: use if_changed
> > build: set PERL
> > build: replace get-fields.sh by a perl script
> > build: remove auto.conf prerequisite from compat/xlat.h target
> >
> > xen/Makefile | 1 +
> > xen/include/Makefile | 106 ++++---
> > xen/tools/compat-xlat-header | 539 +++++++++++++++++++++++++++++++++++
> > xen/tools/get-fields.sh | 528 ----------------------------------
>
> Excellent. I was planning to ask you about this. (I also need to
> refreshing my half series cleaning up other bits of the build.)
>
> One trivial observation is that it would probably be nicer to name the
> script with a .pl extension.
Sound fine, I don't think it matter much here.
> Any numbers on what the speedup in patch 3 is?
Yes, on my machine when doing a full build, with `ccache` enabled, it
saves about 1.17 seconds (out of ~17s), and without ccache, it saves
about 2.0 seconds (out of ~37s).
Without ccache:
before:
$ for i in `seq 4`; do time make -j$(nproc) -s O=build 2>/dev/null >/dev/null; rm -r build; done
make --no-print-directory -j$(nproc) -s O=build > /dev/null 244.98s user 29.24s system 683% cpu 40.146 total
make --no-print-directory -j$(nproc) -s O=build > /dev/null 47.05s user 11.50s system 332% cpu 17.610 total
make --no-print-directory -j$(nproc) -s O=build > /dev/null 47.35s user 11.22s system 330% cpu 17.733 total
make --no-print-directory -j$(nproc) -s O=build > /dev/null 47.31s user 11.23s system 333% cpu 17.577 total
after:
$ for i in `seq 4`; do time make -j$(nproc) -s O=build 2>/dev/null>/dev/null; rm -r build; done
make --no-print-directory -j$(nproc) -s O=build 2> /dev/null > /dev/null 237.28s user 25.97s system 667% cpu 39.456 total
make --no-print-directory -j$(nproc) -s O=build 2> /dev/null > /dev/null 37.60s user 8.20s system 282% cpu 16.214 total
make --no-print-directory -j$(nproc) -s O=build 2> /dev/null > /dev/null 37.95s user 8.67s system 279% cpu 16.651 total
make --no-print-directory -j$(nproc) -s O=build 2> /dev/null > /dev/null 38.02s user 8.40s system 280% cpu 16.545 total
And without ccache:
before:
$ for i in `seq 4`; do time make -j$(nproc) -s O=build 2>/dev/null>/dev/null; rm -r build; done
make --no-print-directory -j$(nproc) -s O=build 2> /dev/null > /dev/null 206.37s user 22.19s system 640% cpu 35.695 total
make --no-print-directory -j$(nproc) -s O=build 2> /dev/null > /dev/null 221.45s user 22.26s system 667% cpu 36.537 total
make --no-print-directory -j$(nproc) -s O=build 2> /dev/null > /dev/null 233.95s user 23.80s system 686% cpu 37.518 total
make --no-print-directory -j$(nproc) -s O=build 2> /dev/null > /dev/null 234.27s user 23.83s system 680% cpu 37.923 total
after:
$ for i in `seq 4`; do time make -j$(nproc) -s O=build 2>/dev/null>/dev/null; rm -r build; done
make --no-print-directory -j$(nproc) -s O=build 2> /dev/null > /dev/null 198.62s user 18.64s system 642% cpu 33.841 total
make --no-print-directory -j$(nproc) -s O=build 2> /dev/null > /dev/null 202.91s user 19.46s system 655% cpu 33.912 total
make --no-print-directory -j$(nproc) -s O=build 2> /dev/null > /dev/null 224.42s user 20.89s system 680% cpu 36.025 total
make --no-print-directory -j$(nproc) -s O=build 2> /dev/null > /dev/null 222.93s user 21.29s system 683% cpu 35.708 total
> Are the generated compat headers identical before and after this
> series? If yes, I'm very tempted to ack and commit it straight away.
Yes, the headers are identical. Hopefully, I've managed to check with
all compat headers enabled, since that depends on .config.
Cheers,
--
Anthony PERARD
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [XEN PATCH 1/4] build: xen/include: use if_changed
2022-06-02 9:11 ` Jan Beulich
@ 2022-06-06 13:39 ` Anthony PERARD
2022-06-07 6:26 ` Jan Beulich
0 siblings, 1 reply; 23+ messages in thread
From: Anthony PERARD @ 2022-06-06 13:39 UTC (permalink / raw)
To: Jan Beulich
Cc: Andrew Cooper, George Dunlap, Julien Grall, Stefano Stabellini,
Wei Liu, xen-devel
On Thu, Jun 02, 2022 at 11:11:15AM +0200, Jan Beulich wrote:
> On 01.06.2022 18:59, Anthony PERARD wrote:
> > Use "define" for the headers*_chk commands as otherwise the "#"
> > is interpreted as a comment and make can't find the end of
> > $(foreach,).
>
> In cmd_xlat_lst you use $(pound) - any reason this doesn't work in
> these rules? Note that I don't mind the use of "define", just that I'm
> puzzled by the justification.
I think I just forgot about $(pound) when I tried to debug why the
command didn't work in some environment (that is when not using define).
I think the second thing that make me not replacing '#' by "$(pound)" is
that reading "#include" in the Makefile is probably better that
reading "$(pound)include".
I guess we should add something to the justification like:
That allow us to keep writing "#include" in the Makefile without
having to replace that by "$(pound)include" which would be a bit
less obvious about the command line purpose.
Thanks,
--
Anthony PERARD
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [XEN PATCH 2/4] build: set PERL
2022-06-02 9:01 ` Jan Beulich
@ 2022-06-06 13:47 ` Anthony PERARD
0 siblings, 0 replies; 23+ messages in thread
From: Anthony PERARD @ 2022-06-06 13:47 UTC (permalink / raw)
To: Jan Beulich
Cc: Andrew Cooper, George Dunlap, Julien Grall, Stefano Stabellini,
Wei Liu, xen-devel
On Thu, Jun 02, 2022 at 11:01:30AM +0200, Jan Beulich wrote:
> On 01.06.2022 18:59, Anthony PERARD wrote:
> > --- a/xen/Makefile
> > +++ b/xen/Makefile
> > @@ -22,6 +22,7 @@ PYTHON_INTERPRETER := $(word 1,$(shell which python3 python python2 2>/dev/null)
> > export PYTHON ?= $(PYTHON_INTERPRETER)
> >
> > export CHECKPOLICY ?= checkpolicy
> > +export PERL ?= perl
>
> For the intended use, is there a minimum version requirement? If so,
> it needs documenting in ./README (and it preferably wouldn't be any
> newer than from around the times our other dependencies are). And
> even when the uses are fully backwards compatible, I think the need
> for the tool wants mentioning there.
I don't think there's a minimum version. The script works in our
Gitlab CI, or at least the builds don't break.
Yes, it would be better to document the tool, I'll add it to the README.
(We already use it in the toolstack, at least for libxl, so it was at
least partially needed before.)
Thanks,
--
Anthony PERARD
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [XEN PATCH 3/4] build: replace get-fields.sh by a perl script
2022-06-01 17:32 ` Elliott Mitchell
@ 2022-06-06 13:51 ` Anthony PERARD
0 siblings, 0 replies; 23+ messages in thread
From: Anthony PERARD @ 2022-06-06 13:51 UTC (permalink / raw)
To: Elliott Mitchell
Cc: xen-devel, Andrew Cooper, George Dunlap, Jan Beulich,
Julien Grall, Stefano Stabellini, Wei Liu
On Wed, Jun 01, 2022 at 10:32:10AM -0700, Elliott Mitchell wrote:
> On Wed, Jun 01, 2022 at 05:59:08PM +0100, Anthony PERARD wrote:
> > diff --git a/xen/tools/compat-xlat-header b/xen/tools/compat-xlat-header
> > new file mode 100755
> > index 0000000000..f1f42a9dde
> > --- /dev/null
> > +++ b/xen/tools/compat-xlat-header
> > @@ -0,0 +1,539 @@
> > +#!/usr/bin/perl -w
> > +
> > +use strict;
> > +use warnings;
>
> I hope to take more of a look at this, but one thing I immediately
> notice: -w is redundant with "use warnings;". I strongly prefer
> "use warnings;", but others may have different preferences.
Sounds good, I might have copy the shebang and the "use*" from an other
script in our repo, without checking what the -w stand for.
Thanks,
--
Anthony PERARD
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [XEN PATCH 1/4] build: xen/include: use if_changed
2022-06-06 13:39 ` Anthony PERARD
@ 2022-06-07 6:26 ` Jan Beulich
0 siblings, 0 replies; 23+ messages in thread
From: Jan Beulich @ 2022-06-07 6:26 UTC (permalink / raw)
To: Anthony PERARD
Cc: Andrew Cooper, George Dunlap, Julien Grall, Stefano Stabellini,
Wei Liu, xen-devel
On 06.06.2022 15:39, Anthony PERARD wrote:
> On Thu, Jun 02, 2022 at 11:11:15AM +0200, Jan Beulich wrote:
>> On 01.06.2022 18:59, Anthony PERARD wrote:
>>> Use "define" for the headers*_chk commands as otherwise the "#"
>>> is interpreted as a comment and make can't find the end of
>>> $(foreach,).
>>
>> In cmd_xlat_lst you use $(pound) - any reason this doesn't work in
>> these rules? Note that I don't mind the use of "define", just that I'm
>> puzzled by the justification.
>
> I think I just forgot about $(pound) when I tried to debug why the
> command didn't work in some environment (that is when not using define).
>
> I think the second thing that make me not replacing '#' by "$(pound)" is
> that reading "#include" in the Makefile is probably better that
> reading "$(pound)include".
>
> I guess we should add something to the justification like:
> That allow us to keep writing "#include" in the Makefile without
> having to replace that by "$(pound)include" which would be a bit
> less obvious about the command line purpose.
I'd be okay with this, and I'd also be okay with adding this while
committing. With it added
Acked-by: Jan Beulich <jbeulich@suse.com>
Jan
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [XEN PATCH 1/4] build: xen/include: use if_changed
2022-06-01 16:59 ` [XEN PATCH 1/4] build: xen/include: use if_changed Anthony PERARD
2022-06-02 9:11 ` Jan Beulich
@ 2022-06-09 10:16 ` Bertrand Marquis
2022-06-09 10:26 ` Jan Beulich
1 sibling, 1 reply; 23+ messages in thread
From: Bertrand Marquis @ 2022-06-09 10:16 UTC (permalink / raw)
To: Anthony PERARD
Cc: xen-devel, Andrew Cooper, George Dunlap, Jan Beulich,
Julien Grall, Stefano Stabellini, Wei Liu
Hi Anthony,
> On 1 Jun 2022, at 17:59, Anthony PERARD <anthony.perard@citrix.com> wrote:
>
> Use "define" for the headers*_chk commands as otherwise the "#"
> is interpreted as a comment and make can't find the end of
> $(foreach,).
>
> Adding several .PRECIOUS as without them `make` deletes the
> intermediate targets. This is an issue because the macro $(if_changed,)
> check if the target exist in order to decide whether to recreate the
> target.
>
> Removing the call to `mkdir` from the commands. Those aren't needed
> anymore because a rune in Rules.mk creates the directory for each
> $(targets).
>
> Remove "export PYTHON" as it is already exported.
With this change, compiling for x86 is now ending up in:
CHK include/headers99.chk
make[9]: execvp: /bin/sh: Argument list too long
make[9]: *** [include/Makefile:181: include/headers++.chk] Error 127
Not quite sure yet why but I wanted to signal it early as other might be impacted.
Arm and arm64 builds are not impacted.
Cheers
Bertrand
>
> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
> ---
> xen/include/Makefile | 108 ++++++++++++++++++++++++++++++-------------
> 1 file changed, 76 insertions(+), 32 deletions(-)
>
> diff --git a/xen/include/Makefile b/xen/include/Makefile
> index 03baf10efb..6d9bcc19b0 100644
> --- a/xen/include/Makefile
> +++ b/xen/include/Makefile
> @@ -45,38 +45,65 @@ public-$(CONFIG_ARM) := $(wildcard $(srcdir)/public/arch-arm/*.h $(srcdir)/publi
> .PHONY: all
> all: $(addprefix $(obj)/,$(headers-y))
>
> -$(obj)/compat/%.h: $(obj)/compat/%.i $(srcdir)/Makefile $(srctree)/tools/compat-build-header.py
> - $(PYTHON) $(srctree)/tools/compat-build-header.py <$< $(patsubst $(obj)/%,%,$@) >>$@.new; \
> - mv -f $@.new $@
> -
> -$(obj)/compat/%.i: $(obj)/compat/%.c $(srcdir)/Makefile
> - $(CPP) $(filter-out -Wa$(comma)% -include %/include/xen/config.h,$(XEN_CFLAGS)) $(cppflags-y) -o $@ $<
> -
> -$(obj)/compat/%.c: $(src)/public/%.h $(srcdir)/xlat.lst $(srcdir)/Makefile $(srctree)/tools/compat-build-source.py
> - mkdir -p $(@D)
> - $(PYTHON) $(srctree)/tools/compat-build-source.py $(srcdir)/xlat.lst <$< >$@.new
> - mv -f $@.new $@
> -
> -$(obj)/compat/.xlat/%.h: $(obj)/compat/%.h $(obj)/compat/.xlat/%.lst $(srctree)/tools/get-fields.sh $(srcdir)/Makefile
> - export PYTHON=$(PYTHON); \
> - while read what name; do \
> - $(SHELL) $(srctree)/tools/get-fields.sh "$$what" compat_$$name $< || exit $$?; \
> - done <$(patsubst $(obj)/compat/%,$(obj)/compat/.xlat/%,$(basename $<)).lst >$@.new
> - mv -f $@.new $@
> +quiet_cmd_compat_h = GEN $@
> +cmd_compat_h = \
> + $(PYTHON) $(srctree)/tools/compat-build-header.py <$< $(patsubst $(obj)/%,%,$@) >>$@.new; \
> + mv -f $@.new $@
> +
> +quiet_cmd_compat_i = CPP $@
> +cmd_compat_i = $(CPP) $(filter-out -Wa$(comma)% -include %/include/xen/config.h,$(XEN_CFLAGS)) $(cppflags-y) -o $@ $<
> +
> +quiet_cmd_compat_c = GEN $@
> +cmd_compat_c = \
> + $(PYTHON) $(srctree)/tools/compat-build-source.py $(srcdir)/xlat.lst <$< >$@.new; \
> + mv -f $@.new $@
> +
> +quiet_cmd_xlat_headers = GEN $@
> +cmd_xlat_headers = \
> + while read what name; do \
> + $(SHELL) $(srctree)/tools/get-fields.sh "$$what" compat_$$name $< || exit $$?; \
> + done <$(patsubst $(obj)/compat/%,$(obj)/compat/.xlat/%,$(basename $<)).lst >$@.new; \
> + mv -f $@.new $@
> +
> +targets += $(headers-y)
> +$(obj)/compat/%.h: $(obj)/compat/%.i $(srctree)/tools/compat-build-header.py FORCE
> + $(call if_changed,compat_h)
> +
> +.PRECIOUS: $(obj)/compat/%.i
> +targets += $(patsubst %.h, %.i, $(headers-y))
> +$(obj)/compat/%.i: $(obj)/compat/%.c FORCE
> + $(call if_changed,compat_i)
> +
> +.PRECIOUS: $(obj)/compat/%.c
> +targets += $(patsubst %.h, %.c, $(headers-y))
> +$(obj)/compat/%.c: $(src)/public/%.h $(srcdir)/xlat.lst $(srctree)/tools/compat-build-source.py FORCE
> + $(call if_changed,compat_c)
> +
> +targets += $(patsubst compat/%, compat/.xlat/%, $(headers-y))
> +$(obj)/compat/.xlat/%.h: $(obj)/compat/%.h $(obj)/compat/.xlat/%.lst $(srctree)/tools/get-fields.sh FORCE
> + $(call if_changed,xlat_headers)
> +
> +quiet_cmd_xlat_lst = GEN $@
> +cmd_xlat_lst = \
> + grep -v '^[[:blank:]]*$(pound)' $< | sed -ne 's,@arch@,$(compat-arch-y),g' -re 's,[[:blank:]]+$*\.h[[:blank:]]*$$,,p' >$@.new; \
> + $(call move-if-changed,$@.new,$@)
>
> .PRECIOUS: $(obj)/compat/.xlat/%.lst
> -$(obj)/compat/.xlat/%.lst: $(srcdir)/xlat.lst $(srcdir)/Makefile
> - mkdir -p $(@D)
> - grep -v '^[[:blank:]]*#' $< | sed -ne 's,@arch@,$(compat-arch-y),g' -re 's,[[:blank:]]+$*\.h[[:blank:]]*$$,,p' >$@.new
> - $(call move-if-changed,$@.new,$@)
> +targets += $(patsubst compat/%.h, compat/.xlat/%.lst, $(headers-y))
> +$(obj)/compat/.xlat/%.lst: $(srcdir)/xlat.lst FORCE
> + $(call if_changed,xlat_lst)
>
> xlat-y := $(shell sed -ne 's,@arch@,$(compat-arch-y),g' -re 's,^[?!][[:blank:]]+[^[:blank:]]+[[:blank:]]+,,p' $(srcdir)/xlat.lst | uniq)
> xlat-y := $(filter $(patsubst compat/%,%,$(headers-y)),$(xlat-y))
>
> -$(obj)/compat/xlat.h: $(addprefix $(obj)/compat/.xlat/,$(xlat-y)) $(obj)/config/auto.conf $(srcdir)/Makefile
> - cat $(filter %.h,$^) >$@.new
> +quiet_cmd_xlat_h = GEN $@
> +cmd_xlat_h = \
> + cat $(filter %.h,$^) >$@.new; \
> mv -f $@.new $@
>
> +$(obj)/compat/xlat.h: $(addprefix $(obj)/compat/.xlat/,$(xlat-y)) $(obj)/config/auto.conf FORCE
> + $(call if_changed,xlat_h)
> +
> ifeq ($(XEN_TARGET_ARCH),$(XEN_COMPILE_ARCH))
>
> all: $(obj)/headers.chk $(obj)/headers99.chk $(obj)/headers++.chk
> @@ -102,27 +129,31 @@ PUBLIC_C99_HEADERS := $(call public-filter-headers,public-c99-headers)
> $(src)/public/io/9pfs.h-prereq := string
> $(src)/public/io/pvcalls.h-prereq := string
>
> -$(obj)/headers.chk: $(PUBLIC_ANSI_HEADERS) $(srcdir)/Makefile
> +quiet_cmd_header_chk = CHK $@
> +cmd_header_chk = \
> for i in $(filter %.h,$^); do \
> $(CC) -x c -ansi -Wall -Werror -include stdint.h \
> -S -o /dev/null $$i || exit 1; \
> echo $$i; \
> - done >$@.new
> + done >$@.new; \
> mv $@.new $@
>
> -$(obj)/headers99.chk: $(PUBLIC_C99_HEADERS) $(srcdir)/Makefile
> - rm -f $@.new
> +quiet_cmd_headers99_chk = CHK $@
> +define cmd_headers99_chk
> + rm -f $@.new; \
> $(foreach i, $(filter %.h,$^), \
> echo "#include "\"$(i)\" \
> | $(CC) -x c -std=c99 -Wall -Werror \
> -include stdint.h \
> $(foreach j, $($(patsubst $(srctree)/%,%,$i)-prereq), -include $(j).h) \
> -S -o /dev/null - \
> - || exit $$?; echo $(i) >> $@.new;)
> + || exit $$?; echo $(i) >> $@.new;) \
> mv $@.new $@
> +endef
>
> -$(obj)/headers++.chk: $(PUBLIC_HEADERS) $(srcdir)/Makefile
> - rm -f $@.new
> +quiet_cmd_headerscxx_chk = CHK $@
> +define cmd_headerscxx_chk
> + rm -f $@.new; \
> if ! $(CXX) -v >/dev/null 2>&1; then \
> touch $@.new; \
> exit 0; \
> @@ -133,8 +164,21 @@ $(obj)/headers++.chk: $(PUBLIC_HEADERS) $(srcdir)/Makefile
> -include stdint.h -include $(srcdir)/public/xen.h \
> $(foreach j, $($(patsubst $(srctree)/%,%,$i)-prereq), -include c$(j)) \
> -S -o /dev/null - \
> - || exit $$?; echo $(i) >> $@.new;)
> + || exit $$?; echo $(i) >> $@.new;) \
> mv $@.new $@
> +endef
> +
> +targets += headers.chk
> +$(obj)/headers.chk: $(PUBLIC_ANSI_HEADERS) FORCE
> + $(call if_changed,header_chk)
> +
> +targets += headers99.chk
> +$(obj)/headers99.chk: $(PUBLIC_C99_HEADERS) FORCE
> + $(call if_changed,headers99_chk)
> +
> +targets += headers++.chk
> +$(obj)/headers++.chk: $(PUBLIC_HEADERS) FORCE
> + $(call if_changed,headerscxx_chk)
>
> endif
>
> --
> Anthony PERARD
>
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [XEN PATCH 1/4] build: xen/include: use if_changed
2022-06-09 10:16 ` Bertrand Marquis
@ 2022-06-09 10:26 ` Jan Beulich
2022-06-09 11:51 ` Bertrand Marquis
0 siblings, 1 reply; 23+ messages in thread
From: Jan Beulich @ 2022-06-09 10:26 UTC (permalink / raw)
To: Bertrand Marquis
Cc: xen-devel, Andrew Cooper, George Dunlap, Julien Grall,
Stefano Stabellini, Wei Liu, Anthony PERARD
On 09.06.2022 12:16, Bertrand Marquis wrote:
>> On 1 Jun 2022, at 17:59, Anthony PERARD <anthony.perard@citrix.com> wrote:
>>
>> Use "define" for the headers*_chk commands as otherwise the "#"
>> is interpreted as a comment and make can't find the end of
>> $(foreach,).
>>
>> Adding several .PRECIOUS as without them `make` deletes the
>> intermediate targets. This is an issue because the macro $(if_changed,)
>> check if the target exist in order to decide whether to recreate the
>> target.
>>
>> Removing the call to `mkdir` from the commands. Those aren't needed
>> anymore because a rune in Rules.mk creates the directory for each
>> $(targets).
>>
>> Remove "export PYTHON" as it is already exported.
>
> With this change, compiling for x86 is now ending up in:
> CHK include/headers99.chk
> make[9]: execvp: /bin/sh: Argument list too long
> make[9]: *** [include/Makefile:181: include/headers++.chk] Error 127
>
> Not quite sure yet why but I wanted to signal it early as other might be impacted.
>
> Arm and arm64 builds are not impacted.
Hmm, that patch has passed the smoke push gate already, so there likely is
more to it than there being an unconditional issue. I did build-test this
before pushing, and I've just re-tested on a 2nd system without seeing an
issue.
Also please remember to trim your replies.
Jan
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [XEN PATCH 1/4] build: xen/include: use if_changed
2022-06-09 10:26 ` Jan Beulich
@ 2022-06-09 11:51 ` Bertrand Marquis
2022-06-09 12:16 ` Jan Beulich
2022-06-09 12:55 ` Anthony PERARD
0 siblings, 2 replies; 23+ messages in thread
From: Bertrand Marquis @ 2022-06-09 11:51 UTC (permalink / raw)
To: Jan Beulich
Cc: xen-devel, Andrew Cooper, George Dunlap, Julien Grall,
Stefano Stabellini, Wei Liu, Anthony PERARD
Hi,
> On 9 Jun 2022, at 11:26, Jan Beulich <jbeulich@suse.com> wrote:
>
> On 09.06.2022 12:16, Bertrand Marquis wrote:
>>> On 1 Jun 2022, at 17:59, Anthony PERARD <anthony.perard@citrix.com> wrote:
>>>
>>> Use "define" for the headers*_chk commands as otherwise the "#"
>>> is interpreted as a comment and make can't find the end of
>>> $(foreach,).
>>>
>>> Adding several .PRECIOUS as without them `make` deletes the
>>> intermediate targets. This is an issue because the macro $(if_changed,)
>>> check if the target exist in order to decide whether to recreate the
>>> target.
>>>
>>> Removing the call to `mkdir` from the commands. Those aren't needed
>>> anymore because a rune in Rules.mk creates the directory for each
>>> $(targets).
>>>
>>> Remove "export PYTHON" as it is already exported.
>>
>> With this change, compiling for x86 is now ending up in:
>> CHK include/headers99.chk
>> make[9]: execvp: /bin/sh: Argument list too long
>> make[9]: *** [include/Makefile:181: include/headers++.chk] Error 127
>>
>> Not quite sure yet why but I wanted to signal it early as other might be impacted.
>>
>> Arm and arm64 builds are not impacted.
>
> Hmm, that patch has passed the smoke push gate already, so there likely is
> more to it than there being an unconditional issue. I did build-test this
> before pushing, and I've just re-tested on a 2nd system without seeing an
> issue.
I have the problem only when building using Yocto, I did a normal build and the
issue is not coming.
Doing a verbose compilation I have this (sorry for the long lines):
for i in include/public/vcpu.h include/public/errno.h include/public/kexec.h include/public/argo.h include/public/xen.h include/public/nmi.h include/public/xencomm.h include/public/xenoprof.h include/public/device_tree_defs.h include/public/version.h include/public/memory.h include/public/features.h include/public/sched.h include/public/xen-compat.h include/public/callback.h include/public/vm_event.h include/public/grant_table.h include/public/physdev.h include/public/tmem.h include/public/hypfs.h include/public/platform.h include/public/pmu.h include/public/elfnote.h include/public/trace.h include/public/event_channel.h include/public/io/vscsiif.h include/public/io/kbdif.h include/public/io/protocols.h include/public/io/ring.h include/public/io/displif.h include/public/io/fsif.h include/public/io/blkif.h include/public/io/console.h include/public/io/sndif.h include/public/io/fbif.h include/public/io/libxenvchan.h include/public/io/netif.h include/public/io/usbif.h include/public/io/pciif.h include/public/io/tpmif.h include/public/io/xs_wire.h include/public/io/xenbus.h include/public/io/cameraif.h include/public/hvm/pvdrivers.h include/public/hvm/e820.h include/public/hvm/hvm_xs_strings.h include/public/hvm/dm_op.h include/public/hvm/ioreq.h include/public/hvm/hvm_info_table.h include/public/hvm/hvm_vcpu.h include/public/hvm/hvm_op.h include/public/hvm/params.h; do x86_64-poky-linux-gcc --sysroot=/home/bermar01/Development/xen-dev/build/profile-qemu-x86_64.prj/tmp/work/core2-64-poky-linux/xen/4.17+git1-r0/recipe-sysroot -x c -ansi -Wall -Werror -include stdint.h -S -o /dev/null $i || exit 1; echo $i; done >include/headers.chk.new; mv include/headers.chk.new include/headers.chk
| rm -f include/headers99.chk.new; echo "#include "\"include/public/io/9pfs.h\" | x86_64-poky-linux-gcc --sysroot=/home/bermar01/Development/xen-dev/build/profile-qemu-x86_64.prj/tmp/work/core2-64-poky-linux/xen/4.17+git1-r0/recipe-sysroot -x c -std=c99 -Wall -Werror -include stdint.h -include string.h -S -o /dev/null - || exit $?; echo include/public/io/9pfs.h >> include/headers99.chk.new; echo "#include "\"include/public/io/pvcalls.h\" | x86_64-poky-linux-gcc --sysroot=/home/bermar01/Development/xen-dev/build/profile-qemu-x86_64.prj/tmp/work/core2-64-poky-linux/xen/4.17+git1-r0/recipe-sysroot -x c -std=c99 -Wall -Werror -include stdint.h -include string.h -S -o /dev/null - || exit $?; echo include/public/io/pvcalls.h >> include/headers99.chk.new; mv include/headers99.chk.new include/headers99.chk
| make[9]: execvp: /bin/sh: Argument list too long
| make[9]: *** [include/Makefile:181: include/headers++.chk] Error 127
| make[9]: *** Waiting for unfinished jobs....
So the command passed to the sub shell by make is quite long.
No idea why this comes out only when building in Yocto but I will dig a bit.
>
> Also please remember to trim your replies.
>
Will do.
Bertrand
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [XEN PATCH 1/4] build: xen/include: use if_changed
2022-06-09 11:51 ` Bertrand Marquis
@ 2022-06-09 12:16 ` Jan Beulich
2022-06-09 12:48 ` Bertrand Marquis
2022-06-09 12:55 ` Anthony PERARD
1 sibling, 1 reply; 23+ messages in thread
From: Jan Beulich @ 2022-06-09 12:16 UTC (permalink / raw)
To: Bertrand Marquis
Cc: xen-devel, Andrew Cooper, George Dunlap, Julien Grall,
Stefano Stabellini, Wei Liu, Anthony PERARD
On 09.06.2022 13:51, Bertrand Marquis wrote:
>> On 9 Jun 2022, at 11:26, Jan Beulich <jbeulich@suse.com> wrote:
>> On 09.06.2022 12:16, Bertrand Marquis wrote:
>>>> On 1 Jun 2022, at 17:59, Anthony PERARD <anthony.perard@citrix.com> wrote:
>>>>
>>>> Use "define" for the headers*_chk commands as otherwise the "#"
>>>> is interpreted as a comment and make can't find the end of
>>>> $(foreach,).
>>>>
>>>> Adding several .PRECIOUS as without them `make` deletes the
>>>> intermediate targets. This is an issue because the macro $(if_changed,)
>>>> check if the target exist in order to decide whether to recreate the
>>>> target.
>>>>
>>>> Removing the call to `mkdir` from the commands. Those aren't needed
>>>> anymore because a rune in Rules.mk creates the directory for each
>>>> $(targets).
>>>>
>>>> Remove "export PYTHON" as it is already exported.
>>>
>>> With this change, compiling for x86 is now ending up in:
>>> CHK include/headers99.chk
>>> make[9]: execvp: /bin/sh: Argument list too long
>>> make[9]: *** [include/Makefile:181: include/headers++.chk] Error 127
>>>
>>> Not quite sure yet why but I wanted to signal it early as other might be impacted.
>>>
>>> Arm and arm64 builds are not impacted.
>>
>> Hmm, that patch has passed the smoke push gate already, so there likely is
>> more to it than there being an unconditional issue. I did build-test this
>> before pushing, and I've just re-tested on a 2nd system without seeing an
>> issue.
>
> I have the problem only when building using Yocto, I did a normal build and the
> issue is not coming.
>
> Doing a verbose compilation I have this (sorry for the long lines):
>
> for i in include/public/vcpu.h include/public/errno.h include/public/kexec.h include/public/argo.h include/public/xen.h include/public/nmi.h include/public/xencomm.h include/public/xenoprof.h include/public/device_tree_defs.h include/public/version.h include/public/memory.h include/public/features.h include/public/sched.h include/public/xen-compat.h include/public/callback.h include/public/vm_event.h include/public/grant_table.h include/public/physdev.h include/public/tmem.h include/public/hypfs.h include/public/platform.h include/public/pmu.h include/public/elfnote.h include/public/trace.h include/public/event_channel.h include/public/io/vscsiif.h include/public/io/kbdif.h include/public/io/protocols.h include/public/io/ring.h include/public/io/displif.h include/public/io/fsif.h include/public/io/blkif.h include/public/io/console.h include/public/io/sndif.h include/public/io/fbif.h include/public/io/libxenvchan.h include/public/io/netif.h include/public/io/usbif.h include/public/io/pciif.h include/public/io/tpmif.h include/public/io/xs_wire.h include/public/io/xenbus.h include/public/io/cameraif.h include/public/hvm/pvdrivers.h include/public/hvm/e820.h include/public/hvm/hvm_xs_strings.h include/public/hvm/dm_op.h include/public/hvm/ioreq.h include/public/hvm/hvm_info_table.h include/public/hvm/hvm_vcpu.h include/public/hvm/hvm_op.h include/public/hvm/params.h; do x86_64-poky-linux-gcc --sysroot=/home/bermar01/Development/xen-dev/build/profile-qemu-x86_64.prj/tmp/work/core2-64-poky-linux/xen/4.17+git1-r0/recipe-sysroot -x c -ansi -Wall -Werror -include stdint.h -S -o /dev/null $i || exit 1; echo $i; done >include/headers.chk.new; mv include/headers.chk.new include/headers.chk
> | rm -f include/headers99.chk.new; echo "#include "\"include/public/io/9pfs.h\" | x86_64-poky-linux-gcc --sysroot=/home/bermar01/Development/xen-dev/build/profile-qemu-x86_64.prj/tmp/work/core2-64-poky-linux/xen/4.17+git1-r0/recipe-sysroot -x c -std=c99 -Wall -Werror -include stdint.h -include string.h -S -o /dev/null - || exit $?; echo include/public/io/9pfs.h >> include/headers99.chk.new; echo "#include "\"include/public/io/pvcalls.h\" | x86_64-poky-linux-gcc --sysroot=/home/bermar01/Development/xen-dev/build/profile-qemu-x86_64.prj/tmp/work/core2-64-poky-linux/xen/4.17+git1-r0/recipe-sysroot -x c -std=c99 -Wall -Werror -include stdint.h -include string.h -S -o /dev/null - || exit $?; echo include/public/io/pvcalls.h >> include/headers99.chk.new; mv include/headers99.chk.new include/headers99.chk
> | make[9]: execvp: /bin/sh: Argument list too long
> | make[9]: *** [include/Makefile:181: include/headers++.chk] Error 127
> | make[9]: *** Waiting for unfinished jobs....
>
> So the command passed to the sub shell by make is quite long.
>
> No idea why this comes out only when building in Yocto but I will dig a bit.
Maybe Yocto has an unusually low limit on command arguments' total size?
The whole thing is just over 2500 chars, which doesn't look to be unusually
long for Unix-like environments.
Jan
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [XEN PATCH 1/4] build: xen/include: use if_changed
2022-06-09 12:16 ` Jan Beulich
@ 2022-06-09 12:48 ` Bertrand Marquis
0 siblings, 0 replies; 23+ messages in thread
From: Bertrand Marquis @ 2022-06-09 12:48 UTC (permalink / raw)
To: Jan Beulich
Cc: xen-devel, Andrew Cooper, George Dunlap, Julien Grall,
Stefano Stabellini, Wei Liu, Anthony PERARD
Hi,
> On 9 Jun 2022, at 13:16, Jan Beulich <jbeulich@suse.com> wrote:
>
> On 09.06.2022 13:51, Bertrand Marquis wrote:
>>> On 9 Jun 2022, at 11:26, Jan Beulich <jbeulich@suse.com> wrote:
>>> On 09.06.2022 12:16, Bertrand Marquis wrote:
>>>>> On 1 Jun 2022, at 17:59, Anthony PERARD <anthony.perard@citrix.com> wrote:
>>>>>
>>>>> Use "define" for the headers*_chk commands as otherwise the "#"
>>>>> is interpreted as a comment and make can't find the end of
>>>>> $(foreach,).
>>>>>
>>>>> Adding several .PRECIOUS as without them `make` deletes the
>>>>> intermediate targets. This is an issue because the macro $(if_changed,)
>>>>> check if the target exist in order to decide whether to recreate the
>>>>> target.
>>>>>
>>>>> Removing the call to `mkdir` from the commands. Those aren't needed
>>>>> anymore because a rune in Rules.mk creates the directory for each
>>>>> $(targets).
>>>>>
>>>>> Remove "export PYTHON" as it is already exported.
>>>>
>>>> With this change, compiling for x86 is now ending up in:
>>>> CHK include/headers99.chk
>>>> make[9]: execvp: /bin/sh: Argument list too long
>>>> make[9]: *** [include/Makefile:181: include/headers++.chk] Error 127
>>>>
>>>> Not quite sure yet why but I wanted to signal it early as other might be impacted.
>>>>
>>>> Arm and arm64 builds are not impacted.
>>>
>>> Hmm, that patch has passed the smoke push gate already, so there likely is
>>> more to it than there being an unconditional issue. I did build-test this
>>> before pushing, and I've just re-tested on a 2nd system without seeing an
>>> issue.
>>
>> I have the problem only when building using Yocto, I did a normal build and the
>> issue is not coming.
>>
>> Doing a verbose compilation I have this (sorry for the long lines):
>>
>> for i in include/public/vcpu.h include/public/errno.h include/public/kexec.h include/public/argo.h include/public/xen.h include/public/nmi.h include/public/xencomm.h include/public/xenoprof.h include/public/device_tree_defs.h include/public/version.h include/public/memory.h include/public/features.h include/public/sched.h include/public/xen-compat.h include/public/callback.h include/public/vm_event.h include/public/grant_table.h include/public/physdev.h include/public/tmem.h include/public/hypfs.h include/public/platform.h include/public/pmu.h include/public/elfnote.h include/public/trace.h include/public/event_channel.h include/public/io/vscsiif.h include/public/io/kbdif.h include/public/io/protocols.h include/public/io/ring.h include/public/io/displif.h include/public/io/fsif.h include/public/io/blkif.h include/public/io/console.h include/public/io/sndif.h include/public/io/fbif.h include/public/io/libxenvchan.h include/public/io/netif.h include/public/io/usbif.h include/public/io/pciif.h include/public/io/tpmif.h include/public/io/xs_wire.h include/public/io/xenbus.h include/public/io/cameraif.h include/public/hvm/pvdrivers.h include/public/hvm/e820.h include/public/hvm/hvm_xs_strings.h include/public/hvm/dm_op.h include/public/hvm/ioreq.h include/public/hvm/hvm_info_table.h include/public/hvm/hvm_vcpu.h include/public/hvm/hvm_op.h include/public/hvm/params.h; do x86_64-poky-linux-gcc --sysroot=/home/bermar01/Development/xen-dev/build/profile-qemu-x86_64.prj/tmp/work/core2-64-poky-linux/xen/4.17+git1-r0/recipe-sysroot -x c -ansi -Wall -Werror -include stdint.h -S -o /dev/null $i || exit 1; echo $i; done >include/headers.chk.new; mv include/headers.chk.new include/headers.chk
>> | rm -f include/headers99.chk.new; echo "#include "\"include/public/io/9pfs.h\" | x86_64-poky-linux-gcc --sysroot=/home/bermar01/Development/xen-dev/build/profile-qemu-x86_64.prj/tmp/work/core2-64-poky-linux/xen/4.17+git1-r0/recipe-sysroot -x c -std=c99 -Wall -Werror -include stdint.h -include string.h -S -o /dev/null - || exit $?; echo include/public/io/9pfs.h >> include/headers99.chk.new; echo "#include "\"include/public/io/pvcalls.h\" | x86_64-poky-linux-gcc --sysroot=/home/bermar01/Development/xen-dev/build/profile-qemu-x86_64.prj/tmp/work/core2-64-poky-linux/xen/4.17+git1-r0/recipe-sysroot -x c -std=c99 -Wall -Werror -include stdint.h -include string.h -S -o /dev/null - || exit $?; echo include/public/io/pvcalls.h >> include/headers99.chk.new; mv include/headers99.chk.new include/headers99.chk
>> | make[9]: execvp: /bin/sh: Argument list too long
>> | make[9]: *** [include/Makefile:181: include/headers++.chk] Error 127
>> | make[9]: *** Waiting for unfinished jobs....
>>
>> So the command passed to the sub shell by make is quite long.
>>
>> No idea why this comes out only when building in Yocto but I will dig a bit.
>
> Maybe Yocto has an unusually low limit on command arguments' total size?
> The whole thing is just over 2500 chars, which doesn't look to be unusually
> long for Unix-like environments.
>
Actually the command to generate headers++.chk is 15294 characters when I build xen normally.
In Yocto it becomes a lot bigger as CC includes a sysroot parameter with a path.
Bertrand
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [XEN PATCH 1/4] build: xen/include: use if_changed
2022-06-09 11:51 ` Bertrand Marquis
2022-06-09 12:16 ` Jan Beulich
@ 2022-06-09 12:55 ` Anthony PERARD
2022-06-09 14:26 ` Bertrand Marquis
2022-06-13 13:58 ` Bertrand Marquis
1 sibling, 2 replies; 23+ messages in thread
From: Anthony PERARD @ 2022-06-09 12:55 UTC (permalink / raw)
To: Bertrand Marquis
Cc: Jan Beulich, xen-devel, Andrew Cooper, George Dunlap,
Julien Grall, Stefano Stabellini, Wei Liu
On Thu, Jun 09, 2022 at 11:51:20AM +0000, Bertrand Marquis wrote:
> Hi,
>
> > On 9 Jun 2022, at 11:26, Jan Beulich <jbeulich@suse.com> wrote:
> >
> > On 09.06.2022 12:16, Bertrand Marquis wrote:
> >> With this change, compiling for x86 is now ending up in:
> >> CHK include/headers99.chk
> >> make[9]: execvp: /bin/sh: Argument list too long
> >> make[9]: *** [include/Makefile:181: include/headers++.chk] Error 127
> >>
> >> Not quite sure yet why but I wanted to signal it early as other might be impacted.
> >>
> >> Arm and arm64 builds are not impacted.
> >
> > Hmm, that patch has passed the smoke push gate already, so there likely is
> > more to it than there being an unconditional issue. I did build-test this
> > before pushing, and I've just re-tested on a 2nd system without seeing an
> > issue.
>
> I have the problem only when building using Yocto, I did a normal build and the
> issue is not coming.
>
Will the following patch help?
From 0f32f749304b233c0d5574dc6b14f66e8709feba Mon Sep 17 00:00:00 2001
From: Anthony PERARD <anthony.perard@citrix.com>
Date: Thu, 9 Jun 2022 13:42:52 +0100
Subject: [XEN PATCH] build,include: rework shell script for headers++.chk
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 to 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 "/".
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>
---
xen/include/Makefile | 17 +++++++++++++----
1 file changed, 13 insertions(+), 4 deletions(-)
diff --git a/xen/include/Makefile b/xen/include/Makefile
index 6d9bcc19b0..ca5e868f38 100644
--- a/xen/include/Makefile
+++ b/xen/include/Makefile
@@ -158,13 +158,22 @@ 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), \
+ $(patsubst $(srctree)/%,%,$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
--
Anthony PERARD
^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [XEN PATCH 1/4] build: xen/include: use if_changed
2022-06-09 12:55 ` Anthony PERARD
@ 2022-06-09 14:26 ` Bertrand Marquis
2022-06-13 13:58 ` Bertrand Marquis
1 sibling, 0 replies; 23+ messages in thread
From: Bertrand Marquis @ 2022-06-09 14:26 UTC (permalink / raw)
To: Anthony PERARD
Cc: Jan Beulich, xen-devel, Andrew Cooper, George Dunlap,
Julien Grall, Stefano Stabellini, Wei Liu
Hi Antony,
> On 9 Jun 2022, at 13:55, Anthony PERARD <anthony.perard@citrix.com> wrote:
>
> On Thu, Jun 09, 2022 at 11:51:20AM +0000, Bertrand Marquis wrote:
>> Hi,
>>
>>> On 9 Jun 2022, at 11:26, Jan Beulich <jbeulich@suse.com> wrote:
>>>
>>> On 09.06.2022 12:16, Bertrand Marquis wrote:
>>>> With this change, compiling for x86 is now ending up in:
>>>> CHK include/headers99.chk
>>>> make[9]: execvp: /bin/sh: Argument list too long
>>>> make[9]: *** [include/Makefile:181: include/headers++.chk] Error 127
>>>>
>>>> Not quite sure yet why but I wanted to signal it early as other might be impacted.
>>>>
>>>> Arm and arm64 builds are not impacted.
>>>
>>> Hmm, that patch has passed the smoke push gate already, so there likely is
>>> more to it than there being an unconditional issue. I did build-test this
>>> before pushing, and I've just re-tested on a 2nd system without seeing an
>>> issue.
>>
>> I have the problem only when building using Yocto, I did a normal build and the
>> issue is not coming.
>>
>
> Will the following patch help?
Yes it does, thanks a lot.
You can add my:
Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com>
Cheers
Bertrand
>
>
> From 0f32f749304b233c0d5574dc6b14f66e8709feba Mon Sep 17 00:00:00 2001
> From: Anthony PERARD <anthony.perard@citrix.com>
> Date: Thu, 9 Jun 2022 13:42:52 +0100
> Subject: [XEN PATCH] build,include: rework shell script for headers++.chk
>
> 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 to 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 "/".
>
> 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>
> ---
> xen/include/Makefile | 17 +++++++++++++----
> 1 file changed, 13 insertions(+), 4 deletions(-)
>
> diff --git a/xen/include/Makefile b/xen/include/Makefile
> index 6d9bcc19b0..ca5e868f38 100644
> --- a/xen/include/Makefile
> +++ b/xen/include/Makefile
> @@ -158,13 +158,22 @@ 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), \
> + $(patsubst $(srctree)/%,%,$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
>
>
>
>
> --
> Anthony PERARD
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [XEN PATCH 1/4] build: xen/include: use if_changed
2022-06-09 12:55 ` Anthony PERARD
2022-06-09 14:26 ` Bertrand Marquis
@ 2022-06-13 13:58 ` Bertrand Marquis
1 sibling, 0 replies; 23+ messages in thread
From: Bertrand Marquis @ 2022-06-13 13:58 UTC (permalink / raw)
To: Anthony PERARD
Cc: Jan Beulich, xen-devel, Andrew Cooper, George Dunlap,
Julien Grall, Stefano Stabellini, Wei Liu
Hi Anthony,
> On 9 Jun 2022, at 13:55, Anthony PERARD <anthony.perard@citrix.com> wrote:
>
> On Thu, Jun 09, 2022 at 11:51:20AM +0000, Bertrand Marquis wrote:
>> Hi,
>>
>>> On 9 Jun 2022, at 11:26, Jan Beulich <jbeulich@suse.com> wrote:
>>>
>>> On 09.06.2022 12:16, Bertrand Marquis wrote:
>>>> With this change, compiling for x86 is now ending up in:
>>>> CHK include/headers99.chk
>>>> make[9]: execvp: /bin/sh: Argument list too long
>>>> make[9]: *** [include/Makefile:181: include/headers++.chk] Error 127
>>>>
>>>> Not quite sure yet why but I wanted to signal it early as other might be impacted.
>>>>
>>>> Arm and arm64 builds are not impacted.
>>>
>>> Hmm, that patch has passed the smoke push gate already, so there likely is
>>> more to it than there being an unconditional issue. I did build-test this
>>> before pushing, and I've just re-tested on a 2nd system without seeing an
>>> issue.
>>
>> I have the problem only when building using Yocto, I did a normal build and the
>> issue is not coming.
>>
>
> Will the following patch help?
>
>
> From 0f32f749304b233c0d5574dc6b14f66e8709feba Mon Sep 17 00:00:00 2001
> From: Anthony PERARD <anthony.perard@citrix.com>
> Date: Thu, 9 Jun 2022 13:42:52 +0100
> Subject: [XEN PATCH] build,include: rework shell script for headers++.chk
>
> 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 to 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 "/".
>
> 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>
> ---
> xen/include/Makefile | 17 +++++++++++++----
> 1 file changed, 13 insertions(+), 4 deletions(-)
>
> diff --git a/xen/include/Makefile b/xen/include/Makefile
> index 6d9bcc19b0..ca5e868f38 100644
> --- a/xen/include/Makefile
> +++ b/xen/include/Makefile
> @@ -158,13 +158,22 @@ 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), \
> + $(patsubst $(srctree)/%,%,$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
>
Just a small reminder that you need to push this patch officially :-)
Cheers
Bertrand
^ permalink raw reply [flat|nested] 23+ messages in thread
end of thread, other threads:[~2022-06-13 13:58 UTC | newest]
Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-06-01 16:59 [XEN PATCH 0/4] xen: rework compat headers generation Anthony PERARD
2022-06-01 16:59 ` [XEN PATCH 1/4] build: xen/include: use if_changed Anthony PERARD
2022-06-02 9:11 ` Jan Beulich
2022-06-06 13:39 ` Anthony PERARD
2022-06-07 6:26 ` Jan Beulich
2022-06-09 10:16 ` Bertrand Marquis
2022-06-09 10:26 ` Jan Beulich
2022-06-09 11:51 ` Bertrand Marquis
2022-06-09 12:16 ` Jan Beulich
2022-06-09 12:48 ` Bertrand Marquis
2022-06-09 12:55 ` Anthony PERARD
2022-06-09 14:26 ` Bertrand Marquis
2022-06-13 13:58 ` Bertrand Marquis
2022-06-01 16:59 ` [XEN PATCH 2/4] build: set PERL Anthony PERARD
2022-06-02 9:01 ` Jan Beulich
2022-06-06 13:47 ` Anthony PERARD
2022-06-01 16:59 ` [XEN PATCH 3/4] build: replace get-fields.sh by a perl script Anthony PERARD
2022-06-01 17:32 ` Elliott Mitchell
2022-06-06 13:51 ` Anthony PERARD
2022-06-01 16:59 ` [XEN PATCH 4/4] build: remove auto.conf prerequisite from compat/xlat.h target Anthony PERARD
2022-06-02 9:16 ` Jan Beulich
2022-06-01 17:17 ` [XEN PATCH 0/4] xen: rework compat headers generation Andrew Cooper
2022-06-06 13:24 ` Anthony PERARD
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.