Skip to content

chore: propose coder dotfiles command #1696

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

Closed
wants to merge 8 commits into from
Closed

Conversation

f0ssel
Copy link
Contributor

@f0ssel f0ssel commented May 23, 2022

This draft is an example of how a customer would interact with the coder dotfiles feature.

Talking with @jsjoeio and @bpmct we tried to come up with an ideal user experience for dotfiles. We want to support a simple dotfiles flow but don't want to require it on users or get in the way of them customizing the flow.

What I'm proposing:

  1. Hook into template startup_script variable to add a script that calls coder dotfiles <url>
  2. Add a coder dotfiles command to our cli that follows the following algorithms:
  1. Provide documentation that links out to usage examples in the example directory of a template using the coder dotfiles flow.

The general flow of the coder dotfiles command will be:

  1. Check out repo, deal with updating if already exists
  2. Check for some type of init script, if exists then run it
  3. if no script exists copy over the files starting with .

You would fill in the dotfiles url during workspace creation like we currently do with "region" in dogfood:
image

@f0ssel f0ssel requested review from ammario and tjcran May 23, 2022 22:03
@bpmct
Copy link
Member

bpmct commented May 23, 2022

I'm in favor of this approach for the community MVP 👍🏼

This ensures the workspace lifecycle is fully-managed at the template level, but also does not remove support for dotfiles (which has first class support in other remote development platforms).

@johnstcn
Copy link
Member

johnstcn commented May 23, 2022

I really like this approach, especially that we can bake it into the coder binary itself and avoid shipping a shell script.

Check out repo, deal with updating if already exists

What do we do if something "weird" happens and we can't fast-forward to the remote branch? Bail? Force-reset?

@f0ssel
Copy link
Contributor Author

f0ssel commented May 23, 2022

I really like this approach, especially that we can bake it into the coder binary itself and avoid shipping a shell script.

Check out repo, deal with updating if already exists

What do we do if something "weird" happens and we can't fast-forward to the remote branch? Bail? Force-reset?

My default plan is to fallback to whatever the current personalize script from V1 does, which I think is just present the git error. Very much open to input on handling cases.

@johnstcn
Copy link
Member

I really like this approach, especially that we can bake it into the coder binary itself and avoid shipping a shell script.

Check out repo, deal with updating if already exists

What do we do if something "weird" happens and we can't fast-forward to the remote branch? Bail? Force-reset?

My default plan is to fallback to whatever the current personalize script from V1 does, which I think is just present the git error. Very much open to input on handling cases.

I think just keeping v1 behaviour is legit for starts 👍

Down the line, I could imagine some additional fanciness, such as:

  • If the checked out repo is dirty, stash all the local changes (maybe also add all untracked files in case they conflict with upstream changes)
  • If the local and remote branches have diverged, rename the local branch with a suffix coder-$(date) and set the original local branch to upstream

In both of those cases, we'd want to warn the user.

@kylecarbs
Copy link
Member

I had no idea that GitHub documented a list of places to check for dotfiles init scripts, but that's really cool. I really like the simple addition of a command, and this integrates ✨ beautifully ✨ with gitssh! 👏

@ammario
Copy link
Member

ammario commented May 24, 2022

It's simple UX, market-driven, easy to implement, etc. I did a cursory glance but I like the proposed solution. I defer to the other smart people in this thread to make the right call.

@ketang
Copy link
Contributor

ketang commented May 24, 2022

I may be misunderstanding this, but it sounds like the above implementation is Terraform-specific. I also am not thrilled about this being under the project owner's control rather than the user's.

@f0ssel
Copy link
Contributor Author

f0ssel commented May 24, 2022

I may be misunderstanding this, but it sounds like the above implementation is Terraform-specific. I also am not thrilled about this being under the project owner's control rather than the user's.

