All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] Fix syslinux_test in out-of-tree builds
@ 2019-01-09 14:59 Colin Watson
  2019-01-14 13:15 ` Daniel Kiper
  0 siblings, 1 reply; 3+ messages in thread
From: Colin Watson @ 2019-01-09 14:59 UTC (permalink / raw)
  To: grub-devel

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 ++++++-
 tests/syslinux/ubuntu10.04_grub.cfg.in | 20 ++++++++++----------
 2 files changed, 16 insertions(+), 11 deletions(-)

diff --git a/Makefile.am b/Makefile.am
index a52a998a1..51e4f127d 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'`; \
+	(for x in tests/syslinux/ubuntu10.04_grub.cfg.in ; do sed "s,@simplified_abs_top_srcdir@,$$simplified_abs_top_srcdir,g" $(srcdir)/"$$x"; done) | $(top_builddir)/config.status --file=$@:-
 CLEANFILES += tests/syslinux/ubuntu10.04_grub.cfg
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


^ permalink raw reply related	[flat|nested] 3+ messages in thread
* Re: [PATCH] Fix syslinux_test in out-of-tree builds
  2019-01-09 14:59 [PATCH] Fix syslinux_test in out-of-tree builds Colin Watson
@ 2019-01-14 13:15 ` Daniel Kiper
  2019-02-27 10:26   ` Colin Watson
  0 siblings, 1 reply; 3+ messages in thread
From: Daniel Kiper @ 2019-01-14 13:15 UTC (permalink / raw)
  To: Colin Watson; +Cc: grub-devel

On Wed, Jan 09, 2019 at 02:59:12PM +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>
> ---
>  Makefile.am                            |  7 ++++++-
>  tests/syslinux/ubuntu10.04_grub.cfg.in | 20 ++++++++++----------
>  2 files changed, 16 insertions(+), 11 deletions(-)
>
> diff --git a/Makefile.am b/Makefile.am
> index a52a998a1..51e4f127d 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

OK, but I would like to see a comment before
grub-core/lib/syslinux_parse.c:simplify_filename() saying that somebody
changing its code should take care of the code here too. Otherwise
sooner or later the tests will be broken again due oversight.

> +# 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'`; \
> +	(for x in tests/syslinux/ubuntu10.04_grub.cfg.in ; do sed "s,@simplified_abs_top_srcdir@,$$simplified_abs_top_srcdir,g" $(srcdir)/"$$x"; done) | $(top_builddir)/config.status --file=$@:-

I think that you can drop this for.

>  CLEANFILES += tests/syslinux/ubuntu10.04_grub.cfg
> 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:
Hmmm... Why number of "/" increases from top to down --->^^^^

Daniel


^ permalink raw reply	[flat|nested] 3+ messages in thread
* Re: [PATCH] Fix syslinux_test in out-of-tree builds
  2019-01-14 13:15 ` Daniel Kiper
@ 2019-02-27 10:26   ` Colin Watson
  0 siblings, 0 replies; 3+ messages in thread
From: Colin Watson @ 2019-02-27 10:26 UTC (permalink / raw)
  To: The development of GNU GRUB

On Mon, Jan 14, 2019 at 02:15:14PM +0100, Daniel Kiper wrote:
> On Wed, Jan 09, 2019 at 02:59:12PM +0000, Colin Watson wrote:
> > +# Mimic simplify_filename from grub-core/lib/syslinux_parse.c, so that we
> 
> OK, but I would like to see a comment before
> grub-core/lib/syslinux_parse.c:simplify_filename() saying that somebody
> changing its code should take care of the code here too. Otherwise
> sooner or later the tests will be broken again due oversight.

Fair enough.

> > +# 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'`; \
> > +	(for x in tests/syslinux/ubuntu10.04_grub.cfg.in ; do sed "s,@simplified_abs_top_srcdir@,$$simplified_abs_top_srcdir,g" $(srcdir)/"$$x"; done) | $(top_builddir)/config.status --file=$@:-
> 
> I think that you can drop this for.

Done.

> >  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:
> Hmmm... Why number of "/" increases from top to down --->^^^^

I guess sometimes get_target_filename is called with an empty string or
something and it doesn't bother to simplify that?  This wasn't
introduced by my patch, so I haven't worked out the exact code paths
involved.

I'll send a v2.

-- 
Colin Watson                                       [cjwatson@ubuntu.com]


^ permalink raw reply	[flat|nested] 3+ messages in thread
end of thread, other threads:[~2019-02-27 10:26 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-09 14:59 [PATCH] Fix syslinux_test in out-of-tree builds Colin Watson
2019-01-14 13:15 ` Daniel Kiper
2019-02-27 10:26   ` Colin Watson

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.