After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 795346 - /etc/os-release parser fails on comments or empty lines
/etc/os-release parser fails on comments or empty lines
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: cerbero
git master
Other Linux
: Normal normal
: 1.15.1
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2018-04-18 07:04 UTC by Peter Marquardt
Modified: 2018-04-29 11:04 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
utils: skip empty lines and comment lines when parsing /etc/os-release (1.51 KB, patch)
2018-04-20 11:55 UTC, Tim-Philipp Müller
committed Details | Review

Description Peter Marquardt 2018-04-18 07:04:39 UTC
command: ./cerbero-uninstalled bootstrap


If there is an empty line in /etc/os-release cerbero fails with

Traceback (most recent call last):
  • File "./cerbero-uninstalled", line 8 in <module>
    from cerbero.main import main
  • File "./cerbero/main.py", line 19 in <module>
    from cerbero import hacks
  • File "./cerbero/hacks.py", line 95 in <module>
    from cerbero.utils.shell import call as shell_call
  • File "./cerbero/utils/shell.py", line 45 in <module>
    PLATFORM = system_info()[0]
  • File "./cerbero/utils/__init__.py", line 169 in system_info
    k,v = line.rstrip().split("=")
ValueError: not enough values to unpack (expected 2, got 1)

Comments introduced by '#' are parsed, so a missing '=' in the comment will also trigger

Traceback (most recent call last):
  • File "./cerbero-uninstalled", line 8 in <module>
    from cerbero.main import main
  • File "./cerbero/main.py", line 19 in <module>
    from cerbero import hacks
  • File "./cerbero/hacks.py", line 95 in <module>
    from cerbero.utils.shell import call as shell_call
  • File "./cerbero/utils/shell.py", line 45 in <module>
    PLATFORM = system_info()[0]
  • File "./cerbero/utils/__init__.py", line 169 in system_info
    k,v = line.rstrip().split("=")
ValueError: not enough values to unpack (expected 2, got 1)

whereas a commented '=' won't.
Comment 1 Tim-Philipp Müller 2018-04-18 08:01:18 UTC
Thanks for the bug report. Are you going to make a patch for this? :)
Comment 2 Peter Marquardt 2018-04-18 08:20:01 UTC
(In reply to Tim-Philipp Müller from comment #1)
> Thanks for the bug report. Are you going to make a patch for this? :)

I'm on page 3 of "python for dummies". Pages 1-2 were about finding bugs in build systems 8-) Sorry, no patch so far. I'll continue reading 8-)
Comment 3 Tim-Philipp Müller 2018-04-18 08:41:23 UTC
Maybe something like


  try:
      ...split..
  except:
      continue

? (Not really a python person myself either, there might be a more idiomatic solution)

Also seems like it doesn't handle there not being a VERSION_ID gracefully
Comment 4 Olivier Crête 2018-04-18 13:03:34 UTC
Which distro has comments in its /etc/os-release ? Can you attach the file here so whoever writes a patch can test against it?
Comment 5 Olivier Crête 2018-04-18 13:04:40 UTC
And indeed comments are valid in there, ref: https://www.freedesktop.org/software/systemd/man/os-release.html
Comment 6 Peter Marquardt 2018-04-18 15:01:16 UTC
(In reply to Olivier Crête from comment #4)
> Which distro has comments in its /etc/os-release ? Can you attach the file
> here so whoever writes a patch can test against it?

Hmmm... as you said, comments are valid. To be honest, empty lines are not mentioned. So we cannot really support them. The parser should stop immediately when it reads an empty line. Maybe tell the user to contact his distro to fix it. maybe force a system upgrade in background to see if this has been fixed already.

Anyways, I don't think we want to discuss the format of /etc/os-release of distro xyz or force a patch writer to install distro xyz for testing purposes, would we ?

sudo echo "#" > /etc/os-release

... would be the first thing he should try for testing 8-) "creating your own distro for dummies - page 12"

open I,'<','/etc/os-release' or die;
while(<I>){
  /^\s*#.*//;
  /^\s*$/ and next;
  /(\S+)\s*=\s*(.*)\s*/ and $V{$1}=$2;  
}
close(I);

untested code in an 80s language ("writing pseudo code in ancient languages - national geograhics, book 314, page 1592") 

or maybe, for testing purposes, create a local copy of /etc/os-release via "cp /etc/os-release os-release" and change the code by deleting the "/etc" part of the filename. There might be a python module for "cp".

sorry, I don't want to troll, but hey: this program parses a text file of a couple of lines, and we are discussing "valid formats" ? give me a break.
Comment 7 Tim-Philipp Müller 2018-04-20 11:55:07 UTC
Created attachment 371154 [details] [review]
utils: skip empty lines and comment lines when parsing /etc/os-release
Comment 8 Paul Menzel 2018-04-20 12:16:20 UTC
I created issue *Specify if blank lines are allowed in `/etc/os-release` #8773* for systemd [1] asking for a clarification.

[1]: https://github.com/systemd/systemd/issues/8773
Comment 9 Tim-Philipp Müller 2018-04-20 14:45:05 UTC
Ok, but it's trivial to skip empty lines so I don't see why we don't just implement that and get on with our lives?

Does the patch fix the issue on your machine?
Comment 10 Tim-Philipp Müller 2018-04-29 11:04:18 UTC
Please re-open if it's still a problem, thanks.

commit f7ebcf3371dc56b04ec33e453e94907a2dedb384
Author: Tim-Philipp Müller <tim@centricular.com>
Date:   Fri Apr 20 12:53:36 2018 +0100

    utils: skip empty lines and comment lines when parsing /etc/os-release
    
    https://bugzilla.gnome.org/show_bug.cgi?id=795346