geotiff test in configure doesn't use specified path

[kyngchaos@…]

When using the --with-geotiff option to specify a location for an external geotiff, configure doesn't add a -L$with_geotiff/lib flag to the test, yet does add it to the LIBS if it succeeds.

  GEOTIFF_SETTING=external

    echo "$as_me:$LINENO: checking for XTIFFClientOpen in -lgeotiff" >&5
...
LIBS="-lgeotiff  $LIBS"
cat >conftest.$ac_ext <<_ACEOF

So, if geotiff is in an isolated location (not in a path for a check that succeeded already and thus in LIBS at this point), the check will not succeed.

[hobu]

William,

I have made a commit to configure.in to attempt to add the test, but I'm not sure I did it in the right place.  Can you re-up and confirm it is fixed?

Howard

[russo@…]

The change you made yesterday does not work.  There are several issues with it.

First, you added "-L$with-geotiff/lib" to LIBS, but there is no such variable --- what winds up getting into LIBS is whatever the value of "$with" is followed by the string "-geotiff."  The result on my system (FreeBSD) is that "-L-geotiff/lib" is there in LIBS and the links at the end fail due to an invalid directory.  You probably meant to use "$with_geotiff"

The other issue is that "$with_geotiff" does not necessarily contain the directory containing the geotiff library.  When I patch configure by hand to replace "$with-geotiff" with "$with_geotiff" in the LIBS line modification, what winds up being there is "-L/lib".

From the look of configure.in, "$with_geotiff" could have the value "yes" rather than a directory name, or (as in my case) could be blank.  So it is not safe to include $with_geotiff/lib in the -L string unconditionally.  If it's yes or blank, it should be expected to be found by the AC_CHECK_LIB call.  Only if it's an actual directory should it be used in a -L flag.


So where you have at line 603 of revision 1.211 of configure.in:

  if test "$GEOTIFF_SETTING" = "external" ; then   
    LIBS="-L$with-geotiff/lib -lgeotiff $LIBS"
    echo "using pre-installed libgeotiff."
  else
    echo "using internal GeoTIFF code."
  fi

it should be instead something more along these lines:

  if test "$GEOTIFF_SETTING" = "external" ; then   
    LIBS="-lgeotiff $LIBS"
    if [ -d $with_geotiff ]; then
      LIBS="-L$with_geotiff/lib $LIBS"
    fi
    echo "using pre-installed libgeotiff."
  else
    echo "using internal GeoTIFF code."
  fi

Also possibly problematic is the earlier use of "-L$with_geotiff/lib" in the AC_CHECK_LIB call at line 598 of the same revision:

if test "$with_geotiff" = "yes" -o "$with_geotiff" = "" ; then

  if test "$TIFF_SETTING" = "internal" ; then
    GEOTIFF_SETTING=internal
  else
    dnl We now require libgeotiff 1.2.1 (for XTIFFClientOpen). 
    AC_CHECK_LIB(geotiff,XTIFFClientOpen,GEOTIFF_SETTING=external,GEOTIFF_SETTIN
G=internal,-L$with_geotiff/lib)

[...]

which doesn't make sense, as the AC_CHECK_LIB call is using -L$with_geotiff/lib right in a block that is only executed if $with_geotiff is known to be "yes" or blank.

In this case, the final argument to AC_CHECK_LIB should be removed, as it will never be a valid -L flag anyway.

The real issue originally reported would be fixed by changing the later lines (lines around line 616 of revision 1.211):

else

  GEOTIFF_SETTING=external

  dnl We now require libgeotiff 1.2.1 (for XTIFFClientOpen). 
  AC_CHECK_LIB(geotiff,XTIFFClientOpen,GEOTIFF_SETTING=external,AC_MSG_ERROR([We
 require at least GeoTIFF 1.2.1. Consider using the one supplied with GDAL]))

  if test -r $with_geotiff/libgeotiff.a ; then
    LIBS="-L$with_geotiff -lgeotiff $LIBS"
    EXTRA_INCLUDES="-I$with_geotiff $EXTRA_INCLUDES"
  else

to say this instead:
else

  GEOTIFF_SETTING=external

  dnl We now require libgeotiff 1.2.1 (for XTIFFClientOpen). 
  AC_CHECK_LIB(geotiff,XTIFFClientOpen,GEOTIFF_SETTING=external,AC_MSG_ERROR([We
 require at least GeoTIFF 1.2.1. Consider using the one supplied with GDAL]),-L$with_geotiff/lib)

  if test -r $with_geotiff/lib/libgeotiff.a ; then
    LIBS="-L$with_geotiff/lib -lgeotiff $LIBS"
    EXTRA_INCLUDES="-I$with_geotiff/include $EXTRA_INCLUDES"
  else

or something like it, depending on what the user is expected to provide when specifying "--with-geotiff=/path" --- if it's the path to the library itself, then it was already wrong because of the "-I$with_geotiff" in the EXTRA_INCLUDES (the includes would rarely be in the same directory as the library), and if it's the path to the parent directory of the lib directory in which libgeotiff is found, then it was wrong because it was including "-L$with_geotiff" instead of "-L$with_geotiff/lib"

[russo@…]

On second thought, the first suggestion I made (adding a "if [ -d $with_geotiff]" deal) doesn't make sense, because it's in the same block of configure that is only executed if $with_geotiff is "yes" or blank.

