Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

install-multi-user: fix when sudo is alias to doas #8606

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

concatime
Copy link

@concatime concatime commented Jun 29, 2023

Motivation

In my system, sudo is a symlink to doas.
The thing is that they behave differently:

$ sudo KEY=value echo good
good
$ doas KEY=value echo bad
doas: KEY=value: command not found

The syntax of K=v before a command is only valid in a shell. But from what I understand, doas runs the command directly. So the fix is to prepend with "env":

$ doas env KEY=value echo good
good

In my case, I had HOME=/root: command not found.

Context

The fix is rather simple, so it wasn't worth to open an issue ticket.

Checklist for maintainers

Maintainers: tick if completed or explain if not relevant

  • agreed on idea
  • agreed on implementation strategy
  • tests, as appropriate
    • functional tests - tests/**.sh
    • unit tests - src/*/tests
    • integration tests - tests/nixos/*
  • documentation in the manual
  • documentation in the internal API docs
  • code and comments are self-explanatory
  • commit message explains why the change was made
  • new feature or incompatible change: updated release notes

Priorities

Add 👍 to pull requests you find important.

In my system, sudo is a symlink to doas.
The thing is that they behave differently::
  $ sudo KEY=value echo good
  good
  $ doas KEY=value echo bad
  doas: KEY=value: command not found
The syntax of K=v before a command is only valid in a shell. But from
what I understand, doas runs the command directly. So the fix is to prepend
with "env":
  $ doas env KEY=value echo good
  good
@concatime concatime requested a review from edolstra as a code owner June 29, 2023 20:16
@abathur
Copy link
Member

abathur commented Jun 30, 2023

Just dot-connecting: this is either a duplicate of or would supersede:

@concatime
Copy link
Author

Well, I didn't see the PR you pinned. It's definitely a duplicate.

@roberth
Copy link
Member

roberth commented Jun 30, 2023

duplicate.

This PR is up to date and arguably more complete, fixing four instances of the problem instead of three.
That does show the fragility of the solution though, but we can solve that by testing with doas (as well as sudo of course) in CI, in tests/installer/default.nix. Could you add a new test there?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants