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
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
# 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.)
More information about the build-dev