Skip to content
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

Jump to bottom

platform.linux_distribution() should honor /etc/os-release #61962

Closed
doko42 opened this issue Apr 16, 2013 · 17 comments
Closed

platform.linux_distribution() should honor /etc/os-release #61962

doko42 opened this issue Apr 16, 2013 · 17 comments
Assignees
@malemburg
Labels
easy stdlib Python modules in the Lib dir type-feature A feature request or enhancement

Comments

@doko42
Copy link
Member

doko42 commented Apr 16, 2013

BPO 17762
Nosy @malemburg, @doko42, @vstinner, @tiran, @jwilk, @ezio-melotti, @merwok, @encukou, @vajrasky
Files
  • add_os_release_support.patch
  • add_os_release_support_2.patch
    		•

    Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = 'https://github.com/malemburg'
closed_at = <Date 2015-05-13.14:19:56.561>
created_at = <Date 2013-04-16.15:01:08.619>
labels = ['easy', 'type-feature', 'library']
title = 'platform.linux_distribution() should honor /etc/os-release'
updated_at = <Date 2015-05-13.14:21:06.545>
user = 'https://github.com/doko42'

    bugs.python.org fields:

    activity = <Date 2015-05-13.14:21:06.545>
actor = 'lemburg'
assignee = 'lemburg'
closed = True
closed_date = <Date 2015-05-13.14:19:56.561>
closer = 'lemburg'
components = ['Library (Lib)']
creation = <Date 2013-04-16.15:01:08.619>
creator = 'doko'
dependencies = []
files = ['32330', '32467']
hgrepos = []
issue_num = 17762
keywords = ['patch', 'easy']
message_count = 17.0
messages = ['187089', '187389', '201013', '201023', '201144', '201152', '201154', '201518', '201543', '201603', '201987', '202394', '202395', '205773', '224548', '243088', '243092']
nosy_count = 12.0
nosy_names = ['lemburg', 'doko', 'vstinner', 'christian.heimes', 'jwilk', 'ezio.melotti', 'eric.araujo', 'Arfrever', 'petr.viktorin', 'vajrasky', 'elixir', 'jaywink']
pr_nums = []
priority = 'normal'
resolution = 'wont fix'
stage = 'needs patch'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue17762'
versions = ['Python 3.4']
    @doko42
    Copy link
    Member Author

    doko42 commented Apr 16, 2013

    http://www.freedesktop.org/software/systemd/man/os-release.html
    is a recent standard describing release information for an operating system. platform.linux_distribution() should know about it.

    • should that be the first file to be parsed?

    • names returned for the ID are different than the ones
      returned from /etc/lsb-release. The os-release thing
      only allows lower case letters for the ID (e.g. Ubuntu
      vs. ubuntu). The list of _supported_distros may
      need to list both.

    • take the version from VERSION_ID

    • there is no real attribute for the code name. The closest
      maybe is VERSION.

    @doko42 doko42 added the stdlib Python modules in the Lib dir label Apr 16, 2013
    @ezio-melotti ezio-melotti added the type-feature A feature request or enhancement label Apr 16, 2013
    @merwok
    Copy link
    Member

    merwok commented Apr 19, 2013

    os-release finally provides a cross-OS release file with a specification. I think it should be authoritative, then the lsb-release system (it’s officially a script but many OSes just parse a static file), then OS-specific files.

    @tiran
    Copy link
    Member

    tiran commented Oct 23, 2013

    *bump up*

    I'd like to see the feature in 3.4. It shouldn't be too hard to implement it. A patch would also solve bpo-1322 and bpo-9514 on most modern systems.

    For the record RHEL 5, RHEL 6.4, SLES 11 and Ubuntu 10.04 don't have /etc/os-release. Ubuntu 12.04 has the file.

    @tiran tiran added the easy label Oct 23, 2013
    @elixir
    Copy link
    Mannequin

    elixir mannequin commented Oct 23, 2013

    I'm working on a patch. Hopefully, it will be ready in a day or two.

    @elixir
    Copy link
    Mannequin

    elixir mannequin commented Oct 24, 2013

    I added a patch (my first patch!).

    platform.linux_distribution() now first looks in /etc/os-release. If this file is not found, checking continues as before.

    @vajrasky
    Copy link
    Mannequin

    vajrasky mannequin commented Oct 24, 2013

    Hi Andrei Duma,

    I have looked at your patch but have not tested it yet. But it seems to me that your patch is a little bit weak against the case where the file /etc/os-release is found, but not fully functional (either garbage, or only releases NAME information but not VERSION).

    But again, I am not so sure we should really bit pedantic about this or not. Need to do some investigation.

    @malemburg
    Copy link
    Member

    On 24.10.2013 16:59, Andrei Dorian Duma wrote:

    I added a patch (my first patch!).

    platform.linux_distribution() now first looks in /etc/os-release. If this file is not found, checking continues as before.

    Looks good.

    @vajrasky
    Copy link
    Mannequin

    vajrasky mannequin commented Oct 28, 2013

    Apparently my fear is unfounded. The dist, version, id have been initialized with empty value. So if the os-release file does not have complete information, it should be okay with the patch from Andrei Duma.

    @vajrasky
    Copy link
    Mannequin

    vajrasky mannequin commented Oct 28, 2013

    Hi, Andrei.

    Could you provide the test? You could take a look at bpo-17429 to see how it is done.

    http://bugs.python.org/issue17429

    We would be grateful if you could test the case where os-release file is non-ascii encoded file (although technically it should be tested in bpo-17429). Sometimes the Linux distro version has non-ascii characters, such as Schrödinger's Cat.

    @elixir
    Copy link
    Mannequin

    elixir mannequin commented Oct 29, 2013

    Yes, I will provide a patch including tests soon.

    @elixir
    Copy link
    Mannequin

    elixir mannequin commented Nov 2, 2013

    New patch. Added tests and fixed uncaught OSError.

    @vstinner
    Copy link
    Member

    vstinner commented Nov 7, 2013

    Comments on add_os_release_support_2.patch:

    • You should not write a huge try/except OSError block. I would prefer something like:

    try:
    f = open(...)
    except OSError:
    return None
    with f:
    ...

    • I'm not sure about that, but you might use errors='surrogateescape', just in case if the file is not correctly encoded. Surrogate characters are maybe less annoying than a huge UnicodeDecodeError

    • You use /etc/os-release even if the file is empty. Do you know if there is a standard, or something like that explaining if some keys are mandatory? For example, we can ignore os-release if 'ID' or 'VERSION_ID' key is missing

    • For the unit test, you should write at least a test on linux_distribution() to check that your private function is used correctly

    • You add unit tests for all escaped characters (', ", \), for comments, and maybe also for maformated lines (to have a well defined behaviour, ignore them or raise an error)

    • _UNIXCONFDIR looks to be dedicated to unit tests, can't you patch builtin open() instead? It would avoid the need of a temporary directory

    @doko42
    Copy link
    Member Author

    doko42 commented Nov 7, 2013

    my concern here is that platform.linux_distribution returns different values for the tuple, wether os-release is found or the lsb config file is found. I don't know about a good solution, however if the return value has different values, that has to be clearly documented, or maybe unified in some form.

    @vajrasky
    Copy link
    Mannequin

    vajrasky mannequin commented Dec 10, 2013

    This patch needs to be updated to tip since this commit: http://hg.python.org/cpython/rev/4580976c07cb

    @jaywink
    Copy link
    Mannequin

    jaywink mannequin commented Aug 2, 2014

    platform.linux_distribution is being deprecated in 3.5 and removed in 3.6 as stated in comment http://bugs.python.org/issue1322#msg207427 in issue bpo-1322

    I'm guessing this issue should be closed when that patch is merged in?

    @encukou
    Copy link
    Member

    encukou commented May 13, 2015

    The functions have been deprecated in bpo-1322, is it time to close this?

    @malemburg
    Copy link
    Member

    See bpo-1322 for why we're closing this.

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Assignees

    @malemburg malemburg

    Labels
    easy stdlib Python modules in the Lib dir type-feature A feature request or enhancement
    Projects
    None yet
    Milestone
    No milestone
    Development

    No branches or pull requests
    7 participants
    @vstinner @encukou @tiran @merwok @malemburg @doko42 @ezio-melotti