On Wed, Nov 27, 2013 at 10:15:09PM +0200, Martin Storsjö wrote:
> On Wed, 27 Nov 2013, Diego Biurrun wrote:
> >--- a/configure
> >+++ b/configure
> >@@ -3355,45 +3380,52 @@ esac
> >
> ># determine libc flavour
> >
> >-# uclibc defines __GLIBC__, so it needs to be checked before glibc.
> >-if check_cpp_condition features.h "defined __UCLIBC__"; then
> >- libc_type=uclibc
> >- add_cppflags -D_POSIX_C_SOURCE=200112 -D_XOPEN_SOURCE=600
> >-elif check_cpp_condition features.h "defined __GLIBC__"; then
> >- libc_type=glibc
> >- add_cppflags -D_POSIX_C_SOURCE=200112 -D_XOPEN_SOURCE=600
> >-# MinGW headers can be installed on Cygwin, so check for newlib first.
> >-elif check_cpp_condition newlib.h "defined _NEWLIB_VERSION"; then
> >- libc_type=newlib
> >- add_cppflags -U__STRICT_ANSI__
> >-elif check_header _mingw.h; then
> >- libc_type=mingw
> >- check_cpp_condition _mingw.h \
> >- "defined (__MINGW64_VERSION_MAJOR) || (__MINGW32_MAJOR_VERSION > 3)
> >|| \
> >- (__MINGW32_MAJOR_VERSION == 3 && __MINGW32_MINOR_VERSION >=
> >15)" ||
> >- die "ERROR: MinGW runtime version must be >= 3.15."
> >- add_cppflags -U__STRICT_ANSI__
> >-elif check_func_headers stdlib.h _get_doserrno; then
> >- libc_type=msvcrt
> >- add_compat strtod.o strtod=avpriv_strtod
> >- add_compat msvcrt/snprintf.o snprintf=avpriv_snprintf \
> >- _snprintf=avpriv_snprintf \
> >- vsnprintf=avpriv_vsnprintf
> >- # The MSVC 2010 headers (Win 7.0 SDK) set _WIN32_WINNT to
> >- # 0x601 by default unless something else is set by the user.
> >- # This can easily lead to us detecting functions only present
> >- # in such new versions and producing binaries requiring windows 7.0.
> >- # Therefore explicitly set the default to XP unless the user has
> >- # set something else on the command line.
> >- check_cpp_condition stdlib.h "defined(_WIN32_WINNT)" || add_cppflags
> >-D_WIN32_WINNT=0x0502
> >-elif check_cpp_condition stddef.h "defined __KLIBC__"; then
> >- libc_type=klibc
> >-elif check_cpp_condition sys/cdefs.h "defined __BIONIC__"; then
> >- libc_type=bionic
> >- add_compat strtod.o strtod=avpriv_strtod
> >-fi
> >+probe_libc(){
> >+ pfx=$1
> >+ # uclibc defines __GLIBC__, so it needs to be checked before glibc.
> >+ if check_${pfx}cpp_condition features.h "defined __UCLIBC__"; then
> >+ eval ${pfx}libc_type=uclibc
> >+ add_${pfx}cppflags -D_POSIX_C_SOURCE=200112 -D_XOPEN_SOURCE=600
> >+ elif check_${pfx}cpp_condition features.h "defined __GLIBC__"; then
> >+ eval ${pfx}libc_type=glibc
> >+ add_${pfx}cppflags -D_POSIX_C_SOURCE=200112 -D_XOPEN_SOURCE=600
> >+ # MinGW headers can be installed on Cygwin, so check for newlib first.
> >+ elif check_${pfx}cpp_condition newlib.h "defined _NEWLIB_VERSION"; then
> >+ eval ${pfx}libc_type=newlib
> >+ add_${pfx}cppflags -U__STRICT_ANSI__
> >+ elif check_${pfx}header _mingw.h; then
> >+ eval ${pfx}libc_type=mingw
> >+ mingw_condition="defined (__MINGW64_VERSION_MAJOR) ||
> >(__MINGW32_MAJOR_VERSION > 3) || (__MINGW32_MAJOR_VERSION == 3 &&
> >__MINGW32_MINOR_VERSION >= 15)"
> >+ check_${pfx}cpp_condition _mingw.h "defined $mingw_condition " ||
> >+ die "ERROR: MinGW runtime version must be >= 3.15."
>
> Any technical reason to split out mingw_condition into a separate
> variable here, or is it just an unrelated refactoring you're doing
> at the same time (which raises a few questions for a reviewer)? If
> there's no pressing need for it (keeping lines short is not enough,
> when it comes to a nontrivial change as this one), I'd rather have
> it kept as before, to reduce the amount of changes here. It can
> always be refactored in a separate commit with an adequate
> explanation.
I /think/ it failed back when I tested this on MinGW, but I have to
test again now. I just rebased on master and tested under Linux to
get some initial comments, hence the [RFC]. Otherwise I completely
agree with you in general.
> >+ add_${pfx}cppflags -U__STRICT_ANSI__
> >+ elif check_${pfx}func_headers stdlib.h _get_doserrno; then
> >+ eval ${pfx}libc_type=msvcrt
> >+ add_compat strtod.o strtod=avpriv_strtod
> >+ add_compat msvcrt/snprintf.o snprintf=avpriv_snprintf \
> >+ _snprintf=avpriv_snprintf \
> >+ vsnprintf=avpriv_vsnprintf
>
> Things like add_compat here (and below) should only be done if this
> is the target libc, not if it's the host one. In general, every
> single action (add/set/whatever) in this needs to include $pfx in
> one way or another, otherwise it's doing something wrong.
When using msvc as host compiler this is not necessary?
> >+ # The MSVC 2010 headers (Win 7.0 SDK) set _WIN32_WINNT to
> >+ # 0x601 by default unless something else is set by the user.
> >+ # This can easily lead to us detecting functions only present
> >+ # in such new versions and producing binaries requiring windows 7.0.
> >+ # Therefore explicitly set the default to XP unless the user has
> >+ # set something else on the command line.
> >+ check_${pfx}cpp_condition stdlib.h "defined(_WIN32_WINNT)" ||
> >+ add_${pfx}cppflags -D_WIN32_WINNT=0x0502
> >+ elif check_${pfx}cpp_condition stddef.h "defined __KLIBC__"; then
> >+ eval ${pfx}libc_type=klibc
> >+ elif check_${pfx}cpp_condition sys/cdefs.h "defined __BIONIC__"; then
> >+ eval ${pfx}libc_type=bionic
> >+ add_compat strtod.o strtod=avpriv_strtod
> >+ fi
> >+}
> >+
> >+probe_libc
> >+probe_libc host_
> >
> >test -n "$libc_type" && enable $libc_type
> >+test -n "$host_libc_type" && enable $host_libc_type
>
> Not quite. We can't just do "enable $host_libc_type", because we've
> got things like "log2_deps=!msvcrt" - what if target is something
> random (e.g. crosscompiling) but the host libc happens to be msvcrt?
>
> I'd say it only makes sense to "enable" the target libc (for use in
> deps or other checks later), and not enable the host libc at all
> (requiring tests that check for it to check for host_libc_type =
> "msvcrt"), or enable this one with a different pattern (e.g. "enable
> host_$host_libc_type").
Good point. This is why I sent this off as an RFC, a fresh pair of eyes :)
Diego
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel
