RFR(M): 8057538: Build the freetype library during configure on Windows

Magnus Ihse Bursie magnus.ihse.bursie at oracle.com
Tue Sep 9 12:38:44 UTC 2014


Hi Volker,

Sorry for not responding to this earlier.

As I said in the bug, I really like this idea!

I have a few comments.

I understand why you would have liked to add the 
ac_executable_extensions stuff, but since you can't due to performance, 
and that is not likely to change in the future either, the extra 
commented-out code in platform.m4 serves no purpose and should be removed.

The description when locating msbuild is enough I think, maybe rewritten 
like this:
# We need to check for 'msbuild.exe' because at the place where we expect to
# find 'msbuild.exe' there's also a directory called 'msbuild' and configure
# won't find the 'msbuild.exe' executable in that case (and
# 'ac_executable_extensions' is unusable due to performance reasons).

However, I'm thinking if maybe the check for msbuild.exe should move to 
toolchain_windows.m4, maybe at the end of 
TOOLCHAIN_SETUP_VISUAL_STUDIO_ENV. It's a bit extra messy (like you 
don't normalize the path), and it's part of the microsoft build tool 
chain mess. :-) If so, you can use the fact that the msbuild.exe you 
want to use is located in VS_PATH, and use that as search path to 
AC_CHECK_PROG. Or maybe that's not useful, I dunno. I'm not 100% sure 
about this though, so if you think it's better were it is, leave it.

Also, I'm not entirely happy on the complexity of the freetype detection 
code. It's a beast. :-/ It's not your fault, and I'm not sure what do to 
counter that, but at least I think it would help if you could extract 
the relevant part of your new compilation code into a separate function 
so we at least try to keep it from growing further (remember that it 
need to be AC_DEFUN and not AC_DEFUN_ONCE in that case).

I must compliment you on the well-designed feedback to the user about 
possible errors and what to do about them! I really like that.

Apart from this minor issues, your code looks good. (For the records, I 
have read the code but not tested it.)

/Magnus



More information about the build-dev mailing list