zqqw / pakku

Pacman wrapper with AUR support
GNU General Public License v3.0
39 stars 3 forks source link

Initial doas support #24

Closed hochata closed 2 years ago

hochata commented 2 years ago

doas seems almost a drop in replacement for sudo, so automatic support was hardcoded.

Other changes are more in line with supporting an arbitrary command to use for root execution, to use something other than doas or some custom command.

More detailed changes are:

hochata commented 2 years ago

Work in Progress!

I created the PR so everyone is aware I am working on this.

Do not merge yet, please!

hochata commented 2 years ago

From my limited testing it works fine, but I will daily drive doas for a while to make sure. Some other things

Please let me know if something is missing or buggy!

hochata commented 2 years ago

For the configuration option, I put it under the regular options section under the name SudoCommand. For what I can tell, Yay doesn't have a configuration file, just a command line option. Paru has it under the bin section, with the name of Sudo. Only sudo and doas are supported. Same for Pikaur with its PrivilegeEscalationTool, under the misc section.

Would put the option in a new section and restrict its value be useful? Or it is fine as I did it originally?

hochata commented 2 years ago

Would it be useful to also add a command line option to override the sudo command?

zqqw commented 2 years ago

I thought the time consuming part would be learning about doas too - but still haven't got to that. I just had some vague idea of using a pakku.conf config line to set doas instead of sudo. Also I would check what happens on a long build ie when you get asked for your password again later on if it has timed out, hopefully it will work the same. And if you add a command line option remember pakku takes pacman options as well as it's own, so try and make it something that won't collide. Possibly you could even auto-detect what is installed and have a preferred option set in pakku.conf that falls back to the other if it isn't present, which could be kind of neat, although it might be more predictable for the user if they have to specify it for themselves either in a conf file or as an option, as well as simpler to add.

hochata commented 2 years ago

The auto detect was actually the easiest part. But for now, if the option is set in the config file, the auto-detect will not be triggered.

But I think a PreferredSudoCommand might be better. First try to use the preferred command, and if it fails, try to auto detect.

For now the search order is

And thinking a bit more about it, a command line option can be added later if someone requests it. Doas and Sudo are incompatible, so (if installed with Pacman) you cannot have both.

So I don't think there are many use cases (other than debugging and testing) where you would use a command line option over a a configuration file.

zqqw commented 2 years ago

Sounds good, if it will work when installed regardless of what was in use, and if you could manually select su in pakku.conf over sudo or doas then it would address this old issue too: https://github.com/kitsunyan/pakku/issues/45 I agree a command line option is probably unlikely to be used, and if it did fall back to autodetect it would make it easier for users if someone configured pakku.conf then forgot about it and later swapped between one thing and another. Everything doesn't need to be implemented at once though if you don't get time to add all these things right now, whatever you are happy with.

hochata commented 2 years ago

Kinda, the su command would be /usr/bin/su root -c "exec \"$@\"" -- sh instead of just /usr/bin/su. During the search, those arguments are automatically added. But if a user wants to override it, they would have to put the whole command.

Do you think hard-coding the extra commands would be useful? Or is it ok like this?

hochata commented 2 years ago

I think it would be a little bit silly to expect the user to provide all the arguments. So I just copied the already existing su args when the user provides /usr/bin/su.

hochata commented 2 years ago

So in resume, this PR adds

@zqqw, if you don't have anything more to say about it, I think its ready to be merged!

zqqw commented 2 years ago

Sounds great, merge away yourself if you're happy it's ready for release, having written and researched this section you have a better understanding of this than me anyway.