tliron / install-gnome-themes

Script to install the latest versions of some fine GNOME 3 themes
MIT License
436 stars 44 forks source link

Use a more distro neutral way to find distro #3

Closed SpacePotatoBear closed 7 years ago

SpacePotatoBear commented 8 years ago

The current way of setting OS is limited to just Ubuntu, I'm going to add Fedora support, would you rather make a more neutral way of finding distro version? (cat /proc/version or uname -a? and use regex on it?) or should it be done via a separate code branch? (if statement)

tliron commented 8 years ago

I like "if" statements, they are coherent and easy to understand and debug. As for method, there must be a nice standard way to check for distro, no?

SpacePotatoBear commented 8 years ago

hmm, most places recommend cat /proc/version, problem is their output isn't a simple "ubuntu" or "fedora" so some regex will be needed (I can do this no problem, just want your input before I do)

as for making a branch using if statements, how should this be done? create a separate set of methods (Installing deps) for each distro, or modifying current methods to check OS and then branch from there?

Lastly since I have your attention, what about adding support for other DE's that use gnome themes? (I.E unity) and adding more themes (evopop?)

(turns out uname wont do the trick, wont give worthwhile info)

tliron commented 8 years ago

My understanding is that uname is quite standard indeed. Well, let's start with supporting Fedora. Of course I'd be happy to add more themes! More DEs shouldn't be a problem, either, though might be complicated with testing GTK versions. Anyway, let's start simple and keep expanding...

SpacePotatoBear commented 8 years ago

I'm at work so I can't do this through github, but I think I have something that works for OS versions (Feodra 24 at least)

name=$(awk '{print $1}' /etc/*-release) if [[ $name =~ .*NAME=([a-Z]{1,}).* ]] ; then OS="${BASH_REMATCH[1]}" fi echo $name

if this works on ubuntu, we should use this instead of lsb-release

tliron commented 8 years ago

It spits out "xenial" for me. What does it spit out for Fedora?

SpacePotatoBear commented 8 years ago

spits out Fedora,

what does echo $(awk '{print $1}' /etc/*-release) output on ubuntu? it should be a very long string of version numbers, I'll just need to adjust the regex. When I get home I'll boot up some vm's to see what distros output what.

Fedora gives me this

Fedora NAME=Fedora VERSION="24 ID=fedora VERSION_ID=24 PRETTY_NAME="Fedora ANSI_COLOR="0;34" CPE_NAME="cpe:/o:fedoraproject:fedora:24" HOME_URL="https://fedoraproject.org/" BUG_REPORT_URL="https://bugzilla.redhat.com/" REDHAT_BUGZILLA_PRODUCT="Fedora" REDHAT_BUGZILLA_PRODUCT_VERSION=24 REDHAT_SUPPORT_PRODUCT="Fedora" REDHAT_SUPPORT_PRODUCT_VERSION=24 PRIVACY_POLICY_URL=https://fedoraproject.org/wiki/Legal:PrivacyPolicy Fedora Fedora

If we can find a common field, I can just get the regex to match to that

tliron commented 8 years ago

DISTRIB_ID=Ubuntu DISTRIB_RELEASE=16.04 DISTRIB_CODENAME=xenial DISTRIB_DESCRIPTION="Ubuntu NAME="Ubuntu" VERSION="16.04 ID=ubuntu ID_LIKE=debian PRETTY_NAME="Ubuntu VERSION_ID="16.04" HOME_URL="http://www.ubuntu.com/" SUPPORT_URL="http://help.ubuntu.com/" BUG_REPORT_URL="http://bugs.launchpad.net/ubuntu/" UBUNTU_CODENAME=xenial

But let's not reinvent the wheel here! I'm sure there is a standard way to get this info. I'm surprised uname isn't working correctly in Fedora. Maybe we just need to change the arguments we send to uname?

SpacePotatoBear commented 8 years ago

I check all args for uname, not one pumps out Fedora or anything related to Fedora/redhat.

I think I know why my regex didn't work, it matched CODENAME= I'll go make it more strict and report back

Edit: lets try this .*PRETTY_NAME=\"([a-Z]{1,}).*

since it seems both ubuntu and fedora have that in there.

tliron commented 8 years ago

Added EvoPop, thanks for the tip.

dominichayesferen commented 7 years ago

@tliron Can you add support for Linux Mint and feren OS? As much as they don't use GNOME, you can still get GNOME in them...

tliron commented 7 years ago

I would imagine any Ubuntu-based OS would automatically work. Have you tried?

dominichayesferen commented 7 years ago

@tliron Yep, it just said it isn't an allowed OS (The Bit where it doesn't identify Ubuntu or any supported OSs)

tliron commented 7 years ago

Ah! Could you possibly test it for me on those and tell me how those OSes identify themselves? I would then add them to the list.

dominichayesferen commented 7 years ago

Well, feren OS uses a file known as /etc/linuxmint/info (Also in Linux Mint), and it contains this:

RELEASE=2017.0 CODENAME=murdock EDITION="64-bit" DESCRIPTION="feren OS 2017.0 Murdock" DESKTOP=Gnome (Don't worry about this line, it says that even on the Linux Mint Cinnamon Version) TOOLKIT=GTK NEW_FEATURES_URL=http://ferenos.weebly.com/blog/new-features-in-feren-os-20170-murdock RELEASE_NOTES_URL=http://ferenos.weebly.com/blog/new-features-in-feren-os-20170-murdock GRUB_TITLE=feren OS 2017.0 Murdock

tliron commented 7 years ago

Can you tell me the output of lsb_release on feren OS?

dominichayesferen commented 7 years ago

lsb_release -a returns (No arguments just complains that there's no LSB Modules available):

Distributor ID: LinuxMint
Description: feren OS 2017.0 Murdock
Release: 18.1
Codename: serena
jrock2004 commented 7 years ago

@tliron what about cat /etc/issue? In Antergos which is arch I get the following:

⇨ cat /etc/issue                                                               
Antergos Linux \r (\l)
dominichayesferen commented 7 years ago

/etc/issue in Linux Mint / feren OS: Linux Mint 18.1 Serena \n \l

tliron commented 7 years ago

I just added support for Mint. Could you please test?

KrumpetPirate commented 7 years ago

@jrock2004 I think we're going to have to do a separate check for Arch based distributions. I also use Antergos so the script won't work here.

KrumpetPirate commented 7 years ago

I propose the following for the OS check: OS=$(head -1 /etc/issue | cut --delimiter=' ' --fields=1)

Output on Ubuntu is 'Ubuntu' Output on Antegros is 'Antergos'

tliron commented 7 years ago

We can do that, but first: does arch not support lsb_release? Seems like standard for any Linux.

KrumpetPirate commented 7 years ago

It does not, or at least my distro does not have lsb_release.

tliron commented 7 years ago

Thanks everyone for the patience and long discussion ... I just assumed that all the LSB stuff would always be found in any Linux.