-
Notifications
You must be signed in to change notification settings - Fork 24
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.
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
Fixes #4360: correct detection of RedHat is LSB is not present #206
Fixes #4360: correct detection of RedHat is LSB is not present #206
Conversation
+ if ($name =~ /RedHat/) { | ||
+ if ($release =~ /Scientific/) { | ||
+ # this is really a scientific linux, we have to change the name | ||
+ $name = "Scientific"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I may have misunderstood because I find this part of the code rather confusing, but I think this patch is the wrong way round...
As I understand it, on a Red Hat system, FusionInventory finds the file /etc/system-release, so sets $name = "Scientific". Then, it reads the contents of /etc/system-release and sets $release = "Red Hat enterprise service release 6.4 (Final)" (or something similar). This is wrong, because name should be "RedHat", not "Scientific".
So I think your patch above would work, like this:
if ($name =~ /Scientific/) {
if ($release =~ /Red Hat/) {
# this is really a scientific linux, we have to change the name
$name = "RedHat"
Do you see what I mean?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
By the way, this is the case because FusionInventory only looks at the first file that exists from it's list, and /etc/system-release is listed before /etc/redhat-release. And also because our patch for Scientific says that /etc/system-release means Scientific.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, I removed the previous patch, which doesn't offer anything (content of /etc/system-release and /etc/redhat-release are identical on Scientific Linux, and RedHat). So we only detect Redhat, and if it is a RedHat, then we look for the content of the file. If it contain Scientific, it is a Scientifc linux.
This is more compliant with the way the system is build (it looks like a RedHat, but when we drill down the files, it is indeed a Scientific Linux)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh OK! That does make sense, then :)
But could you include the file removal for Scientific_linux_distro_name.patch in this PR, in that case? Otherwise, it will hang around forever...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok !
PR updated with the removal of unused patch |
…of_redhat Fixes #4360: correct detection of RedHat is LSB is not present
No description provided.