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

[Colin Watson]

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                                       address@hidden