diff mbox series

[XEN,v4,18/18] build,include: rework compat-build-header.py

Message ID 20200331103102.1105674-19-anthony.perard@citrix.com (mailing list archive)
State Superseded
Headers show
Series xen: Build system improvements | expand

Commit Message

Anthony PERARD March 31, 2020, 10:31 a.m. UTC
Replace a mix of shell script and python script by all python script.

Remove the unnecessary "grep -v '^# [0-9]'". It is to hide the
linemarkers generated by the preprocessor. But adding -P inhibit there
generation, thus the grep isn't needed anymore.

gcc -E -P and clang -E -P have different behavior. While both don't
generates linemarkers, gcc also removes all empty lines while clang
keep them all. We don't need those empty lines, so we don't generates
them in the final compat/%.h headers. (This replace `uniq` which was
only de-duplicating empty line.)

The only changes in the final generated headers it that they don't
have empty lines anymore.

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---

Notes:
    v4:
    - new patch

 xen/include/Makefile             | 13 ++----------
 xen/tools/compat-build-header.py | 36 ++++++++++++++++++++++++++++++--
 2 files changed, 36 insertions(+), 13 deletions(-)

Comments

Jan Beulich April 8, 2020, 1:56 p.m. UTC | #1
On 31.03.2020 12:31, Anthony PERARD wrote:
> Replace a mix of shell script and python script by all python script.
> 
> Remove the unnecessary "grep -v '^# [0-9]'". It is to hide the
> linemarkers generated by the preprocessor. But adding -P inhibit there
> generation, thus the grep isn't needed anymore.
> 
> gcc -E -P and clang -E -P have different behavior. While both don't
> generates linemarkers, gcc also removes all empty lines while clang
> keep them all. We don't need those empty lines, so we don't generates
> them in the final compat/%.h headers. (This replace `uniq` which was
> only de-duplicating empty line.)
> 
> The only changes in the final generated headers it that they don't
> have empty lines anymore.

Making them harder to read? While typically no-one needs to look at
their contents, in case of problems it helps if generated files are
half way accessible to a human as well.

Jan
Anthony PERARD April 16, 2020, 2:17 p.m. UTC | #2
On Wed, Apr 08, 2020 at 03:56:02PM +0200, Jan Beulich wrote:
> On 31.03.2020 12:31, Anthony PERARD wrote:
> > Replace a mix of shell script and python script by all python script.
> > 
> > Remove the unnecessary "grep -v '^# [0-9]'". It is to hide the
> > linemarkers generated by the preprocessor. But adding -P inhibit there
> > generation, thus the grep isn't needed anymore.
> > 
> > gcc -E -P and clang -E -P have different behavior. While both don't
> > generates linemarkers, gcc also removes all empty lines while clang
> > keep them all. We don't need those empty lines, so we don't generates
> > them in the final compat/%.h headers. (This replace `uniq` which was
> > only de-duplicating empty line.)
> > 
> > The only changes in the final generated headers it that they don't
> > have empty lines anymore.
> 
> Making them harder to read? While typically no-one needs to look at
> their contents, in case of problems it helps if generated files are
> half way accessible to a human as well.

I do think they are still readable. Those empty lines don't add much.
There are so many of them that a `uniq` is needed...

For example, with dm_op.h, we have this:

<<<<<<< before
#pragma pack(4)
typedef uint16_t ioservid_compat_t;
struct compat_dm_op_create_ioreq_server {

    uint8_t handle_bufioreq;
    uint8_t pad[3];

    ioservid_compat_t id;
};
struct compat_dm_op_get_ioreq_server_info {

    ioservid_compat_t id;

    uint16_t flags;

    evtchn_port_compat_t bufioreq_port;

    uint64_t ioreq_gfn;

    uint64_t bufioreq_gfn;
};
struct compat_dm_op_ioreq_server_range {

    ioservid_compat_t id;
    uint16_t pad;

    uint32_t type;

    uint64_t start, end;
};
=======
#pragma pack(4)
typedef uint16_t ioservid_compat_t;
struct compat_dm_op_create_ioreq_server {
    uint8_t handle_bufioreq;
    uint8_t pad[3];
    ioservid_compat_t id;
};
struct compat_dm_op_get_ioreq_server_info {
    ioservid_compat_t id;
    uint16_t flags;
    evtchn_port_compat_t bufioreq_port;
    uint64_t ioreq_gfn;
    uint64_t bufioreq_gfn;
};
struct compat_dm_op_ioreq_server_range {
    ioservid_compat_t id;
    uint16_t pad;
    uint32_t type;
    uint64_t start, end;
};
>>>>>>> after

