[PATCH v2] Fix syslinux_test in out-of-tree builds

[PATCH v2] Fix syslinux_test in out-of-tree builds

[Colin Watson]

syslinux_parse simplifies some filenames by removing things like ".."
segments, but the tests assumed that @abs_top_srcdir@ would be
untouched, which is not true in the case of out-of-tree builds where
@abs_top_srcdir@ may contain ".." segments.

Performing the substitution requires some awkwardness in Makefile.am due
to details of how config.status works.

Signed-off-by: Colin Watson <cjwatson@ubuntu.com>
---
 Makefile.am                            |  7 ++++++-
 grub-core/lib/syslinux_parse.c         |  3 +++
 tests/syslinux/ubuntu10.04_grub.cfg.in | 20 ++++++++++----------
 3 files changed, 19 insertions(+), 11 deletions(-)

diff --git a/Makefile.am b/Makefile.am
index a52a998a1..5b3cf5b4d 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -476,6 +476,11 @@ EXTRA_DIST += ChangeLog ChangeLog-2015
 
 syslinux_test: $(top_builddir)/config.status tests/syslinux/ubuntu10.04_grub.cfg
 
+# Mimic simplify_filename from grub-core/lib/syslinux_parse.c, so that we
+# can predict its behaviour in tests.  We have to pre-substitute this before
+# calling config.status, as config.status offers no reliable way to hook in
+# a command between setting ac_abs_top_srcdir and emitting output files.
 tests/syslinux/ubuntu10.04_grub.cfg: $(top_builddir)/config.status tests/syslinux/ubuntu10.04_grub.cfg.in
-	(for x in tests/syslinux/ubuntu10.04_grub.cfg.in ; do cat $(srcdir)/"$$x"; done) | $(top_builddir)/config.status --file=$@:-
+	simplified_abs_top_srcdir=`echo "$(abs_top_srcdir)" | sed 's,//,/,g; s,/\./,/,g; :loop; s,/[^/][^/]*/\.\.\(/\|$$\),\1,; t loop'`; \
+	sed "s,@simplified_abs_top_srcdir@,$$simplified_abs_top_srcdir,g" $(srcdir)/tests/syslinux/ubuntu10.04_grub.cfg.in | $(top_builddir)/config.status --file=$@:-
 CLEANFILES += tests/syslinux/ubuntu10.04_grub.cfg
diff --git a/grub-core/lib/syslinux_parse.c b/grub-core/lib/syslinux_parse.c
index c96d85ee5..c117c584d 100644
--- a/grub-core/lib/syslinux_parse.c
+++ b/grub-core/lib/syslinux_parse.c
@@ -807,6 +807,9 @@ print_file (struct output_buffer *outbuf,
   return print_escaped (outbuf, from, to);
 }
 
