From 7190724ba9d9782066062b1663a063e0caf1f591 Mon Sep 17 00:00:00 2001 From: Toshio Kuratomi Date: Wed, 12 Dec 2018 11:12:00 -0800 Subject: [PATCH] Prefer the VERSION_CODENAME field of os-release to parsing it from VERSION Preferring the codename field lets distributions accurately encode their codename preferences instead of us having to parse them from the VERSION in ways that may be error prone. This came up in two known instances: * Ubuntu 16.04 has a VERSION string of "16.04.1 LTS (Xenial Xerces)" and a VERSION_CODENAME of "xenial". distro was mistakenly returning "xenial xerces" because of the way the VERSION string was formatted. * Fedora 21+ do not have a code name. Our parser was returning the variant because Fedora sticks the variant in parenthesis in the VERSION field. Fedora 28+ will add an empty VERSION_CODENAME field so that we can recognize they do not have a code name: https://src.fedoraproject.org/rpms/fedora-release/pull-request/58# --- distro.py | 47 ++++++---- .../distros/fedora30/etc/fedora-release | 1 + .../resources/distros/fedora30/etc/os-release | 1 + .../distros/fedora30/etc/redhat-release | 1 + .../distros/fedora30/etc/system-release | 1 + .../distros/fedora30/usr/lib/os-release | 19 ++++ .../distros/ubuntu16/bin/lsb_release | 39 +++++++++ .../distros/ubuntu16/etc/debian_version | 1 + .../distros/ubuntu16/etc/lsb-release | 4 + .../resources/distros/ubuntu16/etc/os-release | 10 +++ tests/test_distro.py | 86 ++++++++++++++++++- 11 files changed, 190 insertions(+), 20 deletions(-) create mode 100644 tests/resources/distros/fedora30/etc/fedora-release create mode 120000 tests/resources/distros/fedora30/etc/os-release create mode 120000 tests/resources/distros/fedora30/etc/redhat-release create mode 120000 tests/resources/distros/fedora30/etc/system-release create mode 100644 tests/resources/distros/fedora30/usr/lib/os-release create mode 100755 tests/resources/distros/ubuntu16/bin/lsb_release create mode 100644 tests/resources/distros/ubuntu16/etc/debian_version create mode 100644 tests/resources/distros/ubuntu16/etc/lsb-release create mode 100644 tests/resources/distros/ubuntu16/etc/os-release diff --git a/distro.py b/distro.py index 80a9036..5d454fa 100755 --- a/distro.py +++ b/distro.py @@ -812,10 +812,14 @@ def codename(self): For details, see :func:`distro.codename`. """ - return self.os_release_attr('codename') \ - or self.lsb_release_attr('codename') \ - or self.distro_release_attr('codename') \ - or '' + try: + # Handle os_release specially since distros might purposefully set + # this to empty string to have no codename + return self._os_release_info['codename'] + except KeyError: + return self.lsb_release_attr('codename') \ + or self.distro_release_attr('codename') \ + or '' def info(self, pretty=False, best=False): """ @@ -963,23 +967,30 @@ def _parse_os_release_content(lines): if isinstance(v, bytes): v = v.decode('utf-8') props[k.lower()] = v - if k == 'VERSION': - # this handles cases in which the codename is in - # the `(CODENAME)` (rhel, centos, fedora) format - # or in the `, CODENAME` format (Ubuntu). - codename = re.search(r'(\(\D+\))|,(\s+)?\D+', v) - if codename: - codename = codename.group() - codename = codename.strip('()') - codename = codename.strip(',') - codename = codename.strip() - # codename appears within paranthese. - props['codename'] = codename - else: - props['codename'] = '' else: # Ignore any tokens that are not variable assignments pass + + if 'version_codename' in props: + # os-release added a version_codename field. Use that in + # preference to anything else Note that some distros purposefully + # do not have code names. They should be setting + # version_codename="" + props['codename'] = props['version_codename'] + elif 'ubuntu_codename' in props: + # Same as above but a non-standard field name used on older Ubuntus + props['codename'] = props['ubuntu_codename'] + elif 'version' in props: + # If there is no version_codename, parse it from the version + codename = re.search(r'(\(\D+\))|,(\s+)?\D+', props['version']) + if codename: + codename = codename.group() + codename = codename.strip('()') + codename = codename.strip(',') + codename = codename.strip() + # codename appears within paranthese. + props['codename'] = codename + return props @cached_property diff --git a/tests/resources/distros/fedora30/etc/fedora-release b/tests/resources/distros/fedora30/etc/fedora-release new file mode 100644 index 0000000..88caaa8 --- /dev/null +++ b/tests/resources/distros/fedora30/etc/fedora-release @@ -0,0 +1 @@ +Fedora release 30 (Thirty) diff --git a/tests/resources/distros/fedora30/etc/os-release b/tests/resources/distros/fedora30/etc/os-release new file mode 120000 index 0000000..c4c75b4 --- /dev/null +++ b/tests/resources/distros/fedora30/etc/os-release @@ -0,0 +1 @@ +../usr/lib/os-release \ No newline at end of file diff --git a/tests/resources/distros/fedora30/etc/redhat-release b/tests/resources/distros/fedora30/etc/redhat-release new file mode 120000 index 0000000..fc49fa6 --- /dev/null +++ b/tests/resources/distros/fedora30/etc/redhat-release @@ -0,0 +1 @@ +fedora-release \ No newline at end of file diff --git a/tests/resources/distros/fedora30/etc/system-release b/tests/resources/distros/fedora30/etc/system-release new file mode 120000 index 0000000..fc49fa6 --- /dev/null +++ b/tests/resources/distros/fedora30/etc/system-release @@ -0,0 +1 @@ +fedora-release \ No newline at end of file diff --git a/tests/resources/distros/fedora30/usr/lib/os-release b/tests/resources/distros/fedora30/usr/lib/os-release new file mode 100644 index 0000000..2a82709 --- /dev/null +++ b/tests/resources/distros/fedora30/usr/lib/os-release @@ -0,0 +1,19 @@ +NAME=Fedora +VERSION="30 (Thirty)" +ID=fedora +VERSION_ID=30 +VERSION_CODENAME="" +PLATFORM_ID="platform:f30" +PRETTY_NAME="Fedora 30 (Thirty)" +ANSI_COLOR="0;34" +LOGO=fedora-logo-icon +CPE_NAME="cpe:/o:fedoraproject:fedora:30" +HOME_URL="https://fedoraproject.org/" +DOCUMENTATION_URL="https://docs.fedoraproject.org/en-US/fedora/30/system-administrators-guide/" +SUPPORT_URL="https://fedoraproject.org/wiki/Communicating_and_getting_help" +BUG_REPORT_URL="https://bugzilla.redhat.com/" +REDHAT_BUGZILLA_PRODUCT="Fedora" +REDHAT_BUGZILLA_PRODUCT_VERSION=30 +REDHAT_SUPPORT_PRODUCT="Fedora" +REDHAT_SUPPORT_PRODUCT_VERSION=30 +PRIVACY_POLICY_URL="https://fedoraproject.org/wiki/Legal:PrivacyPolicy" diff --git a/tests/resources/distros/ubuntu16/bin/lsb_release b/tests/resources/distros/ubuntu16/bin/lsb_release new file mode 100755 index 0000000..a385c9e --- /dev/null +++ b/tests/resources/distros/ubuntu16/bin/lsb_release @@ -0,0 +1,39 @@ +#!/bin/bash +# +# lsb_release command for testing the ld module. +# Only the -a option is supported. +# +# This version of the lsb_release command reads an lsb-release file. +# +# The lsb-release file has the usual format, e.g.: +# DISTRIB_ID=Ubuntu +# DISTRIB_RELEASE=14.04 +# DISTRIB_CODENAME=trusty +# DISTRIB_DESCRIPTION="Ubuntu 14.04.3 LTS" +# Where each line is optional. If a line is missing, the default value +# will be the empty string. +# + +if [[ "$@" != "-a" ]]; then + echo "Usage: lsb_release -a" + exit 2 +fi + +# Because the PATH is set to just this directory, we cannot use 'dirname' +# or other external programs, but need to use built-in abilities of bash. +LSB_FILE="${0%/*}/../etc/lsb-release" + +if [[ ! -f $LSB_FILE ]]; then + echo "Error: LSB release file does not exist: $LSB_FILE" + exit 1 +fi + +source $LSB_FILE + +echo "No LSB modules are available." +echo "Distributor ID: ${DISTRIB_ID:-}" +echo "Description: ${DISTRIB_DESCRIPTION:-}" +echo "Release: ${DISTRIB_RELEASE:-}" +echo "Codename: ${DISTRIB_CODENAME:-}" + +exit 0 diff --git a/tests/resources/distros/ubuntu16/etc/debian_version b/tests/resources/distros/ubuntu16/etc/debian_version new file mode 100644 index 0000000..b0b57ed --- /dev/null +++ b/tests/resources/distros/ubuntu16/etc/debian_version @@ -0,0 +1 @@ +stretch/sid diff --git a/tests/resources/distros/ubuntu16/etc/lsb-release b/tests/resources/distros/ubuntu16/etc/lsb-release new file mode 100644 index 0000000..c4b5944 --- /dev/null +++ b/tests/resources/distros/ubuntu16/etc/lsb-release @@ -0,0 +1,4 @@ +DISTRIB_ID=Ubuntu +DISTRIB_RELEASE=16.04 +DISTRIB_CODENAME=xenial +DISTRIB_DESCRIPTION="Ubuntu 16.04.1 LTS" diff --git a/tests/resources/distros/ubuntu16/etc/os-release b/tests/resources/distros/ubuntu16/etc/os-release new file mode 100644 index 0000000..18483dd --- /dev/null +++ b/tests/resources/distros/ubuntu16/etc/os-release @@ -0,0 +1,10 @@ +NAME="Ubuntu" +VERSION="16.04.1 LTS (Xenial Xerus)" +ID=ubuntu +ID_LIKE=debian +PRETTY_NAME="Ubuntu 16.04.1 LTS" +VERSION_ID="16.04" +HOME_URL="http://www.ubuntu.com/" +SUPPORT_URL="http://help.ubuntu.com/" +BUG_REPORT_URL="http://bugs.launchpad.net/ubuntu/" +UBUNTU_CODENAME=xenial diff --git a/tests/test_distro.py b/tests/test_distro.py index f5ac2b2..7fc8bfd 100644 --- a/tests/test_distro.py +++ b/tests/test_distro.py @@ -211,6 +211,21 @@ def test_fedora23_os_release(self): } self._test_outcome(desired_outcome) + def test_fedora30_os_release(self): + # Fedora 21 and above no longer have code names but the metadata in os-release was only + # changed in a detectable way in Fedora 30+. The piece in parenthesis in the pretty_name + # field contains the VARIANT and differs depending on the variant which was installed. + desired_outcome = { + 'id': 'fedora', + 'name': 'Fedora', + 'pretty_name': 'Fedora 30 (Thirty)', + 'version': '30', + 'pretty_version': '30', + 'best_version': '30', + 'codename': '' + } + self._test_outcome(desired_outcome) + def test_kvmibm1_os_release(self): desired_outcome = { 'id': 'kvmibm', @@ -344,6 +359,19 @@ def test_ubuntu14_os_release(self): } self._test_outcome(desired_outcome) + def test_ubuntu16_os_release(self): + desired_outcome = { + 'id': 'ubuntu', + 'name': 'Ubuntu', + 'pretty_name': 'Ubuntu 16.04.1 LTS', + 'version': '16.04', + 'pretty_version': '16.04 (xenial)', + 'best_version': '16.04.1', + 'like': 'debian', + 'codename': 'xenial' + } + self._test_outcome(desired_outcome) + def test_amazon2016_os_release(self): desired_outcome = { 'id': 'amzn', @@ -697,6 +725,19 @@ def test_fedora23_dist_release(self): } self._test_outcome(desired_outcome, 'fedora', '23') + def test_fedora30_dist_release(self): + desired_outcome = { + 'id': 'fedora', + 'name': 'Fedora', + 'pretty_name': 'Fedora 30 (Thirty)', + 'version': '30', + 'pretty_version': '30 (Thirty)', + 'best_version': '30', + 'codename': 'Thirty', + 'major_version': '30' + } + self._test_outcome(desired_outcome, 'fedora', '30') + def test_gentoo_dist_release(self): desired_outcome = { 'id': 'gentoo', @@ -1075,6 +1116,27 @@ def test_fedora23_release(self): } self._test_release_file_info('fedora-release', desired_info) + def test_fedora30_release(self): + desired_outcome = { + 'id': 'fedora', + 'name': 'Fedora', + 'pretty_name': 'Fedora 30 (Thirty)', + 'version': '30', + 'pretty_version': '30', + 'best_version': '30', + 'codename': '', + 'major_version': '30' + } + self._test_outcome(desired_outcome) + + desired_info = { + 'id': 'fedora', + 'name': 'Fedora', + 'version_id': '30', + 'codename': 'Thirty' + } + self._test_release_file_info('fedora-release', desired_info) + def test_kvmibm1_release(self): desired_outcome = { 'id': 'kvmibm', @@ -1362,6 +1424,26 @@ def test_ubuntu14_release(self): # release file: self._test_non_existing_release_file() + def test_ubuntu16_release(self): + desired_outcome = { + 'id': 'ubuntu', + 'name': 'Ubuntu', + 'pretty_name': 'Ubuntu 16.04.1 LTS', + 'version': '16.04', + 'pretty_version': '16.04 (xenial)', + 'best_version': '16.04.1', + 'like': 'debian', + 'codename': 'xenial', + 'major_version': '16', + 'minor_version': '04' + } + self._test_outcome(desired_outcome) + + # Test the info from the searched distro release file + # Does not have one; /etc/debian_version is not considered a distro + # release file: + self._test_non_existing_release_file() + def test_amazon2016_release(self): desired_outcome = { 'id': 'amzn', @@ -1655,12 +1737,12 @@ def _test_none(info): info = _distro.info(pretty=True, best=True) _test_none(info) - def test_linux_disribution(self): + def test_linux_distribution(self): _distro = distro.LinuxDistribution(False, self.ubuntu14_os_release) i = _distro.linux_distribution() assert i == ('Ubuntu', '14.04', 'Trusty Tahr') - def test_linux_disribution_full_false(self): + def test_linux_distribution_full_false(self): _distro = distro.LinuxDistribution(False, self.ubuntu14_os_release) i = _distro.linux_distribution(full_distribution_name=False) assert i == ('ubuntu', '14.04', 'Trusty Tahr')