Re: [Qemu-devel] [PATCH for-4.0 1/7] configure: Add a test for the minimum compiler version

[Daniel P . Berrangé]

On Mon, Dec 03, 2018 at 03:27:35PM +0100, Thomas Huth wrote:
> On 2018-12-03 15:16, Daniel P. Berrangé wrote:
> > On Mon, Dec 03, 2018 at 03:05:31PM +0100, Thomas Huth wrote:
> >> So far we only had implicit requirements for the minimum compiler version,
> >> e.g. we require at least GCC 4.1 for the support of atomics. However,
> >> such old compiler versions are not tested anymore by the developers, so
> >> they are not really supported anymore. Since we recently declared 
> >> explicitly
> >> what platforms we intend to support, we can also get more explicit on the
> >> compiler version now. The supported distributions use the following version
> >> of GCC:
> >>
> >>       RHEL-7: 4.8.5
> >>       Debian (Stretch): 6.3.0
> >>       Debian (Jessie): 4.8.4
> >>       OpenBSD (ports): 4.9.4
> >>       FreeBSD (ports): 8.2.0
> >>       OpenSUSE Leap 15: 7.3.1
> >>       Ubuntu (Xenial): 5.3.1
> >>       macOS (Homebrew): 8.2.0
> >>
> >> So we can safely assume GCC 4.8 these days. For Clang, the situation is
> >> a little bit more ambiguous, since it is sometimes not available in the
> >> main distros but rather third party repositories. At least Debian Jessie
> >> uses version 3.5, and EPEL7 for RHEL7 uses 3.4, so let's use 3.4 as
> >> minimum Clang version now - we still can adjust this later if necessary.
> >>
> >> Signed-off-by: Thomas Huth <address@hidden>
> >> ---
> >>  configure | 19 +++++++++++++++++++
> >>  1 file changed, 19 insertions(+)
> >>
> >> diff --git a/configure b/configure
> >> index 0a3c6a7..f1e305e 100755
> >> --- a/configure
> >> +++ b/configure
> >> @@ -1840,6 +1840,25 @@ if test "$bogus_os" = "yes"; then
> >>      error_exit "Unrecognized host OS (uname -s reports '$(uname -s)')"
> >>  fi
> >>  
> >> +# Check whether the compiler matches our minimum requirements:
> >> +cat > $TMPC << EOF
> >> +#if defined(__clang_major__) && defined(__clang_minor__)
> >> +# if __clang_major__ < 3 || (__clang_major__ == 3 && __clang_minor__ < 4)
> >> +#  error You need at least Clang v3.4 to compile QEMU
> >> +# endif
> > 
> > NB although this will succeed, it is not technically checking the
> > right thing on macOS platforms as their clang used a completely
> > different numbering scheme. It just happens to have larger numbers
> > than upstream clang, so we'll always succeed.
> 
> Does that only apply to the version string that is reported by "clang
> --version" or does it also apply to the __clang_major/minor__ macros?
> 
> In the latter case, I think I could add a comment to the patch here.
> 
> I'm also not sure whether the LLVM version necessarily matches the Clang
> version? In older versions, you can also see "4.2 (clang-425.0.28)
> (based on LLVM 3.2svn)" in the tables there...

The first number in the clang version string is what we're interested
in, and that matches the XCode version number, and is what's reflected
in the __clang_major/minor__ macros.  The the LLVM version in the table
reflects what upstream release they forked the XCode version from.

> > IOW, to require clang >= 3.4 on macOS, we would need to require
> > their version >= 5.1
> 
> If they also messed up the macros, I think we likely have a problem the
> other way round: What upstream Clang version corresponds to the Apple
> Clang version 3.4 ? ... but if I interpret the old table right, they
> just started the confusion with 5.0, so I think the check above should
> still be fine.

I think you're mis-reading it. There is  no Apple Clang version 3.4 - at
least not in this table.

The earliest version Apple Clang version is 5.0 (in XCode 5.0.0), and
that corresponds to upstream CLang version 3.3

Effectively what this means is that we need a separate check for Apple's
version of CLang. We can't simply use __apple__ as that would fire if
someone had built upstream clang on macOS. Instead we need to toggle
on  __apple_build_version__ which is specific to Apple's fork

 #if defined(__clang_major__) && defined(__clang_minor__)
 #  ifdef __apple_build_version__
 #    if __clang_major__ < 5 || (__clang_major__ == 5 && __clang_minor__ < 1)
 #      error You need at least XCode CLang v5.1 to compile QEMU
 #    endif
 #  else
 #    if __clang_major__ < 3 || (__clang_major__ == 3 && __clang_minor__ < 4)
 #      error You need at least Clang v3.4 to compile QEMU
 #    endif
 #  endif
 ....gcc....


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|