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

[WIP] more portable root user expansion #634

Closed

Conversation

abathur
Copy link
Collaborator

@abathur abathur commented Mar 7, 2020

Opening for discussion. Needs changes before merge. I suspect we can delete parts or break them out into other tests, and you may have ideas on another approach.

Thoughts/notes:

  1. [[ ~ == $(eval printf \'%s\\\n\' \~$LOGNAME) ]] is actually just part of an earlier attempt at fixing the test, but I kept the line for discussion since I haven't seen this narrow slice of behavior covered in another test. If you think it's worthwhile, I could imagine folding it into how _must_exist checks the first expansion, or creating a new test in tilde.test.sh?
  2. Because using grep to test/filter the root user's path /root weakens the stdout assertion a little, I tried to compensate by also testing that the directories exist. If you think testing the end of the root user's path is enough, I can cut _must_exist and some related changes:
    • I pared {/src,root} back to {,root} since ~/src is less-likely to exist than ~/
    • delayed overriding HOME to test that the user home exists before testing output

@andychu
Copy link
Contributor

andychu commented Mar 7, 2020

Hm I get that the underlying problem is that ~root isn't isolated... I think the "real" solution is to compare against what's in /etc/passwd then?

However I'm also kind of wary of making the tests less readable in the name of isolation. Like if we need to go back and debug brace expansion, it's nice to have dumb simple tests. I have a hard time reading this even if it's correct

I guess this is why I wanted to get an "overview" of what's wrong... i.e. so we can see if there are patterns that can let us fix a ton of tests at once, rather than rewriting tests one-by-one

@andychu
Copy link
Contributor

andychu commented Mar 7, 2020

So I am making good progress on #167 but we haven't audited all the failures, either in Alpine or in Nix, and I think that will help with this specific issue

@abathur
Copy link
Collaborator Author

abathur commented Mar 7, 2020

Agh. This is partly/temporarily moot anyways. I convinced myself this would work but I was forgetting that, on NixOS, the build sandbox sets root's homedir to /build; I'll close for now.

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

Successfully merging this pull request may close these issues.

2 participants