[libc++][test] Replace platform.linux_distribution by distro.linux_distribution

[aaronpuchert]

Summary

Since Python 3.5, platform.linux_distribution is deprecated [1], and in
Python 3.8 it has been removed entirely. The supposed replacement is
from the package distro. Unfortunately this adds another dependency to
the tests, but only on Linux (I think).

[1] https://docs.python.org/3.5/library/platform.html#platform.linux_distribution

[aaronpuchert]

An alternative would be to assume that any distribution new enough to have Python 3.8 also has /etc/os-release, so we fall back to parsing that if platform.linux_distribution isn't there. Or the other way around: we check for /etc/os-release and fall back to platform.linux_distribution.

[ldionne]

Is import distro supported in Python 2.x?

[aaronpuchert]

In D70991#1778232, @ldionne wrote:

Is import distro supported in Python 2.x?

It is supported on 2.7, see https://pypi.org/project/distro/.

An alternative to this change would be to assume that distributions recent enough to have Python 3.8 also have /etc/os-release, which has become somewhat of a standard. So we could check if that file exists, then use it if it does and fallback to platform.linux_distribution() if it doesn't. Then we wouldn't need an additional dependency.

[aaronpuchert]

In D70991#1778631, @aaronpuchert wrote:

An alternative to this change would be to assume that distributions recent enough to have Python 3.8 also have /etc/os-release, which has become somewhat of a standard. So we could check if that file exists, then use it if it does and fallback to platform.linux_distribution() if it doesn't. Then we wouldn't need an additional dependency.

@ldionne, what do you think about this ^^?

I'm not really happy about the additional dependency myself and have actually just removed the code in openSUSE to unblock the Python 3.8 update, because we have no blacklisted tests. That's not upstreamable of course.

But checking for /etc/os-release and falling back to platform.linux_distribution() seems like it could solve the problem.

[ldionne]

Do we have a list of dependencies required for running the tests? If so, it might not be unreasonable to add distro. If not, or if that list is empty (i.e. we only need a base Python installation), then I think adding a new dependency has a higher bar and I would go for your other solution.

[aaronpuchert]

On openSUSE we're running the llvm, clang, and libc++ test suite with just python3-base as dependency. This doesn't include the distro package, so it would be the first dependency outside of the Python standard library.

[ldionne]

In D70991#1894615, @aaronpuchert wrote:

On openSUSE we're running the llvm, clang, and libc++ test suite with just python3-base as dependency. This doesn't include the distro package, so it would be the first dependency outside of the Python standard library.

In that case, I suggest we avoid adding an additional dependency.

[aaronpuchert]

Already solved via D72501, not as I proposed, but it should be fine.

libcxx/utils/libcxx/test/target_info.py

 
 class LinuxLocalTI(DefaultTargetInfo):
     def __init__(self, full_config):
         super(LinuxLocalTI, self).__init__(full_config)
 
     def platform(self):
         return 'linux'
 
-    def platform_name(self):
-        name, _, _ = platform.linux_distribution()
+    def platform_name_ver(self):
+        import distro
+        name, ver, _ = distro.linux_distribution()
         # Some distros have spaces, e.g. 'SUSE Linux Enterprise Server'
         # lit features can't have spaces
         name = name.lower().strip().replace(' ', '-')
-        return name # Permitted to be None
-
-    def platform_ver(self):
-        _, ver, _ = platform.linux_distribution()
         ver = ver.lower().strip().replace(' ', '-')
-        return ver # Permitted to be None.
+        return (name, ver) # Both permitted to be None
 
     def add_locale_features(self, features):
         add_common_locales(features, self.full_config.lit_config)
         # Some linux distributions have different locale data than others.
         # Insert the distributions name and name-version into the available
         # features to allow tests to XFAIL on them.
-        name = self.platform_name()
-        ver = self.platform_ver()
+        name, ver = self.platform_name_ver()
         if name:
             features.add(name)
         if name and ver:
             features.add('%s-%s' % (name, ver))
 
     def add_cxx_compile_flags(self, flags):
         flags += ['-D__STDC_FORMAT_MACROS',
                   '-D__STDC_LIMIT_MACROS',