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

internal/setup: move sudo into SudoDevbox function + fix macOS CI #2043

Merged
merged 1 commit into from
May 14, 2024

Conversation

gcurtis
Copy link
Collaborator

@gcurtis gcurtis commented May 9, 2024

Move the code that calls sudo devbox into the setup package and clean it up a bit. Fix some issues that were preventing the cache from being configured in non-interactive environments (such as CI).

  • Don't re-prompt the user for confirmation in the sudo process.
  • Don't call NeedsRun a second time in the sudo process.
  • Don't prompt the user when stdin isn't a tty. This fixes cache configuration on macOS in CI.
  • Preserve Devbox environment variables such as DEVBOX_DEBUG, DEVBOX_API_TOKEN, DEVBOX_PROD, and DEVBOX_USE_VERSION. This addresses cases where the sudo devbox process would use the wrong auth or API endpoint.
  • Combine the nixcache aws and nix setup tasks into one task. There was a lot of overlap and this simplifies the setup code.

Reran the cache-upload workflow against this branch and it looks like the macOS runner is able to use the cache now - https://github.com/jetify-com/devbox/actions/runs/9022662173/job/24792707507#step:5:828

@gcurtis gcurtis requested a review from mikeland73 May 9, 2024 19:55
@gcurtis gcurtis changed the title internal/setup: move sudo into SudoDevbox function internal/setup: move sudo into SudoDevbox function + fix macOS CI May 9, 2024
Comment on lines +168 to +175
"XDG_CACHE_HOME",
"XDG_CONFIG_DIRS",
"XDG_CONFIG_HOME",
"XDG_DATA_DIRS",
"XDG_DATA_HOME",
"XDG_RUNTIME_DIR",
"XDG_STATE_HOME",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are there no issues with propagating all of these?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think so. Were you thinking of something specific? I added them in case the user has their XDG directories customized. Otherwise devbox cache credentials will use the default dirs.

internal/devbox/providers/nixcache/setup.go Outdated Show resolved Hide resolved
Move the code that calls `sudo devbox` into the `setup` package and
clean it up a bit. Fix some issues that were preventing the cache from
being configured in non-interactive environments (such as CI).

- Don't re-prompt the user for confirmation in the sudo process.
- Don't call NeedsRun a second time in the sudo process.
- Don't prompt the user when stdin isn't a tty. This fixes cache
  configuration on macOS in CI.
- Preserve Devbox environment variables such as DEVBOX_DEBUG,
  DEVBOX_API_TOKEN, DEVBOX_PROD, and DEVBOX_USE_VERSION. This addresses
  edge cases where the sudo devbox process would use the wrong auth or
  API endpoint.
- Combine the nixcache aws and nix setup tasks into one task. There was
  a lot of overlap and this simplifies the setup code.
@gcurtis gcurtis merged commit 1a31e20 into main May 14, 2024
24 checks passed
@gcurtis gcurtis deleted the gcurtis/sudo branch May 14, 2024 01:42
Copy link

sentry-io bot commented May 14, 2024

Suspect Issues

This pull request was deployed and Sentry observed the following issues:

  • ‼️ Generic Error: nixcache: run setup: setup: task : update nix config: don't know how to restart ... go.jetpack.io/devbox/internal/devbox/providers/... View Issue

Did you find this useful? React with a 👍 or 👎

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

Successfully merging this pull request may close these issues.

None yet

2 participants