Allow using a system-installed giflib
Erik Joelsson
erik.joelsson at oracle.com
Fri Mar 15 09:42:20 UTC 2013
On 2013-03-15 04:10, Andrew Hughes wrote:
> ----- Original Message -----
>> Hi,
>>
>> Updated webrev at:
>> http://cr.openjdk.java.net/~omajid/webrevs/system-giflib/01/
>>
>> The webrev borrows some idioms that Andrew Hughes pointed out, and
>> uses
>> system headers for giflib too.
>>
>> I checked this by building --with-libgif=system and also by removing
>> the
>> src/share/native/sun/awt/giflib dir and using --with-giflib=system.
>>
> This looks better, though I'd still like to see it using a more
> standard --enable format. That then removes the need to have that
> third error case. I'll post a webrev to fix both this and zlib once
> committed.
>
Agree, ok from me.
>> On 03/08/2013 12:26 PM, Phil Race wrote:
>>> If I understand correctly, this removes the directory containing
>>> the JDK's copy of giflib sources from the set of locations to be
>>> compiled etc, and replaces it with just a link line pointer to use
>>> "libgif" which is then expected to be on the default linker path,
>>> ie in /usr/lib.
>>>
>>> I think this is fine except I would guard USE_EXTERNAL_LIBGIF
>>> with an expectation that this is at least "not" windows, since
>>> I'm pretty sure that option would always be a mistake there.
>> The updated webrev does that. I don't have a windows machine to test
>> this on, unfortunately.
>>
> This does cause some duplication. Is this really a major issue? It's
> not the default and, if I was someone on Windows trying to get system
> giflib working, this would just get in the way. We had to remove something
> similar in the zlib case that blocked it if not on Mac OS X.
>
This part is weird to me. Sure, trying to set --with-giflib=system on
windows is wrong, but that's why configure is verifying that it's
actually there and that the toolchain is able to compile and link
against it. I just tried the patch on windows, and configure fails at
finding giflib, even if it's installed in cygwin, as expected. I do not
think we should be hand holding to this extent in the makefiles. If we
really want to guard this on windows, do it in configure, with a helpful
error message, rather then silently ignoring the option in make. But as
pointed out, it's really not necessary.
/Erik
More information about the build-dev
mailing list