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.

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

Use SUSE_LINUX as OS_VENDOR on all SUSE systems. #1241

Merged

Conversation

jsmeix
Copy link
Member

@jsmeix jsmeix commented Mar 14, 2017

Clean up the SetOSVendorAndVersion function a bit.

In particular set SUSE_LINUX as OS_VENDOR
on all SUSE systems but do not set OS_MASTER_VENDOR
same as OS_VENDOR because then scripts in
a .../$OS_VENDOR/... or .../$OS_MASTER_VENDOR/...
sub-directory would get sourced twice by
the (buggy?) SourceStage function.

This pull request is triggered by
#1171 (comment)
and it is also related to
#1214 (comment)

@jsmeix jsmeix added cleanup enhancement Adaptions and new features labels Mar 14, 2017
@jsmeix jsmeix added this to the Rear v2.1 milestone Mar 14, 2017
@jsmeix jsmeix self-assigned this Mar 14, 2017
@jsmeix
Copy link
Member Author

jsmeix commented Mar 15, 2017

I like to merge it soon because I think it is a good first step
in the right direction and if somthing fails I will fix it.

@gdha
Copy link
Member

gdha commented Mar 15, 2017 via email

…e automatically added comments in /etc/rear/os.conf in the recovery system
@jsmeix jsmeix merged commit 36b776e into rear:master Mar 15, 2017
@jsmeix
Copy link
Member Author

jsmeix commented Mar 15, 2017

Tested on SLES12 SP2 where it works for me.
If accidentally something fails on other systems I will fix it.

@jsmeix jsmeix deleted the use_SUSE_LINUX_as_OS_VENDOR_on_all_SUSE_systems branch March 15, 2017 12:00
@schlomo
Copy link
Member

schlomo commented Apr 19, 2017

Triggered by #1311 I looked again at this and realized what (IMHO) happened here: OS_VENDOR and OS_MASTER_VENDOR should indeed never be the same. Apparently on some SUSE distros this happened, setting both to SUSE_LINUX.

While the current fix in this PR works it actually goes against the original concept of the *_MASTER_* variables. The original concept was to have all vendor-generic code under the *_MASTER_* name and extend this with derivate-specific code only where needed.

Maybe with Fedora/RHEL/CentOS this works better because they are more consistent in the output of lsb_release?

@jsmeix
Copy link
Member Author

jsmeix commented Apr 19, 2017

The code of SetOSVendorAndVersion() somehow
works "backwards" because it first sets OS_VENDOR
and from that value it derives a value that is set for
OS_MASTER_VENDOR for several Linux distributions.

I implemented that behaviour now also for SUSE.
Now OS_MASTER_VENDOR="SUSE" for SUSE distributions.
I think before OS_MASTER_VENDOR was not set for SUSE.

As a second step all SUSE_LINUX directories could now be
renamed into SUSE but I don't like that because it works as is.

It never happened in real usage that OS_VENDOR and
OS_MASTER_VENDOR were same for SUSE.
What happened was that I had set both same during my tests
but then I noticed that scripts get run twice because of this.

When OS_VENDOR and OS_MASTER_VENDOR should
indeed never be the same, a test is missing in ReaR that
errors out when both are the same.
Way too often ReaR does not verify conditions
but blindly proceeds "bona fide".

According to the code of SetOSVendorAndVersion()

    case "$OS_VENDOR_VERSION" in
        (*Oracle*|*CentOS*|*FedoraCore*|*RedHat*|*Scientific*)
            OS_MASTER_VENDOR="Fedora"

I don't think Fedora/RHEL/CentOS are consistent
in the output of lsb_release.
Each one has its own name in the output of lsb_release
(which is expected) but SetOSVendorAndVersion()
maps them to a consistent OS_MASTER_VENDOR.

@schlomo
Copy link
Member

schlomo commented Apr 19, 2017

+1 for adding a test to prevent running with OS_VENDOR and OS_MASTER_VENDOR set to the same.

Fedora-based distros so far (as I have seen) are much more consistent with regard to their lsb_release -i output compared to SUSE, where different SLES versions yield different results.

And yes, I would strongly suggest to actually use OS_MASTER_VENDOR for SUSE and set OS_VENDOR to something that differentiates between the different SUSE flavors.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants