8000 bpo-28468: Add platform.freedesktop_osrelease by tiran · Pull Request #23492 · python/cpython · GitHub
[go: up one dir, main page]
Skip to content

bpo-28468: Add platform.freedesktop_osrelease #23492

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Nov 30, 2020
Merged

bpo-28468: Add platform.freedesktop_osrelease #23492

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
8a99778
bpo-28468: Add platform.freedesktop_osrelease
tiran Nov 24, 2020
ceb9f5d
Address review
tiran Nov 24, 2020
8c0550e
Update Doc/whatsnew/3.10.rst
tiran Nov 24, 2020
0fc7d03
Only cache on success
tiran Nov 24, 2020
0ed04ea
Unquote 5 backslash quoted characters
tiran Nov 24, 2020
679af65
Victor prefers to keep ID_LIKE untouched
tiran Nov 25, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
bpo-28468: Add platform.freedesktop_osrelease 
Signed-off-by: Christian Heimes <christian@python.org>
  • Loading branch information
@tiran
tiran committed Nov 24, 2020
commit 8a997785c976bd1a2896b848087339d7a03c4286
23 changes: 23 additions & 0 deletions Doc/library/platform.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -253,3 +253,26 @@ Unix Platforms
using :program:`gcc`.

The file is read and scanned in chunks of *chunksize* bytes.


Linux Platforms
---------------

.. function:: freedesktop_osrelease()

Get operating system identification from ``os-release`` file and return
it as a dict. The ``os-release`` file is a `freedesktop.org standard
<https://www.freedesktop.org/software/systemd/man/os-release.html>`_ and
supported by majority of Linux distributions. All fields except ``NAME``
and ``ID`` are optional. If present, the ``ID_LIKE`` parsed and presented
as a tuple of strings.

Note that fields like ``NAME``, ``VERSION``, and ``VARIANT`` are strings
suitable for presentation to users. Programs should use fields like
``ID`` + ``ID_LIKE``, ``VERSION_ID``, or ``VARIANT_ID`` to identify
Linux distributions. Vendors may include additional fields.

Raises :exc:`OSError` when neither ``/etc/os-release`` nor
``/usr/lib/os-release`` can be read.

.. versionadded:: 3.10
8 changes: 8 additions & 0 deletions Doc/whatsnew/3.10.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -254,6 +254,14 @@ Added negative indexing support to :attr:`PurePath.parents
<pathlib.PurePath.parents>`.
(Contributed by Yaroslav Pankovych in :issue:`21041`)

platform
--------

Added :func:`platform.freedesktop_osrelease()` to retrieve operation system
identification from `freedesktop.org os-release
<https://www.freedesktop.org/software/systemd/man/os-release.html>`_ standard.
(Contributed by Christian Heimes in :issue:`28468`)

py_compile
----------

Expand Down
53 changes: 53 additions & 0 deletions Lib/platform.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1230,6 +1230,59 @@ def platform(aliased=0, terse=0):
_platform_cache[(aliased, terse)] = platform
return platform

### freedesktop.org os-release standard
# https://www.freedesktop.org/software/systemd/man/os-release.html

# NAME=value with optional quotes (' or ")
_osrelease_line = re.compile(
"^(?P<name>[a-zA-Z0-9_]+)=(?P<quote>[\"\']?)(?P<value>.*)(?P=quote)$"
)
# /etc takes precedence over /usr/lib
_osrelease_candidates = ("/etc/os-release", "/usr/lib/os-release", )
_osrelease_cache = None


def _parse_osrelease(lines):
# NAME and ID fields are mandatory fields with well-known defaults
# in pratice all Linux distributions override NAME and ID.
info = {
"NAME": "Linux",
"ID": "linux"
}
for line in lines:
mo = _osrelease_line.match(line)
if mo is not None:
info[mo.group('name')] = mo.group('value')

# ID_LIKE is a space separated field of ids
if 'ID_LIKE' in info:
info['ID_LIKE'] = tuple(
id_like for id_like in info['ID_LIKE'].split(' ') if id_like
)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest to remove this code.

I'm not sure if it's worth it to have a special case only for the ID_LIKE variable. The platform module is mostly based on strings. You introduce a special case where a single variable is a tuple. Since it's well specified that it's space separated, the caller can split the string by themselves, no?

Splitting at spaces it trivial in Python:

>>> "debian ubuntu".split()
['debian', 'ubuntu']

Copy link
Member Author
@tiran tiran Nov 24, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the only special case that a lot of users will get wrong and that won't get detected by CI. For almost all distros the ID_LIKE is either not present or it is a single entry, often debian or fedora. For example Ubuntu has ID_LIKE=debian. Only very few distros have an ID_LIKE with multiple entries.

Users always have to split the field and always check for "name in info["ID_LIKE"].split()". The explicit handling of ID_LIKE get rids of a foot gun.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem is that if tomorrow a new field is added with a value separated by spaces, the caller might expect Python to magically split it as a tuple. But now, we will have to update the function whenever a new field is added for add a special case.

If there is no special case and it's a dummy parser, the code is more likely to stay for the same for the next 10 years.

I would prefer to not reintroduce the same problem that we fixed by removing linux_distribution(): having to update the stdlib when the filename changes or file content evolves.

