Fix feature detection when fdti requires libusb for linking

[Joerg Mayer]

see attached patch. patch is too small to be copyrightable...

ciao
     Joerg

Fix feature detection when fdti requires libusb for linking

[Carl-Daniel Hailfinger]

Hi Jörg,

thanks for your patch.
Review follows.

On 23.08.2009 07:52, Joerg Mayer wrote:
> Index: Makefile
>
> --- Makefile	(revision 696)
> +++ Makefile	(working copy)
> @@ -75,12 +75,13 @@
>  endif
>  endif
>  
> +FTDILIBS = $(shell pkg-config --libs libftdi)

AFAICS this breaks if pkg-config is not installed. Maybe use -lftdi as
fallback? Besides that, on my machine it looks like this:
# pkg-config --libs libftdi
-L/usr/local/lib -lftdi -lusb

However, compiling and linking with -lftdi (and no other linker
parameter) works fine here. I'm not familiar enough with pkg-config to
know why this is the case.

Wouldn't we need
# pkg-config --cflags libftdi
for FEATURE_CFLAGS as well?
> FEATURE_CFLAGS += $(shell LC_ALL=C grep -q "FTDISUPPORT := yes" .features && printf "%s" "-D'FT2232_SPI_SUPPORT=1'")
>  
> -FEATURE_LIBS += $(shell LC_ALL=C grep -q "FTDISUPPORT := yes" .features && printf "%s" "-lftdi")
> +FEATURE_LIBS += $(shell LC_ALL=C grep -q "FTDISUPPORT := yes" .features && echo $(FTDILIBS))
>  
>  $(PROGRAM): $(OBJS)
> $(CC) $(LDFLAGS) -o $(PROGRAM) $(OBJS) $(LIBS) $(FEATURE_LIBS)
> $(CC) $(LDFLAGS) -o $(PROGRAM) $(OBJS) $(FEATURE_LIBS) $(LIBS)

Is this one just cosmetic or am I missing some deep meaning?
> # TAROPTIONS reduces information leakage from the packager's system.
>  # If other tar programs support command line arguments for setting uid/gid of
> @@ -133,7 +134,7 @@
>    	   echo "struct ftdi_context *ftdic = NULL;";	   \
>    	   echo "int main(int argc, char **argv)"; \
>    	   echo "{ return ftdi_init(ftdic); }"; ) > .featuretest.c )
> @$(CC) $(CFLAGS) $(LDFLAGS) .featuretest.c -o .featuretest $(LIBS) -lftdi >/dev/null 2>&1 &&	\
> @$(CC) $(CFLAGS) $(LDFLAGS) .featuretest.c -o .featuretest $(FTDILIBS) $(LIBS)>/dev/null 2>&1 &&	\

OK.
> ( echo "found."; echo "FTDISUPPORT := yes" >> .features.tmp ) ||	\
> ( echo "not found."; echo "FTDISUPPORT := no" >> .features.tmp )
>
> @$(DIFF) -q .features.tmp .features >/dev/null 2>&1 && rm .features.tmp || mv .features.tmp .features

When you resubmit (or simply answer the questions above), please add a
Signed-off-by statement as well. Details of this procedure are explained at
http://www.coreboot.org/Development_Guidelines#Sign-off_Procedure

Regards,
Carl-Daniel

Fix feature detection when fdti requires libusb for linking

[Stefan Reinauer]

On 8/24/09 4:05 AM, Carl-Daniel Hailfinger wrote:
> Hi Jörg,
>
> thanks for your patch.
> Review follows.
>
> On 23.08.2009 07:52, Joerg Mayer wrote:
> > Index: Makefile
> >
> > --- Makefile	(revision 696)
> > +++ Makefile	(working copy)
> > @@ -75,12 +75,13 @@
> >  endif
> >  endif
> >  
> > +FTDILIBS = $(shell pkg-config --libs libftdi)
>
> AFAICS this breaks if pkg-config is not installed.

