Re: [libvirt PATCH 2/5] meson: Check for os-release's ID_LIKE in addition to ID

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Jan 26, 2022 at 04:13:13PM +0100, Andrea Bolognani wrote:
> This makes is possible to reduce the number of cases we have to

s/is/it/

> consider, because 'sles' declares itself to be like 'suse' and
> both 'rhel' and 'centos' declare themselves to be like 'fedora'.
> 
> We have to move the check for Ubuntu before the one for Debian,
> however, because 'ubuntu' declares itself to be like 'debian'
> and it would end up with the wrong defaults otherwise.
> 
> Suggested-by: Olaf Hering <olaf@xxxxxxxxx>
> Signed-off-by: Andrea Bolognani <abologna@xxxxxxxxxx>
> ---
>  meson.build | 13 +++++--------
>  1 file changed, 5 insertions(+), 8 deletions(-)
> 
> diff --git a/meson.build b/meson.build
> index ab5f7433a6..388e2cfa5e 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -1659,24 +1659,21 @@ if not get_option('driver_qemu').disabled()
>        default_qemu_user = 'root'
>        default_qemu_group = 'wheel'
>      else
> -      os_release = run_command('grep', '^ID=', '/etc/os-release', check: false).stdout()
> +      os_release = run_command('grep', '-E', '^ID(_LIKE)*=', '/etc/os-release', check: false).stdout()
>        if os_release.contains('arch')
>          default_qemu_user = 'nobody'
>          default_qemu_group = 'nobody'
> -      elif (os_release.contains('centos') or
> -            os_release.contains('fedora') or
> +      elif (os_release.contains('fedora') or
>              os_release.contains('gentoo') or
> -            os_release.contains('rhel') or
> -            os_release.contains('sles') or
>              os_release.contains('suse'))

I would add comments here, for example:

# Check for Fedora like OSes which includes CentOS, RHEL and similar.

And the same for SUSE and SLES.

>          default_qemu_user = 'qemu'
>          default_qemu_group = 'qemu'
> -      elif os_release.contains('debian')
> -        default_qemu_user = 'libvirt-qemu'
> -        default_qemu_group = 'libvirt-qemu'
>        elif os_release.contains('ubuntu')
>          default_qemu_user = 'libvirt-qemu'
>          default_qemu_group = 'kvm'
> +      elif os_release.contains('debian')
> +        default_qemu_user = 'libvirt-qemu'
> +        default_qemu_group = 'libvirt-qemu'

This should be commented as well as we know that not everyone checks the
commit message so it's better to have it tied to the code itself.

>        else
>          default_qemu_user = 'root'
>          default_qemu_group = 'root'

In general I like this idea and at the same time I don't like it at all
:D. It will catch more OSes based on fedora/ubuntu/suse but at the same
time it might cause issues in the future if something changes.

Let's see what others think about it.

Pavel

Attachment: signature.asc
Description: PGP signature


[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux