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

[studio] Use system artifact cache dir when running studio as root. #2844

Merged
merged 1 commit into from
Jul 28, 2017

Conversation

fnichol
Copy link
Collaborator

@fnichol fnichol commented Jul 28, 2017

This change was motivated by a bug fix to the artifact caching feature
introduced in #2737 but upon closer inspection, a much better
refactoring presented itself.

The key bit of insight was to remember that on Linux systems where
sudo will be called, we can set up the appropriate environment
variables before we become the root user (under the hood we call
exec with sudo to re-run the program with higher permissions). Now
that all the home directory-related logic is run under the non-root
user, there are far fewer edge conditions and we no longer need the
logic which attempt to determine that original calling user via the
SUDO_USER environment variable.

I did to myself and to all of us way back. Tech debt paid down!

tenor-138098484

@thesentinels
Copy link
Contributor

Thanks for the pull request! Here is what will happen next:

  1. Your PR will be reviewed by the maintainers
  2. If everything looks good, one of them will approve it, and your PR will be merged.

Thank you for contributing!

This change was motivated by a bug fix to the artifact caching feature
introduced in #2737 but upon closer inspection, a much better
refactoring presented itself.

The key bit of insight was to remember that on Linux systems where
`sudo` will be called, we can set up the appropriate environment
variables *before* we become the root user (under the hood we call
`exec` with sudo to re-run the program with higher permissions). Now
that all the home directory-related logic is run under the non-root
user, there are far fewer edge conditions and we no longer need the
logic which attempt to determine that original calling user via the
`SUDO_USER` environment variable.

Signed-off-by: Fletcher Nichol <fnichol@nichol.ca>
@fnichol fnichol force-pushed the fnichol/studio-sudo-refactor branch from 87c7345 to a16daa8 Compare July 28, 2017 01:05
@fnichol
Copy link
Collaborator Author

fnichol commented Jul 28, 2017

I rebased against master so hopefully appveyor will pass now

@mwrock
Copy link
Contributor

mwrock commented Jul 28, 2017

I'm building locally to test here

@reset
Copy link
Collaborator

reset commented Jul 28, 2017

@thesentinels approve

@thesentinels
Copy link
Contributor

🤘 I am testing your branch against master before merging it. We do this to ensure that the master branch is never failing tests.

@thesentinels
Copy link
Contributor

:neckbeard: Travis CI has started testing this PR.

1 similar comment
@thesentinels
Copy link
Contributor

:neckbeard: Travis CI has started testing this PR.

@thesentinels
Copy link
Contributor

💖 Travis CI reports this PR passed.

It always makes me feel nice when humans approve of one anothers work. I'm merging this PR now.

I just want you and the contributor to answer me one question:

gif-keyboard-3280869874741411265

@thesentinels thesentinels merged commit 8bf57ea into master Jul 28, 2017
@thesentinels thesentinels deleted the fnichol/studio-sudo-refactor branch July 28, 2017 01:35
@eeyun eeyun added I-linux and removed P-linux labels Mar 6, 2018
@christophermaier christophermaier added Focus: Studio Related to the Habitat Studio (core/hab-studio) component Focus: CLI Related to the Habitat CLI (core/hab) component and removed A-studio labels Jul 24, 2020
@christophermaier christophermaier added Platform: Linux Deals with Linux-specific behavior Platform: Windows Deals with Windows-specific behavior Platform: macOS Deals with macOS-specific behavior Type: Bug Issues that describe broken functionality and removed I-linux labels Jul 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Focus: CLI Related to the Habitat CLI (core/hab) component Focus: Studio Related to the Habitat Studio (core/hab-studio) component Platform: Linux Deals with Linux-specific behavior Platform: macOS Deals with macOS-specific behavior Platform: Windows Deals with Windows-specific behavior Type: Bug Issues that describe broken functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants