On Thu, 2011-07-28 at 10:14 +0200, Jiri Moskovcak wrote:
This patch reinstates some functions we removed from python bindings
-
as it turned out, we don't have /etc/*release when anaconda is running
so we need to obtain "os_release" from anaconda and we should avoid
reading /etc/*-release, because it shows an error message on tty1:
Can't open /etc/system-release: No such file or directory
which from users POV can be considered as a bug. So this patch tries to
get the os_release from anaconda then from /etc and the function
dd_create_basic_files() now skips creating os_release if it already
exists (this check shouldn't generate any error msg if the file doesn't
exist). It also modifies create_dump_dir_from_problem_data() moving the
dd_create_basic_files() to the end when all fields from problem_data are
saved in dump_dir, so the mentioned check for "os_release" can happen.
Please review carefully, I can help with anaconda images if someone
would like to test it (I did of course ;)).
- ####version = getVersion_fromRPM()
- ####if version:
- #### return version
+ version = getVersion_fromRPM()
+ if version:
+ return version
Anaconda does not need this, right? Therefore
I propose to leave getVersion_fromRPM() commented out.
(Same for getProduct_fromRPM()).
My point, let's have only minimal compat code we need:
getVersion_fromPRODUCT() and getProduct_fromPRODUCT()
- try:
+ if os.path.exists(each_path):
file = open(each_path, "r")
content = file.read()
if content.startswith("Red Hat Enterprise Linux"):
return "Red Hat Enterprise Linux"
+
if content.startswith("Fedora"):
return "Fedora"
+
i = content.find(" release")
if i > -1:
return content[0:i]
- except:
- pass
It is conceptually wrong to do
if (file_exists(filename)) { f = open(filename); ... }
- it's racy. What if open will nevertheless fail (say, EACCES)?
That's right: you'll get an exception.
The correct way (IMO) is
f = open(filename);
if (f) { ... }
In this particular case it's very unlikely (/etc/system-release
is world-readable), but as a matter of principle, it is wrong.
The rest of the patch look good to me.
--
vda