I'm attaching a patch shortly that should do the trick.  The patch is against revision 1.211 of configure.in.  It assumes that the path that the user is expected to give to --with-geotiff is the directory above the "lib" directory containing the library (i.e. that $with_geotiff/lib/libgeotiff.a is what we want to link against).  If that is wrong, then the whole thing needs a minor tweak, and one must make sure that the directory used in the EXTRA_INCLUDES is right, too.

[russo@…]

Patch that should let --with-geotiff=/path/to/geotiff work, and restore correct function if not specified.

[hobu]

Tom,

Thanks for the patch.  I have applied it in current CVS.  Please verify that it is doing what it is supposed to if you can.

Howard

[russo@…]

I don't know if it solves William's issue, he should check if it does.  But in looking over my patch I see that it has a mistake in the second part of the patch:

  dnl We now require libgeotiff 1.2.1 (for XTIFFClientOpen). 
  AC_CHECK_LIB(geotiff,XTIFFClientOpen,GEOTIFF_SETTING=external,AC_MSG_ERROR([We require at least GeoTIFF 1.2.1. Consider using the one supplied with GDAL]),-L$with_geotiff/lib)

  if test -r $with_geotiff/lib/libgeotiff.a ; then
    LIBS="-L$with_geotiff/lib -lgeotiff $LIBS"
    EXTRA_INCLUDES="-I$with_geotiff/include $EXTRA_INCLUDES"
  else
    LIBS="-L$with_geotiff/lib -lgeotiff $LIBS"
    EXTRA_INCLUDES="-I$with_geotiff/include $EXTRA_INCLUDES"
  fi

  echo "using libgeotiff from $with_geotiff."


This is silly, as it does the same thing in both the "if" and "else" sections.  I'm sorry for that.  On my system I wasn't even using this section, so this error didn't stand out.

The whole check for geotiff is iffy and should probably be re-written to use AC_CHECK_LIB in a more standard pattern. But a small tweak should probably be enough to get the job done without the effort involved in doing it over.  I don't have time to make and test a patch, but I'll suggest a change that might be better than what I submitted before.

If the code above (which is now in CVS if you applied my patch) were replaced with something like:


  dnl We now require libgeotiff 1.2.1 (for XTIFFClientOpen). 
  AC_CHECK_LIB(geotiff,XTIFFClientOpen,GEOTIFF_SETTING=external,GEOTIFF_SETTING=not_found,-L$with_geotiff/lib)

  if test $GEOTIFF_SETTING = "external" ; then
    LIBS="-L$with_geotiff/lib -lgeotiff $LIBS"
    if test -d $with_geotiff/include ; then
      EXTRA_INCLUDES="-I$with_geotiff/include $EXTRA_INCLUDES"
    fi
  else
    dnl check if libgeotiff is in $with_geotiff instead of $with_geotiff/lib, e.g. an uninstalled build in its source directory
    AC_CHECK_LIB(geotiff,XTIFFClientOpen,GEOTIFF_SETTING=external,AC_MSG_ERROR([We require at least GeoTIFF 1.2.1. Consider using the one supplied with GDAL]),-L$with_geotiff)
    if test $GEOTIFF_SETTING = "external" ; 
      LIBS="-L$with_geotiff -lgeotiff $LIBS"
      EXTRA_INCLUDES="-I$with_geotiff $EXTRA_INCLUDES"
  fi

  echo "using libgeotiff from $with_geotiff."

What changes is that we first try AC_CHECK_LIB in $with_geotiff/lib to see if we find libgeotiff, and if not, then try $with_geotiff.  It removes the very questionable test for the existence of $with_geotiff/lib/libgeotiff.a, and replaces it with a check of whether AC_CHECK_LIB succeeded.  If the check in $with_geotiff also fails, we bomb the same way as before, but if it succeeds, we assume it's an uninstalled build (that appears to be the assumption that was there before the patch) and so the includes and .a file are in the same directory.

This would still be a little iffy, as it simply assumes that the include files are in $with_geotiff/include if the library is found in $with_geotiff/lib, and similarly assumes includes are in $with_geotiff if the lib is in $with_geotiff.  Ideally the probe would use AC_CHECK_HEADER properly to find the right headers, but to add that would probably be to compound the problem, and it would be better to rewrite the test altogether than to band-aid it further.

William:  you might try making the change I suggest above and see if your build works. 

I'm sorry I didn't follow up on this before you applied my patch.  I had not added myself to the "cc" list and so didn't see your follow-up comment until I checked this bug just now.  I hope this new suggestion is closer to the mark.

[hobu]

Tom,

Can you supply a patch against current CVS to implement the logic you described?

Howard

[russo@…]

Bleah.  The formatting on the suggested replacement code was a mess, and there   was a syntax error.  I'm preparing a proper patch now, and will attach it in a    few seconds.

[russo@…]

Another, possibly better patch

[hobu]

applied in latest CVS.  Please test and confirm guys.

Howard

[kyngchaos@…]

works for me.  using custom geotiff install prefix, static libgeotiff.  configure test succeeds, lib path is in gdalmake.opt LIBS and include path is in CPPFLAGS, as they should be.

thanks

[warmerdam]

I have done a bunch of testing with the new configure and it seems to work
as desired.

[hobu]

Declaring this one dead.