Hi Vincent,

On Sun, Sep 29, 2013 at 10:27:30PM +0200, Vincent Bernat wrote:
>  ??? 29 septembre 2013 18:30 CEST, Willy Tarreau <[email protected]> :
> 
> > So maybe we should in fact stop setting PCREDIR to $(pcre-config --prefix),
> > which will result in PCRE_INC/PCRE_LIB remaining silent unless PCREDIR is
> > forced. I suspect the following patch should fix it :
> >
> > diff --git a/Makefile b/Makefile
> > index 0529e89..89f9f39 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -544,7 +544,6 @@ ifneq ($(USE_PCRE)$(USE_STATIC_PCRE)$(USE_PCRE_JIT),)
> >  # Forcing PCREDIR to an empty string will let the compiler use the default
> >  # locations.
> >  
> > -PCREDIR            := $(shell pcre-config --prefix 2>/dev/null || echo 
> > /usr/local)
> >  ifneq ($(PCREDIR),)
> >  PCRE_INC   := $(PCREDIR)/include
> >  PCRE_LIB   := $(PCREDIR)/lib
> 
> I would use `pcre-config --libs` and `pcre-config --cflags` instead. The
> user can still override this on make command line.
> 
> PCRE_CFLAGS := $(shell pcre-config --cflags)
> PCRE_LIBS := $(shell pcre-config --libs)

But these would still cause the issue we're currently facing because
they'll add -L/usr/lib or whatever is in the system path, so that does
not fix the issue. Really I'd rather get rid of pcre-config completely.
Anyway, in cross-compile environments, you can't use it and you have
to force the value otherwise you're screwed, and in local builds, you
simply don't need to specify them.

> As for the ordering of -L, I would expect the user to specify `make
> LDFLAGS="-L/tmp/openssl/static"` and you rename the internal `LDFLAGS`
> to something else (instead of introducing another variable, LDFLAGS is
> the variable that a user can expect to override). $(LDFLAGS) will be
> used early (as currently done), so the specified directory will be first
> in the search path.

That's interesting. In fact, the worst problem we have is that (except
for ADDINC/ADDLIB), the variables the user can specify already hold a
value and are overridden. That's how we added more and more variables,
so that you can specify extra build information without losing existing
ones.

I've just checked, and all my build scripts already specify LDFLAGS, but
just for passing -g/-s/-static (basically what we can expect in LDFLAGS),
but none of them expects LDFLAGS to also override ARCH_FLAGS. So I think
it could already improve things if instead of having :

   LDFLAGS = $(ARCH_FLAGS) -g
   ...
   $(LD) $(LDFLAGS) -o $@ $^ $(LDOPTS)

we had :

   LDFLAGS = -g
   ...
   $(LD) $(ARCH_FLAGS) $(LDFLAGS) -o $@ $^ $(LDOPTS)

or even ideally :

   ARCH_LDFLAGS = $(ARCH_FLAGS)
   LDFLAGS = -g
   ...
   $(LD) $(ARCH_LDFLAGS) $(LDFLAGS) -o $@ $^ $(LDOPTS)

I think we wouldn't break existing build scripts by such a small change
and would make it easier for packagers to force some paths without the
risk of overriding the arch-specific flags.

Willy


Reply via email to