Re: [PATCH] 18969 - multiple string test failures due to missing locale dependencies

[Mike Frysinger <vapier at gentoo dot org>]

On 26 Sep 2015 15:00, Martin Sebor wrote:
> On 09/26/2015 12:24 PM, Mike Frysinger wrote:
> > On 24 Sep 2015 15:58, Martin Sebor wrote:
> >> --- a/string/Makefile
> >> +++ b/string/Makefile
> >> @@ -75,4 +75,13 @@ ifeq ($(run-built-tests),yes)
> >>    $(objpfx)tst-svc-cmp.out: tst-svc.expect $(objpfx)tst-svc.out
> >>           cmp $^ > $@; \
> >>           $(evaluate-test)
> >> +
> >> +LOCALES = de_DE.UTF-8 en_US.ISO-8859-1 en_US.UTF-8 tr_TR.ISO-8859-9
> >> tr_TR.UTF-8
> >> +include ../gen-locales.mk
> >
> > should this be wrapped in ifeq ($(run-built-tests),yes) ?
> 
> The patch does insert the whole hunk into an existing block that
> is guarded by the conditional (see the line that starts with @@),

yeah, looks like just one more line of context was needed ;)

> although what I suspect really needs to be wrapped is the use of
> the gen-locales variable defined in gen-locales.mk. IIUC, whether
> or not the include is wrapped shouldn't make a practical difference
> since when gen-locales isn't used nothing should depend on the
> locales.

patch as-is looks ok then

> FWIW, when I wrote the patch I looked for other makefiles that
> include gen-locales.mk for examples. I found just two: one that
> includes it unconditionally (benchtests/Makefile) and another
> that does so only when run-built-tests is defined to yes
> (localedata/Makefile). I went with the latter approach on
> the assumption that it's more likely to be correct. But I'd
> be happy to be corrected.

the benchtests is meant to be run more directly.  i wouldn't
really consider it as an example here.
-mike

Attachment: signature.asc
Description: Digital signature