All of lore.kernel.org
 help / color / mirror / Atom feed
From: Daniel Kiper <dkiper@net-space.pl>
To: Colin Watson <cjwatson@ubuntu.com>
Cc: grub-devel@gnu.org
Subject: Re: [PATCH] Fix syslinux_test in out-of-tree builds
Date: Mon, 14 Jan 2019 14:15:14 +0100	[thread overview]
Message-ID: <20190114131514.q2iwhatl75iwdmau@tomti.i.net-space.pl> (raw)
In-Reply-To: <20190109145912.tqlk4r6yo7nhuvoq@riva.ucam.org>

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


  reply	other threads:[~2019-01-14 13:15 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-01-09 14:59 [PATCH] Fix syslinux_test in out-of-tree builds Colin Watson
2019-01-14 13:15 ` Daniel Kiper [this message]
2019-02-27 10:26   ` Colin Watson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190114131514.q2iwhatl75iwdmau@tomti.i.net-space.pl \
    --to=dkiper@net-space.pl \
    --cc=cjwatson@ubuntu.com \
    --cc=grub-devel@gnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
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.