[Merge] ~andersson123/ubuntu-release-upgrader:fix_distupgradequirks_traceback into ubuntu-release-upgrader:ubuntu/main

[Paride Legovini]

Review: Needs Fixing

Launchpad ate my previous comment, re-writing it.

Diff comments:

> diff --git a/DistUpgrade/DistUpgradeQuirks.py b/DistUpgrade/DistUpgradeQuirks.py
> index 46b72a7..45b965c 100644
> --- a/DistUpgrade/DistUpgradeQuirks.py
> +++ b/DistUpgrade/DistUpgradeQuirks.py
> @@ -1470,6 +1470,8 @@ class DistUpgradeQuirks(object):
>              return
>  
>          try:
> +            if not {"SUDO_UID", "PKEXEC_UID"}.intersection(os.environ):
> +                raise ValueError()

I think we're abusing ValueError here. We have the `except ValueError` to cover the case when e.g. SUDO_UID is set to a bogus value that pwd.getpwuid() can't resolve to a passwd database entry. But this is not the case here: we should explicitly cover the "missing variables" case, without artificially making the code fall into an exception meant to cover another case.

What I suggest is explicitly checking for the presence of variables before the `try`:

if not {'SUDO_UID', 'PKEXEC_UID'}.intersection(os.environ):
    logging.debug(
        'Cannot determine how root privileged were gained, will not change font'
    )
    return

(I used single quotes to match the existing code style.)

>              uid = int(os.getenv('SUDO_UID', os.getenv('PKEXEC_UID')))
>              pwuid = pwd.getpwuid(uid)
>          except ValueError:


-- 
https://code.launchpad.net/~andersson123/ubuntu-release-upgrader/+git/ubuntu-release-upgrader/+merge/453214
Your team Ubuntu Core Development Team is subscribed to branch ubuntu-release-upgrader:ubuntu/main.