Maybe we should make pkg-config a requirement? The same problem happens
with libpci and libz.

Or is there a better way to solve this?

Stefan

Fix feature detection when fdti requires libusb for linking

[Joerg Mayer]

On Mon, Aug 24, 2009 at 04:05:37AM +0200, Carl-Daniel Hailfinger wrote:
> On 23.08.2009 07:52, Joerg Mayer wrote:
> > Index: Makefile
> >
> > --- Makefile	(revision 696)
> > +++ Makefile	(working copy)
> > @@ -75,12 +75,13 @@
> >  endif
> >  endif
> >  
> > +FTDILIBS = $(shell pkg-config --libs libftdi)
>
> AFAICS this breaks if pkg-config is not installed. Maybe use -lftdi as
> fallback? Besides that, on my machine it looks like this:
> # pkg-config --libs libftdi
> -L/usr/local/lib -lftdi -lusb

You are right, for this to work properly we'd need to check for pkg-config
first, with the downside of making the Makefile more and more unreadable.
> However, compiling and linking with -lftdi (and no other linker
> parameter) works fine here. I'm not familiar enough with pkg-config to
> know why this is the case.

The reason has nothing at all to do with pkg-config, it has to do with the
way libftdi is built on my system: It has usb support and thus reqiures that
whenever I link against libftdi, I need to link against libusb as well.
> Wouldn't we need
> # pkg-config --cflags libftdi
> for FEATURE_CFLAGS as well?

Yes, that would be the proper way - I didn't do it for the same reasons that
I left out the pkg-config check: It isn't needed on my system :-) and because
it makes the Makefile more complex.
> > FEATURE_CFLAGS += $(shell LC_ALL=C grep -q "FTDISUPPORT := yes" .features && printf "%s" "-D'FT2232_SPI_SUPPORT=1'")
> >  
> > -FEATURE_LIBS += $(shell LC_ALL=C grep -q "FTDISUPPORT := yes" .features && printf "%s" "-lftdi")
> > +FEATURE_LIBS += $(shell LC_ALL=C grep -q "FTDISUPPORT := yes" .features && echo $(FTDILIBS))
> >  
> >  $(PROGRAM): $(OBJS)
> > $(CC) $(LDFLAGS) -o $(PROGRAM) $(OBJS) $(LIBS) $(FEATURE_LIBS)
> > $(CC) $(LDFLAGS) -o $(PROGRAM) $(OBJS) $(FEATURE_LIBS) $(LIBS)
>
> Is this one just cosmetic or am I missing some deep meaning?

IIRC, some systems or some linker flags require that the libs are listed
in dependency order - so lets call that cosmetic.
> > # TAROPTIONS reduces information leakage from the packager's system.
> >  # If other tar programs support command line arguments for setting uid/gid of
> > @@ -133,7 +134,7 @@
> >    	   echo "struct ftdi_context *ftdic = NULL;";	   \
> >    	   echo "int main(int argc, char **argv)"; \
> >    	   echo "{ return ftdi_init(ftdic); }"; ) > .featuretest.c )
> > @$(CC) $(CFLAGS) $(LDFLAGS) .featuretest.c -o .featuretest $(LIBS) -lftdi >/dev/null 2>&1 &&	\
> > @$(CC) $(CFLAGS) $(LDFLAGS) .featuretest.c -o .featuretest $(FTDILIBS) $(LIBS)>/dev/null 2>&1 &&	\
>
> OK.
> > ( echo "found."; echo "FTDISUPPORT := yes" >> .features.tmp ) ||	\
> > ( echo "not found."; echo "FTDISUPPORT := no" >> .features.tmp )
> >
> > @$(DIFF) -q .features.tmp .features >/dev/null 2>&1 && rm .features.tmp || mv .features.tmp .features

Given all the details above: Change things as you like and feel free to
add a
Signed-off-by: Joerg Mayer 
line.