Thanks,
Jan Beulich April 16, 2020, 2:34 p.m. UTC | #3
On 16.04.2020 16:17, Anthony PERARD wrote:
> On Wed, Apr 08, 2020 at 03:56:02PM +0200, Jan Beulich wrote:
>> On 31.03.2020 12:31, Anthony PERARD wrote:
>>> Replace a mix of shell script and python script by all python script.
>>>
>>> Remove the unnecessary "grep -v '^# [0-9]'". It is to hide the
>>> linemarkers generated by the preprocessor. But adding -P inhibit there
>>> generation, thus the grep isn't needed anymore.
>>>
>>> gcc -E -P and clang -E -P have different behavior. While both don't
>>> generates linemarkers, gcc also removes all empty lines while clang
>>> keep them all. We don't need those empty lines, so we don't generates
>>> them in the final compat/%.h headers. (This replace `uniq` which was
>>> only de-duplicating empty line.)
>>>
>>> The only changes in the final generated headers it that they don't
>>> have empty lines anymore.
>>
>> Making them harder to read? While typically no-one needs to look at
>> their contents, in case of problems it helps if generated files are
>> half way accessible to a human as well.
> 
> I do think they are still readable. Those empty lines don't add much.
> There are so many of them that a `uniq` is needed...
> 
> For example, with dm_op.h, we have this:

Let me take a different example, grant_table.h: Not all of the
blank lines it has are useful, but I think the file would suffer
if all of them got removed.

Jan
diff mbox series

Patch

diff --git a/xen/include/Makefile b/xen/include/Makefile
index 74b26a028902..7e2d0ff667e8 100644
--- a/xen/include/Makefile
+++ b/xen/include/Makefile
@@ -53,20 +53,11 @@  all: $(headers-y)
 xlat_lst = xlat.lst
 
 compat/%.h: compat/%.i Makefile $(BASEDIR)/tools/compat-build-header.py
-	set -e; id=_$$(echo $@ | tr '[:lower:]-/.' '[:upper:]___'); \
-	echo "#ifndef $$id" >$@.new; \
-	echo "#define $$id" >>$@.new; \
-	echo "#include <xen/compat.h>" >>$@.new; \
-	$(if $(filter-out compat/arch-%.h,$@),echo "#include <$(patsubst compat/%,public/%,$@)>" >>$@.new;) \
-	$(if $(prefix-y),echo "$(prefix-y)" >>$@.new;) \
-	grep -v '^# [0-9]' $< | \
-	$(PYTHON) $(BASEDIR)/tools/compat-build-header.py | uniq >>$@.new; \
-	$(if $(suffix-y),echo "$(suffix-y)" >>$@.new;) \
-	echo "#endif /* $$id */" >>$@.new
+	$(PYTHON) $(BASEDIR)/tools/compat-build-header.py <$< $@ "$(prefix-y)" "$(suffix-y)" >>$@.new; \
 	mv -f $@.new $@
 
 compat/%.i: compat/%.c Makefile
-	$(CPP) $(filter-out -Wa$(comma)% -include %/include/xen/config.h,$(XEN_CFLAGS)) $(cppflags-y) -o $@ $<
+	$(CPP) -P $(filter-out -Wa$(comma)% -include %/include/xen/config.h,$(XEN_CFLAGS)) $(cppflags-y) -o $@ $<
 
 compat/%.c: public/%.h $(xlat_lst) Makefile $(BASEDIR)/tools/compat-build-source.py
 	mkdir -p $(@D)
diff --git a/xen/tools/compat-build-header.py b/xen/tools/compat-build-header.py
index b85c43f13faf..d89a900ea02c 100755
--- a/xen/tools/compat-build-header.py
+++ b/xen/tools/compat-build-header.py
@@ -2,6 +2,12 @@ 
 
 import re,sys
 
+try:
+    type(str.maketrans)
+except AttributeError:
+    # For python2
+    import string as str
+
 pats = [
  [ r"__InClUdE__(.*)", r"#include\1\n#pragma pack(4)" ],
  [ r"__IfDeF__ (XEN_HAVE.*)", r"#ifdef \1" ],
@@ -20,7 +26,33 @@  pats = [
  [ r"(^|[^\w])long([^\w]|$$)", r"\1int\2" ]
 ];
 
+output_filename = sys.argv[1]
+
+# tr '[:lower:]-/.' '[:upper:]___'
+header_id = '_' + \
+    output_filename.upper().translate(str.maketrans('-/.','___'))
+
+header = """#ifndef {0}
+#define {0}
+#include <xen/compat.h>""".format(header_id)
+
+print(header)
+
+if not re.match("compat/arch-.*.h$", output_filename):
+    x = output_filename.replace("compat/","public/")
+    print('#include <%s>' % x)
+
+def print_if_nonempty(s):
+    if len(s):
+        print(s)
+
+print_if_nonempty(sys.argv[2])
+
 for line in sys.stdin.readlines():
     for pat in pats:
-        line = re.subn(pat[0], pat[1], line)[0]
-    print(line.rstrip())
+        line = re.sub(pat[0], pat[1], line.rstrip())
+    print_if_nonempty(line)
+
+print_if_nonempty(sys.argv[3])
+
+print("#endif /* %s */" % header_id)