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

Thomas Huth posted 7 patches 6 years, 10 months ago
[Qemu-devel] [PATCH for-4.0 1/7] configure: Add a test for the minimum compiler version
Posted by Thomas Huth 6 years, 10 months ago
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 <thuth@redhat.com>
---
 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
+#elif defined(__GNUC__) && defined(__GNUC_MINOR__)
+# if __GNUC__ < 4 || (__GNUC__ == 4 && __GNUC_MINOR__ < 8)
+#  error You need at least GCC v4.8 to compile QEMU
+# endif
+#else
+# error You either need GCC or Clang to compiler QEMU
+#endif
+int main (void) { return 0; }
+EOF
+if ! compile_prog "" "" ; then
+    error_exit "You need at least GCC v4.8 or Clang v3.4 to compiler QEMU"
+fi
+
 gcc_flags="-Wold-style-declaration -Wold-style-definition -Wtype-limits"
 gcc_flags="-Wformat-security -Wformat-y2k -Winit-self -Wignored-qualifiers $gcc_flags"
 gcc_flags="-Wno-missing-include-dirs -Wempty-body -Wnested-externs $gcc_flags"
-- 
1.8.3.1


Re: [Qemu-devel] [PATCH for-4.0 1/7] configure: Add a test for the minimum compiler version
Posted by Daniel P. Berrangé 6 years, 10 months ago
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 <thuth@redhat.com>
> ---
>  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.

Compare clang on Ubuntu:

  https://travis-ci.org/qemu/qemu/jobs/460892603

  $ clang --version
  clang version 5.0.0 (tags/RELEASE_500/final)

with clang on macOS:

  https://travis-ci.org/qemu/qemu/jobs/460892611

  $ clang --version
  Apple LLVM version 9.1.0 (clang-902.0.39.2)

Fortunately there's a mapping recorded here:

   https://en.wikipedia.org/wiki/Xcode#Latest_versions

See "LLVM" and "CLang version string" columns.

IOW, to require clang >= 3.4 on macOS, we would need to require
their version >= 5.1

> +#elif defined(__GNUC__) && defined(__GNUC_MINOR__)
> +# if __GNUC__ < 4 || (__GNUC__ == 4 && __GNUC_MINOR__ < 8)
> +#  error You need at least GCC v4.8 to compile QEMU
> +# endif
> +#else
> +# error You either need GCC or Clang to compiler QEMU
> +#endif
> +int main (void) { return 0; }
> +EOF
> +if ! compile_prog "" "" ; then
> +    error_exit "You need at least GCC v4.8 or Clang v3.4 to compiler QEMU"
> +fi
> +
>  gcc_flags="-Wold-style-declaration -Wold-style-definition -Wtype-limits"
>  gcc_flags="-Wformat-security -Wformat-y2k -Winit-self -Wignored-qualifiers $gcc_flags"
>  gcc_flags="-Wno-missing-include-dirs -Wempty-body -Wnested-externs $gcc_flags"
> -- 
> 1.8.3.1
> 

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 :|

Re: [Qemu-devel] [PATCH for-4.0 1/7] configure: Add a test for the minimum compiler version
Posted by Peter Maydell 6 years, 10 months ago
On Mon, 3 Dec 2018 at 14:16, Daniel P. Berrangé <berrange@redhat.com> wrote:
> 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.
>
> Compare clang on Ubuntu:
>
>   https://travis-ci.org/qemu/qemu/jobs/460892603
>
>   $ clang --version
>   clang version 5.0.0 (tags/RELEASE_500/final)
>
> with clang on macOS:
>
>   https://travis-ci.org/qemu/qemu/jobs/460892611
>
>   $ clang --version
>   Apple LLVM version 9.1.0 (clang-902.0.39.2)
>
> Fortunately there's a mapping recorded here:
>
>    https://en.wikipedia.org/wiki/Xcode#Latest_versions
>
> See "LLVM" and "CLang version string" columns.

...but overall it is better to check for feature presence/absence
where possible rather than using a version number check.
clang does at least make this easier than it has historically
been for gcc.

thanks
-- PMM

Re: [Qemu-devel] [PATCH for-4.0 1/7] configure: Add a test for the minimum compiler version
Posted by Thomas Huth 6 years, 10 months ago
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 <thuth@redhat.com>
>> ---
>>  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...

> 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.

 Thomas

Re: [Qemu-devel] [PATCH for-4.0 1/7] configure: Add a test for the minimum compiler version
Posted by Daniel P. Berrangé 6 years, 10 months ago
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 <thuth@redhat.com>
> >> ---
> >>  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 :|

Re: [Qemu-devel] [PATCH for-4.0 1/7] configure: Add a test for the minimum compiler version
Posted by Philippe Mathieu-Daudé 6 years, 10 months ago
On 12/3/18 3:05 PM, 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

I'd like to track this in a machine parsable format, but sure where it
better fits however, I'd prefer the git repo, and having the wiki
pointing to the git repo.

[...]

Re: [Qemu-devel] [PATCH for-4.0 1/7] configure: Add a test for the minimum compiler version
Posted by Thomas Huth 6 years, 10 months ago
On 2018-12-05 18:30, Philippe Mathieu-Daudé wrote:
> On 12/3/18 3:05 PM, 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
> 
> I'd like to track this in a machine parsable format, but sure where it
> better fits however, I'd prefer the git repo, and having the wiki
> pointing to the git repo.

I don't think that it makes sense to put fixed version numbers into the
git or wiki - the information will expire soon, and it is additional
maintenance to keep them up to date. We already got the generic
description here:

https://qemu.weilnetz.de/doc/qemu-doc.html#Supported-build-platforms

So you just have to follow these instructions to get to the supported
versions.

 Thomas

Re: [Qemu-devel] [PATCH for-4.0 1/7] configure: Add a test for the minimum compiler version
Posted by Daniel P. Berrangé 6 years, 10 months ago
On Thu, Dec 06, 2018 at 06:34:25AM +0100, Thomas Huth wrote:
> On 2018-12-05 18:30, Philippe Mathieu-Daudé wrote:
> > On 12/3/18 3:05 PM, 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
> > 
> > I'd like to track this in a machine parsable format, but sure where it
> > better fits however, I'd prefer the git repo, and having the wiki
> > pointing to the git repo.
> 
> I don't think that it makes sense to put fixed version numbers into the
> git or wiki - the information will expire soon, and it is additional
> maintenance to keep them up to date. We already got the generic
> description here:
> 
> https://qemu.weilnetz.de/doc/qemu-doc.html#Supported-build-platforms
> 
> So you just have to follow these instructions to get to the supported
> versions.

Agreed, I don't really see a benefit in tracking the per-distro versions
in QEMU - they're only useful for informing us abuot what min version
to pick, which then goes into configure.

If we absolutely wanted to, we could use a standard variable name in
configure to make it easy to grep for min versions.

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 :|