diff mbox series

[v3,2/3] xen/banner: Drop the fig-to-oct.py script

Message ID 20191207211634.9958-3-andrew.cooper3@citrix.com (mailing list archive)
State New, archived
Headers show
Series xen: Build fixes related to Python3 | expand

Commit Message

Andrew Cooper Dec. 7, 2019, 9:16 p.m. UTC
The script is 664 rather than 775, so the banner conversion doesn't actually
work if $(PYTHON) is empty:

  /bin/sh: tools/fig-to-oct.py: Permission denied
  make[3]: *** [include/xen/compile.h] Error 126
  make[3]: Leaving directory `/builds/xen-project/people/andyhhp/xen/xen'

Fixing this is easy, but using python here is wasteful.  compile.h doesn't
need XEN_BANNER rendering in octal, and text is much more simple to handle.
Replace fig-to-oct.py with a smaller sed script.  This could be a shell
one-liner, but it is much more simple to comment sensibly, and doesn't need to
include the added cognative load of makefile and shell escaping.

While changing this logic, take the opportunity to optimise the banner
space (and time on the serial port) by dropping trailing whitespace, which is
84 characters for current staging.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: George Dunlap <George.Dunlap@eu.citrix.com>
CC: Ian Jackson <ian.jackson@citrix.com>
CC: Jan Beulich <JBeulich@suse.com>
CC: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
CC: Stefano Stabellini <sstabellini@kernel.org>
CC: Wei Liu <wl@xen.org>
CC: Julien Grall <julien@xen.org>
CC: Juergen Gross <jgross@suse.com>

v2:
 * New
v3:
 * Fix error: backslash-newline at end of file [-Werror]
 * Implement as a sed script.

This (v3) is how happy in the CI:
  https://gitlab.com/xen-project/people/andyhhp/xen/pipelines/101409945

Spotted by Gitlab CI, caused by `which` not being present in some of the
CentOS containers.  While this is more of a container bug than anything else,
it does highlight that the build ought to cope.
---
 xen/Makefile                 |  2 +-
 xen/tools/fig-to-oct.py      | 18 ------------------
 xen/tools/process-banner.sed | 14 ++++++++++++++
 3 files changed, 15 insertions(+), 19 deletions(-)
 delete mode 100644 xen/tools/fig-to-oct.py
 create mode 100755 xen/tools/process-banner.sed

Comments

George Dunlap Dec. 9, 2019, 11:06 a.m. UTC | #1
On 12/7/19 9:16 PM, Andrew Cooper wrote:
> The script is 664 rather than 775, so the banner conversion doesn't actually
> work if $(PYTHON) is empty:
> 
>   /bin/sh: tools/fig-to-oct.py: Permission denied
>   make[3]: *** [include/xen/compile.h] Error 126
>   make[3]: Leaving directory `/builds/xen-project/people/andyhhp/xen/xen'
> 
> Fixing this is easy, but using python here is wasteful.  compile.h doesn't
> need XEN_BANNER rendering in octal, and text is much more simple to handle.
> Replace fig-to-oct.py with a smaller sed script.  This could be a shell
> one-liner, but it is much more simple to comment sensibly, and doesn't need to
> include the added cognative load of makefile and shell escaping.
> 
> While changing this logic, take the opportunity to optimise the banner
> space (and time on the serial port) by dropping trailing whitespace, which is
> 84 characters for current staging.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

It's more work than I would have put into it. :-)  But since it's
already done:

Acked-by: George Dunlap <george.dunlap@citrix.com>
diff mbox series

Patch

diff --git a/xen/Makefile b/xen/Makefile
index 99701e3165..949ca6eb03 100644
--- a/xen/Makefile
+++ b/xen/Makefile
@@ -176,7 +176,7 @@  include/xen/compile.h: include/xen/compile.h.in .banner
 	    -e 's!@@changeset@@!$(shell tools/scmversion $(XEN_ROOT) || echo "unavailable")!g' \
 	    < include/xen/compile.h.in > $@.new
 	@cat .banner
-	@$(PYTHON) tools/fig-to-oct.py < .banner >> $@.new
+	@sed -rf tools/process-banner.sed < .banner >> $@.new
 	@mv -f $@.new $@
 
 include/asm-$(TARGET_ARCH)/asm-offsets.h: arch/$(TARGET_ARCH)/asm-offsets.s
diff --git a/xen/tools/fig-to-oct.py b/xen/tools/fig-to-oct.py
deleted file mode 100644
index db4fd32159..0000000000
--- a/xen/tools/fig-to-oct.py
+++ /dev/null
@@ -1,18 +0,0 @@ 
-#!/usr/bin/env python
-import sys
-
-chars_per_line = 18
-chars_so_far = 0
-
-sys.stdout.write('"')
-
-for char in sys.stdin.read():
-
-    sys.stdout.write("\\%03o" % ord(char))
-    chars_so_far = chars_so_far + 1
-
-    if chars_so_far == chars_per_line:
-        chars_so_far = 0
-        sys.stdout.write('" \\\n"')
-
-sys.stdout.write('"\n')
diff --git a/xen/tools/process-banner.sed b/xen/tools/process-banner.sed
new file mode 100755
index 0000000000..56c76558bc
--- /dev/null
+++ b/xen/tools/process-banner.sed
@@ -0,0 +1,14 @@ 
+#!/bin/sed -rf
+# Process a text input, to turn it into a C string for the XEN_BANNER macro.
+
+# Strip trailing whitespace.
+s_ *$__
+
+# Escape backslashes.
+s_\\_\\\\_g
+
+# Enclose the line in "...\n".
+s_(.*)_"\1\\n"_
+
+# Trailing \ on all but the final line.
+$!s_$_ \\_