As an alternative, I offer to write a CMakefile for flashrom. While
I'm no expert working with CMake, I've worked with it a bit and am confident
that I could handle the rather trivial dependencies this project has.
Thas is to say getting rid of "make features" and handling that stuff via
CMake.
The downside is that flashrom requires a new build tool and the project
maintainers need to know something about it. Also, *right now* it still is a
bit of an overkill. The good thing is, that the project is bound to acquire
new features (no, I'm not talking about sending email ;) and it looks like other
platforms might like to pick up flashrom as well - and CMake is multiplatform
including Windows.

Ciao
       Joerg

[PATCH] libftdi detection via pkg-config, LIBS dependency order

[Carl-Daniel Hailfinger]

Retrieve the proper linker flags for libftdi via pkg-config and fall
back if pkg-config isn't available or if it doesn't know libftdi.

Fix $LIBS and $FEATURE_LIBS to honor dependency order.

The original patch is from Jörg, it has been updated to work on the
current tree and to have a fallback in case pkg-config is not available
or not working.

Signed-off-by: Jörg Mayer 
Acked-by: Carl-Daniel Hailfinger

Index: flashrom-libftdi_pkgconfig/Makefile
===================================================================
--- flashrom-libftdi_pkgconfig/Makefile	(Revision 761)
+++ flashrom-libftdi_pkgconfig/Makefile	(Arbeitskopie)
@@ -120,10 +120,11 @@
 OBJS += satasii.o
 endif

+FTDILIBS := $(shell pkg-config --libs libftdi 2>/dev/null || printf "%s" "-lftdi -lusb")
 ifeq ($(CONFIG_FT2232SPI), yes)
 # This is a totally ugly hack.
 FEATURE_CFLAGS += $(shell LC_ALL=C grep -q "FTDISUPPORT := yes" .features && printf "%s" "-D'FT2232_SPI_SUPPORT=1'")
-FEATURE_LIBS += $(shell LC_ALL=C grep -q "FTDISUPPORT := yes" .features && printf "%s" "-lftdi")
+FEATURE_LIBS += $(shell LC_ALL=C grep -q "FTDISUPPORT := yes" .features && printf "%s" "$(FTDILIBS)")
 OBJS += ft2232_spi.o
 endif

@@ -146,7 +147,7 @@
 FEATURE_LIBS += $(shell LC_ALL=C grep -q "NEEDLIBZ := yes" .libdeps && printf "%s" "-lz")

$(PROGRAM): $(OBJS)
-	$(CC) $(LDFLAGS) -o $(PROGRAM) $(OBJS) $(LIBS) $(FEATURE_LIBS)
+	$(CC) $(LDFLAGS) -o $(PROGRAM) $(OBJS) $(FEATURE_LIBS) $(LIBS)

# TAROPTIONS reduces information leakage from the packager's system.
 # If other tar programs support command line arguments for setting uid/gid of
@@ -218,7 +219,7 @@
    	   echo "struct ftdi_context *ftdic = NULL;";	   \
    	   echo "int main(int argc, char **argv)"; \
    	   echo "{ return ftdi_init(ftdic); }"; ) > .featuretest.c )
-	@$(CC) $(CFLAGS) $(LDFLAGS) .featuretest.c -o .featuretest $(LIBS) -lftdi >/dev/null 2>&1 &&	\
+	@$(CC) $(CFLAGS) $(LDFLAGS) .featuretest.c -o .featuretest $(FTDILIBS) $(LIBS) >/dev/null 2>&1 &&	\
    	( echo "found."; echo "FTDISUPPORT := yes" >> .features.tmp ) ||	\
    	( echo "not found."; echo "FTDISUPPORT := no" >> .features.tmp )
    @$(DIFF) -q .features.tmp .features >/dev/null 2>&1 && rm .features.tmp || mv .features.tmp .features

[PATCH] libftdi detection via pkg-config, LIBS dependency order

[Carl-Daniel Hailfinger]

Jörg checked it, and told me to go ahead.

Committed in r762.

Regards,
Carl-Daniel