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

f0ssel added10 commits

May 25, 2022 19:11

pr comments

2197126

save dotfiles url config

9e073ab

fixup

0467613

%q

3b505b0

testing

fd9ba2a

testing

624265f

testing

44efa67

organize:

b059fdf

add symlink backup test

b265149

handle script for windows

581d6b8

johnstcn approved these changes

May 25, 2022

View reviewed changes

Copy link

Member

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 OutdatedShow resolvedHide resolved

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

johnstcnMay 25, 2022

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

ContributorAuthor

f0sselMay 25, 2022

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.

cli/dotfiles_test.go Outdated

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

johnstcnMay 25, 2022

Choose a reason for hiding this comment

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

praise: nice tests!

f0ssel added2 commits

May 25, 2022 20:45

switch to require

6dfc469

pr comments

df5f6cd

Copy link

ContributorAuthor

f0ssel commentedMay 25, 2022

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

skip install script test on windows

b29ddf4

Copy link

ContributorAuthor

f0ssel commentedMay 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.

fix command

a435224

f0ssel merged commit35ccb88 intomain

May 25, 2022

f0ssel deleted the f0ssel/dotfiles-cmd branch

May 25, 2022 21:43

kylecarbs reviewed

May 25, 2022

View reviewed changes

cli/dotfiles.go

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

kylecarbsMay 25, 2022

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!