[flashrom] 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 <jmayer at loplof.de>
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
-- 
Joerg Mayer                                           <jmayer at loplof.de>
We are stuck with technology when what we really want is just stuff that
works. Some say that should read Microsoft instead of technology.