If you want a more advanced view of the os-release, I suggest to write a higher level API on top of it and put it on PyPI, so it can be easily updated (similar to the distro module which contains many special cases).

return info


def freedesktop_osrelease():
"""Return operation system identification from freedesktop.org os-release
"""
global _osrelease_cache

if _osrelease_cache is None:
for candidate in _osrelease_candidates:
try:
with open(candidate) as f:
_osrelease_cache = _parse_osrelease(f)
break
except OSError as e:
e.__traceback__ = None
_osrelease_cache = e

if isinstance(_osrelease_cache, Exception):
raise _osrelease_cache
else:
return _osrelease_cache.copy()


### Command line interface

if __name__ == '__main__':
Expand Down
93 changes: 93 additions & 0 deletions Lib/test/test_platform.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,63 @@
from test import support
from test.support import os_helper

FEDORA_OSRELEASE = """\
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The trailing \ is surprising. Is it needed? Maybe write """...""".lstrip()?

NAME=Fedora
VERSION="32 (Thirty Two)"
ID=fedora
VERSION_ID=32
VERSION_CODENAME=""
PLATFORM_ID="platform:f32"
PRETTY_NAME="Fedora 32 (Thirty Two)"
ANSI_COLOR="0;34"
LOGO=fedora-logo-icon
CPE_NAME="cpe:/o:fedoraproject:fedora:32"
HOME_URL="https://fedoraproject.org/"
DOCUMENTATION_URL="https://docs.fedoraproject.org/en-US/fedora/f32/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=32
REDHAT_SUPPORT_PRODUCT="Fedora"
REDHAT_SUPPORT_PRODUCT_VERSION=32
PRIVACY_POLICY_URL="https://fedoraproject.org/wiki/Legal:PrivacyPolicy"
"""

UBUNTU_OSRELEASE = """\
NAME="Ubuntu"
VERSION="20.04.1 LTS (Focal Fossa)"
ID=ubuntu
ID_LIKE=debian
PRETTY_NAME="Ubuntu 20.04.1 LTS"
VERSION_ID="20.04"
HOME_URL="https://www.ubuntu.com/"
SUPPORT_URL="https://help.ubuntu.com/"
BUG_REPORT_URL="https://bugs.launchpad.net/ubuntu/"
PRIVACY_POLICY_URL="https://www.ubuntu.com/legal/terms-and-policies/privacy-policy"
VERSION_CODENAME=focal
UBUNTU_CODENAME=focal
"""

TEST_OSRELEASE = r"""
# test data
ID_LIKE=egg spam viking
EMPTY=
# comments and empty lines are ignored

SINGLE_QUOTE='single'
EMPTY_SINGLE=''
DOUBLE_QUOTE="double"
EMPTY_DOUBLE=""
QUOTES="double's"
"""


class PlatformTest(unittest.TestCase):
def clear_caches(self):
platform._platform_cache.clear()
platform._sys_version_cache.clear()
platform._uname_cache = None
platform._osrelease_cache = None

def test_architecture(self):
res = platform.architecture()
Expand Down Expand Up @@ -382,6 +433,48 @@ def test_macos(self):
self.assertEqual(platform.platform(terse=1), expected_terse)
self.assertEqual(platform.platform(), expected)

def test_freedesktop_osrelease(self):
self.addCleanup(self.clear_caches)
self.clear_caches()

if any(os.path.isfile(fn) for fn in platform._osrelease_candidates):
info = platform.freedesktop_osrelease()
self.assertIn("NAME", info)
self.assertIn("ID", info)

info["CPYTHON_TEST"] = "test"
self.assertNotIn("CPYTHON_TEST", platform.freedesktop_osrelease())
else:
with self.assertRaises(OSError):
platform.freedesktop_osrelease()

def test_parse_osrelease(self):
info = platform._parse_osrelease(FEDORA_OSRELEASE.split("\n"))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remark: For the purpose of the unit test, .splitlines() would also work, and it's less strict, no?

self.assertEqual(info["NAME"], "Fedora")
self.assertEqual(info["ID"], "fedora")
self.assertNotIn("ID_LIKE", info)
self.assertEqual(info["VERSION_CODENAME"], "")

info = platform._parse_osrelease(UBUNTU_OSRELEASE.split("\n"))
self.assertEqual(info["NAME"], "Ubuntu")
self.assertEqual(info["ID"], "ubuntu")
self.assertEqual(info["ID_LIKE"], ("debian",))
self.assertEqual(info["VERSION_CODENAME"], "focal")

info = platform._parse_osrelease(TEST_OSRELEASE.split("\n"))
expected = {
"ID": "linux",
"NAME": "Linux",
"ID_LIKE": ("egg", "spam", "viking"),
"EMPTY": "",
"DOUBLE_QUOTE": "double",
"EMPTY_DOUBLE": "",
"SINGLE_QUOTE": "single",
"EMPTY_SINGLE": "",
"QUOTES": "double's",
}
self.assertEqual(info, expected)


if __name__ == '__main__':
unittest.main()
2 changes: 2 additions & 0 deletions Misc/NEWS.d/next/Library/2020-11-24-13-18-05.bpo-28468.8Gh2d4.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Add :func:`platform.freedesktop_osrelease` function to parse freedesktop.org
``os-release`` files.
0