It is not terraform specific, the coder dotfiles command can be run anytime. For example, if we didn't add this to our dogfood template I could just run coder dotfiles git@github.com:f0ssel/coder-dotfiles.git every now and then in my workspace and I'll get the same experience in a manual fashion.

It is up to the template owners to implement the call to coder dotfiles in the startup script, but the url can be easily made a variable that can be filled by the user at workspace build. The terraform changes here are an example of what the real diff would look like if we added added dotfiles to the example local docker template.

@ketang
Copy link
Contributor

ketang commented May 24, 2022

I do understand that there's a use case where customers want the same dotfiles for everyone, so this serves that. I guess it doesn't get in the way of something user-driven.

That just leaves me with the concern about it being Terraform specific. I think this is a good opportunity to consider the workspace lifecycle hooks idea. I think it's not too different in complexity, especially if we're just doing a start event initially, but it's more flexible and more general.

@f0ssel
Copy link
Contributor Author

f0ssel commented May 24, 2022

Yes it will support any kind of commands for the startup script, so you could have it be:
coder dotfiles git@github.com:coder/org-dotfiles.git
or it could be
coder dotfiles ${var.dotfiles}

Up to the template owner.

The startup_script is essentially a one-off workspace lifecycle hook, it always runs if specified after the agent has started successfully. I believe there's value in a formal design for workspace lifecycle hooks, but right now I have what I need to implement this today with startup_script, so I think that can be discussed in a separate issue. Whatever we land on there, I'm sure we can hook in the dotfiles command somewhere that makes sense.

@tjcran
Copy link

tjcran commented May 24, 2022

I'm in favor of this approach as a first-step.

@dwahler
Copy link
Contributor

dwahler commented May 24, 2022

Hook into template startup_script variable to add a script that calls coder dotfiles <url>

This seems like a possible point of incompatibility.

I'm not sure about Codespaces, but I just tested how dotfiles work on Gitpod, and it looks like they preserve the /workspace working directory but don't preserve the contents of the /home/gitpod home directory across workspace restarts. So if a dotfiles repo has an install script, we could end up running it multiple times in the same homedir, even though Gitpod only runs it once. This could break things if the script isn't idempotent.

I guess ultimately, the decision of how to handle this is up to the template author. But if we plan to ship templates where the home directory is persistent, then to maximize compatibility, maybe we should provide a first_run_script in addition to startup_script, and use that to invoke the dotfiles command?

Or we could just punt on it for now, and warn users that the script may be run an undefined number of times.

@f0ssel
Copy link
Contributor Author

f0ssel commented May 25, 2022

@dwahler that is a great point. I appreciate you doing this research into how others have solved similar problems.

We currently have the expectation of idempotent install scripts in V1 (not that it's a total authority, but an existing expectation), and the coder dotfiles command itself is also idempotent. I feel pretty okay with that requirement today but I do see the divergence from the way others handle it.

Another point is we don't really have an opinion on what persists, it's completely up to the template. I can see this being a good case for letting the command take in an argument for where to store the repo, so the caller can place it somewhere ephemeral if they'd like to.

@ketang
Copy link
Contributor

ketang commented May 25, 2022

If we see this as the first lifecycle hook, I think we should reconsider the name startup_script and instead use language consistent with the internal event name and event handlers in general, e.g. onStart.

@f0ssel
Copy link
Contributor Author

f0ssel commented May 26, 2022

If we see this as the first lifecycle hook, I think we should reconsider the name startup_script and instead use language consistent with the internal event name and event handlers in general, e.g. onStart.

I agree, but we should discuss in a dedicated issue so we can prioritize it correctly. I think it's doable to make these changes before community if we find it valuable.

@f0ssel
Copy link
Contributor Author

f0ssel commented May 26, 2022

Closing now that the implementation is merged.

@f0ssel f0ssel closed this May 26, 2022
@f0ssel f0ssel deleted the f0ssel/dotfiles-poc branch May 26, 2022 20:49
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.

8 participants