Skip to content

feat: add dotfiles command #1723

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

Merged
merged 32 commits into from
May 25, 2022
Merged

feat: add dotfiles command #1723

merged 32 commits into from
May 25, 2022

Conversation

f0ssel
Copy link
Contributor

@f0ssel f0ssel commented May 24, 2022

This PR is the implementation of the coder dotfiles command.

Refer to #1696 for more discussion.

@f0ssel f0ssel marked this pull request as ready for review May 24, 2022 18:55
@f0ssel f0ssel requested review from kylecarbs and a team May 24, 2022 19:02
@f0ssel
Copy link
Contributor Author

f0ssel commented May 24, 2022

I will attempt to get some tests going for this, but I expect it to be pretty difficult with all the shell callouts. I'll see how it goes.

@f0ssel
Copy link
Contributor Author

f0ssel commented May 24, 2022

I've added some cool tests that use local git repos to check for the different logic paths.

@Emyrk
Copy link
Member

Emyrk commented May 25, 2022

Is the idea to run this from within the workspace?

@f0ssel
Copy link
Contributor Author

f0ssel commented May 25, 2022

Is the idea to run this from within the workspace?

Yeah, mostly through the startup_script hook with the -y flag, but you can totally run it manually in the workspace if it's not integrated in the template.

Copy link
Member

@johnstcn johnstcn left a comment

Choose a reason for hiding this comment

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

Nice work! Some minor comments.

cli/dotfiles.go Outdated
Comment on lines 180 to 200
to := filepath.Join(symlinkDir, df)
_, _ = fmt.Fprintf(cmd.OutOrStdout(), "Symlinking %s to %s...\n", from, to)

isRegular, err := isRegular(to)
if err != nil {
return xerrors.Errorf("checking symlink for %s: %w", to, err)
}
// move conflicting non-symlink files to file.ext.bak
if isRegular {
backup := fmt.Sprintf("%s.bak", to)
_, _ = fmt.Fprintf(cmd.OutOrStdout(), "Moving %s to %s...\n", to, backup)
err = os.Rename(to, backup)
if err != nil {
return xerrors.Errorf("renaming dir %s: %w", to, err)
}
}

err = os.Symlink(from, to)
if err != nil {
return xerrors.Errorf("symlinking %s to %s: %w", from, to, err)
}
Copy link
Member

Choose a reason for hiding this comment

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

suggestion, non-blocking: this looks ripe for ripping out into a utility function and testing independently

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I already check this happy path in the tests, but I agree this can get hairy, will circle back if I have time.

Comment on lines 19 to 111
t.Run("MissingArg", func(t *testing.T) {
cmd, _ := clitest.New(t, "dotfiles")
err := cmd.Execute()
assert.Error(t, err)
})
t.Run("NoInstallScript", func(t *testing.T) {
_, root := clitest.New(t)
testRepo := testGitRepo(t, root)

// nolint:gosec
err := os.WriteFile(filepath.Join(testRepo, ".bashrc"), []byte("wow"), 0750)
assert.NoError(t, err)

c := exec.Command("git", "add", ".bashrc")
c.Dir = testRepo
err = c.Run()
assert.NoError(t, err)

c = exec.Command("git", "commit", "-m", `"add .bashrc"`)
c.Dir = testRepo
out, err := c.CombinedOutput()
assert.NoError(t, err, string(out))

cmd, _ := clitest.New(t, "dotfiles", "--global-config", string(root), "--symlink-dir", string(root), "-y", testRepo)
err = cmd.Execute()
assert.NoError(t, err)

b, err := os.ReadFile(filepath.Join(string(root), ".bashrc"))
assert.NoError(t, err)
assert.Equal(t, string(b), "wow")
})
t.Run("InstallScript", func(t *testing.T) {
_, root := clitest.New(t)
testRepo := testGitRepo(t, root)

// nolint:gosec
err := os.WriteFile(filepath.Join(testRepo, "install.sh"), []byte("#!/bin/bash\necho wow > "+filepath.Join(string(root), ".bashrc")), 0750)
assert.NoError(t, err)

c := exec.Command("git", "add", "install.sh")
c.Dir = testRepo
err = c.Run()
assert.NoError(t, err)

c = exec.Command("git", "commit", "-m", `"add install.sh"`)
c.Dir = testRepo
err = c.Run()
assert.NoError(t, err)

cmd, _ := clitest.New(t, "dotfiles", "--global-config", string(root), "--symlink-dir", string(root), "-y", testRepo)
err = cmd.Execute()
assert.NoError(t, err)

b, err := os.ReadFile(filepath.Join(string(root), ".bashrc"))
assert.NoError(t, err)
assert.Equal(t, string(b), "wow\n")
})
t.Run("SymlinkBackup", func(t *testing.T) {
_, root := clitest.New(t)
testRepo := testGitRepo(t, root)

// nolint:gosec
err := os.WriteFile(filepath.Join(testRepo, ".bashrc"), []byte("wow"), 0750)
assert.NoError(t, err)

// add a conflicting file at destination
// nolint:gosec
err = os.WriteFile(filepath.Join(string(root), ".bashrc"), []byte("backup"), 0750)
assert.NoError(t, err)

c := exec.Command("git", "add", ".bashrc")
c.Dir = testRepo
err = c.Run()
assert.NoError(t, err)

c = exec.Command("git", "commit", "-m", `"add .bashrc"`)
c.Dir = testRepo
out, err := c.CombinedOutput()
assert.NoError(t, err, string(out))

cmd, _ := clitest.New(t, "dotfiles", "--global-config", string(root), "--symlink-dir", string(root), "-y", testRepo)
err = cmd.Execute()
assert.NoError(t, err)

b, err := os.ReadFile(filepath.Join(string(root), ".bashrc"))
assert.NoError(t, err)
assert.Equal(t, string(b), "wow")

// check for backup file
b, err = os.ReadFile(filepath.Join(string(root), ".bashrc.bak"))
assert.NoError(t, err)
assert.Equal(t, string(b), "backup")
})
Copy link
Member

Choose a reason for hiding this comment

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

praise: nice tests!

@f0ssel
Copy link
Contributor Author

f0ssel commented May 25, 2022

Need to get the install script path happy running on windows, looking into it

@f0ssel
Copy link
Contributor Author

f0ssel commented May 25, 2022

After speaking with a few people, I'm just going to skip the windows install script test. Windows can still work in general, but specifically the install script case is weird enough to defer for now.

@f0ssel f0ssel merged commit 35ccb88 into main May 25, 2022
@f0ssel f0ssel deleted the f0ssel/dotfiles-cmd branch May 25, 2022 21:43
Comment on lines +27 to +28
Short: "Checkout and install a dotfiles repository.",
Example: "coder dotfiles [-y] git@github.com:example/dotfiles.git",
Copy link
Member

Choose a reason for hiding this comment

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

I think we should improve the usage here. It will not be immediately obvious to users how we intend for them to use this (inside or outside of a workspace).

A few examples will go a long way!

kylecarbs pushed a commit that referenced this pull request Jun 10, 2022
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.

6 participants