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

Reply via email to