uninstall: use polkit on PermissionError #8846
Conversation
As for an install, the uninstall should be able to use the polkit to gain privileges.
dabrain34
force-pushed
the
dab_uninstall_use_polkit
branch
from
00dc3a3
to
f9ee46e
Compare
| @@ -32,17 +40,26 @@ def do_uninstall(log: str) -> None: | |||
| print('Deleted:', fname) | |||
| successes += 1 | |||
| except Exception as e: | |||
| print(f'Could not delete {fname}: {e}.') | |||
| failures += 1 | |||
| if e.__class__ == PermissionError and shutil.which('pkexec') is not None and 'PKEXEC_UID' not in os.environ: | |||
There was a problem hiding this comment.
IMHO it's cleaner to have multiple except:
try:
...
except PermissionError:
...
except Exception as e:
...
|
We already have #7345 asking for the install logic to stop using polkit/pkexec with the rationale "this is a bad idea and extremely disturbing". Now we want to add more polkit/pkexec logic? |
|
TBF, #7345 is just 1 person asking to stop using pkexec a year ago. Surely if it was serious issue a PR would have been submitted by now. Personally I think it makes sense to use pkexec, there are other CLI that does that like systemctl, so it's not totally surprising behaviour IMHO. |
|
Also we can uninstall only via ninja ATM, so the rational from #3567 still apply to uninstall I think? |
|
It's not just one person, and no patch was submitted because it is a design issue, not a bugfix, so presumably people don't want to go to the effort of writing a patch if it's going to be rejected. The rationale from #3567 explicitly does NOT apply. The install target depends on the "all" target and there was a concern that people would "accidentally touching a file before running sudo ninja install, causing the outputs to become owned by root". This is NOT applicable to |
|
As we are still using the install with polkit, I assume we should be consistent and use polkit also to uninstall . I dont see any blocker or any security issue to not use it. |
As for an install, the uninstall should be able
to use the polkit to gain privileges.