+/* Makefile.am mimics this when generating
+   tests/syslinux/ubuntu10.04_grub.cfg, so changes here may need to be
+   reflected there too.  */
 static void
 simplify_filename (char *str)
 {
diff --git a/tests/syslinux/ubuntu10.04_grub.cfg.in b/tests/syslinux/ubuntu10.04_grub.cfg.in
index 846e4acf0..441dec045 100644
--- a/tests/syslinux/ubuntu10.04_grub.cfg.in
+++ b/tests/syslinux/ubuntu10.04_grub.cfg.in
@@ -41,7 +41,7 @@ menuentry 'Test memory' --hotkey 'm' --id 'memtest' {
   linux$linux_suffix '/'/'/install/mt86plus' 
 }
 menuentry 'Boot from first hard disk' --hotkey 'b' --id 'hd' {
-# File (host)/@abs_top_srcdir@/tests/syslinux/ubuntu10.04/isolinux/gtk.cfg not found
+# File (host)/@simplified_abs_top_srcdir@/tests/syslinux/ubuntu10.04/isolinux/gtk.cfg not found
   # UNSUPPORTED command 'menu begin advanced'
   # UNSUPPORTED command 'menu title Advanced options'
   # UNSUPPORTED command 'menu color title	* #FFFFFFFF *'
@@ -63,14 +63,14 @@ menuentry 'Boot from first hard disk' --hotkey 'b' --id 'hd' {
 }
 menuentry 'Back..' --hotkey 'b' --id 'mainmenu' {
   # UNSUPPORTED command 'menu exit'
-# File (host)/@abs_top_srcdir@/tests/syslinux/ubuntu10.04/isolinux/adgtk.cfg not found
+# File (host)/@simplified_abs_top_srcdir@/tests/syslinux/ubuntu10.04/isolinux/adgtk.cfg not found
   # UNSUPPORTED command 'menu end'
   # UNSUPPORTED entry type 0
 true;
 }
 menuentry 'Help' --hotkey 'h' --id 'help' {
   # UNSUPPORTED command 'ui gfxboot bootlogo'
-#'@abs_top_srcdir@/tests/syslinux/ubuntu10.04/isolinux'/'prompt.cfg' (host)@abs_top_srcdir@/tests/syslinux/ubuntu10.04/isolinux/prompt.cfg:
+#'@abs_top_srcdir@/tests/syslinux/ubuntu10.04/isolinux'/'prompt.cfg' (host)@simplified_abs_top_srcdir@/tests/syslinux/ubuntu10.04/isolinux/prompt.cfg:
   background_image '@abs_top_srcdir@/tests/syslinux/ubuntu10.04/isolinux/'/'splash.png'
   # UNSUPPORTED command 'display f1.txt'
   # UNSUPPORTED command 'menu hshift 13'
@@ -114,7 +114,7 @@ menuentry 'Test memory' --hotkey 'm' --id 'memtest' {
   linux$linux_suffix '/'/'/install/mt86plus' 
 }
 menuentry 'Boot from first hard disk' --hotkey 'b' --id 'hd' {
-# File (host)/@abs_top_srcdir@/tests/syslinux/ubuntu10.04/isolinux//gtk.cfg not found
+# File (host)/@simplified_abs_top_srcdir@/tests/syslinux/ubuntu10.04/isolinux//gtk.cfg not found
   # UNSUPPORTED command 'menu begin advanced'
   # UNSUPPORTED command 'menu title Advanced options'
   # UNSUPPORTED command 'menu color title	* #FFFFFFFF *'
@@ -136,13 +136,13 @@ menuentry 'Boot from first hard disk' --hotkey 'b' --id 'hd' {
 }
 menuentry 'Back..' --hotkey 'b' --id 'mainmenu' {
   # UNSUPPORTED command 'menu exit'
-# File (host)/@abs_top_srcdir@/tests/syslinux/ubuntu10.04/isolinux//adgtk.cfg not found
+# File (host)/@simplified_abs_top_srcdir@/tests/syslinux/ubuntu10.04/isolinux//adgtk.cfg not found
   # UNSUPPORTED command 'menu end'
   # UNSUPPORTED entry type 0
 true;
 }
 menuentry 'Help' --hotkey 'h' --id 'help' {
-#'@abs_top_srcdir@/tests/syslinux/ubuntu10.04/isolinux/'/'prompt.cfg' (host)@abs_top_srcdir@/tests/syslinux/ubuntu10.04/isolinux/prompt.cfg:
+#'@abs_top_srcdir@/tests/syslinux/ubuntu10.04/isolinux/'/'prompt.cfg' (host)@simplified_abs_top_srcdir@/tests/syslinux/ubuntu10.04/isolinux/prompt.cfg:
   syslinux_configfile -r '/'/'/' -c '@abs_top_srcdir@/tests/syslinux/ubuntu10.04/isolinux/'/'' '@abs_top_srcdir@/tests/syslinux/ubuntu10.04/isolinux/'/'prompt.cfg'
 }
 menuentry 'menu' --id 'menu' {
@@ -156,7 +156,7 @@ menuentry 'menu' --id 'menu' {
   # UNSUPPORTED command 'f8 f8.txt'
   # UNSUPPORTED command 'f9 f9.txt'
   # UNSUPPORTED command 'f0 f10.txt'
-#'@abs_top_srcdir@/tests/syslinux/ubuntu10.04/isolinux/'/'isolinux.cfg' (host)@abs_top_srcdir@/tests/syslinux/ubuntu10.04/isolinux/isolinux.cfg:
+#'@abs_top_srcdir@/tests/syslinux/ubuntu10.04/isolinux/'/'isolinux.cfg' (host)@simplified_abs_top_srcdir@/tests/syslinux/ubuntu10.04/isolinux/isolinux.cfg:
   background_image '@abs_top_srcdir@/tests/syslinux/ubuntu10.04/isolinux//'/'splash.png'
 # D-I config version 2.0
   # UNSUPPORTED command 'menu hshift 13'
@@ -200,7 +200,7 @@ menuentry 'Test memory' --hotkey 'm' --id 'memtest' {
   linux$linux_suffix '/'/'/install/mt86plus' 
 }
 menuentry 'Boot from first hard disk' --hotkey 'b' --id 'hd' {
-# File (host)/@abs_top_srcdir@/tests/syslinux/ubuntu10.04/isolinux///gtk.cfg not found
+# File (host)/@simplified_abs_top_srcdir@/tests/syslinux/ubuntu10.04/isolinux///gtk.cfg not found
   # UNSUPPORTED command 'menu begin advanced'
   # UNSUPPORTED command 'menu title Advanced options'
   # UNSUPPORTED command 'menu color title	* #FFFFFFFF *'
@@ -222,14 +222,14 @@ menuentry 'Boot from first hard disk' --hotkey 'b' --id 'hd' {
 }
 menuentry 'Back..' --hotkey 'b' --id 'mainmenu' {
   # UNSUPPORTED command 'menu exit'
-# File (host)/@abs_top_srcdir@/tests/syslinux/ubuntu10.04/isolinux///adgtk.cfg not found
+# File (host)/@simplified_abs_top_srcdir@/tests/syslinux/ubuntu10.04/isolinux///adgtk.cfg not found
   # UNSUPPORTED command 'menu end'
   # UNSUPPORTED entry type 0
 true;
 }
 menuentry 'Help' --hotkey 'h' --id 'help' {
   # UNSUPPORTED command 'ui gfxboot bootlogo'
-#'@abs_top_srcdir@/tests/syslinux/ubuntu10.04/isolinux//'/'prompt.cfg' (host)@abs_top_srcdir@/tests/syslinux/ubuntu10.04/isolinux/prompt.cfg:
+#'@abs_top_srcdir@/tests/syslinux/ubuntu10.04/isolinux//'/'prompt.cfg' (host)@simplified_abs_top_srcdir@/tests/syslinux/ubuntu10.04/isolinux/prompt.cfg:
   syslinux_configfile -r '/'/'/' -c '@abs_top_srcdir@/tests/syslinux/ubuntu10.04/isolinux//'/'' '@abs_top_srcdir@/tests/syslinux/ubuntu10.04/isolinux//'/'prompt.cfg'
 }
 }
-- 
2.20.1

Re: [PATCH v2] Fix syslinux_test in out-of-tree builds

[Daniel Kiper]

On Wed, Feb 27, 2019 at 10:26:30AM +0000, Colin Watson wrote:
> syslinux_parse simplifies some filenames by removing things like ".."
> segments, but the tests assumed that @abs_top_srcdir@ would be
> untouched, which is not true in the case of out-of-tree builds where
> @abs_top_srcdir@ may contain ".." segments.
>
> Performing the substitution requires some awkwardness in Makefile.am due
> to details of how config.status works.
>
> Signed-off-by: Colin Watson <cjwatson@ubuntu.com>

Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>

Daniel

Rules for committers? (was Re: [PATCH v2] Fix syslinux_test in out-of-tree builds)

[Colin Watson]

On Fri, Mar 01, 2019 at 12:52:37PM +0100, Daniel Kiper wrote:
> On Wed, Feb 27, 2019 at 10:26:30AM +0000, Colin Watson wrote:
> > Signed-off-by: Colin Watson <cjwatson@ubuntu.com>
> 
> Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>

This reminds me: as somebody who has direct GRUB commit access but
hasn't used it much directly since the new maintenance team took the
reins, can I ask if there are established rules written down for how we
should be using our commit access these days?  It seems that it would
probably be more efficient if those of us who have commit access were to
push our own patches once they've been suitably reviewed rather than
relying on the smaller team of maintainers to do it.

Is the rule basically "has a Reviewed-by from a maintainer", or is it
more liberal than that?

Also, it would be good if this sort of thing were written down in
docs/grub-dev.texi rather than just in a mailing list response.  (I'd be
happy to try to polish a mailing list response into a patch to the
developer manual, though.)

-- 
Colin Watson                                       [cjwatson@ubuntu.com]