Repository: exercism/cli
Pull request #614 (2018-07-12 12:17:24)

Do not clobber when configuring workspace
We already make sure we're not clobbering anything when setting the default workspace,
however if you pass the --workspace flag explicitly, we have just been trusting the
input.

There is one case where we don't want to trust the input, and that is if they
downloaded the CLI to the exact same place as the workspace that they
pass as a flag.

This will typically happen if:

1. They downloaded the CLI to their home directory
2. Extracted the archive right there
3. Followed the default instructions for configuring
4. At this point, the error message will
   - explain what is wrong, and
   - show how to use --workspace to explicitly configure it
5. If they then use --workspace with the default it blows up

Instead of trying to overwrite the CLI binary with a directory,
this adds an extra check to say that if there's a directory at the
location, it's fine, but if it's anything else, exit with an error
message.

kytrinyx: I'm going to merge this in, but as always I would love to hear it if you have thoughts on anything.

Patch:
diff --git a/cmd/configure.go b/cmd/configure.go
index 7744786..6e08016 100644
--- a/cmd/configure.go
+++ b/cmd/configure.go
@@ -150,6 +150,25 @@ func runConfigure(configuration config.Configuration, flags *pflag.FlagSet) erro
 		workspace = cfg.GetString("workspace")
 	}
 	workspace = config.Resolve(workspace, configuration.Home)
+
+	if workspace != "" {
+		// If there is a non-directory here, then we cannot proceed.
+		if info, err := os.Lstat(workspace); !os.IsNotExist(err) && !info.IsDir() {
+			msg := `
+
+			There is already something at the workspace location you are configuring:
+
+			  %s
+
+			Please rename it, or set a different workspace location:
+
+			  %s configure %s --workspace=PATH_TO_DIFFERENT_FOLDER
+			`
+
+			return errors.New(fmt.Sprintf(msg, workspace, BinaryName, commandify(flags)))
+		}
+	}
+
 	if workspace == "" {
 		workspace = config.DefaultWorkspaceDir(configuration)
 
@@ -160,7 +179,10 @@ func runConfigure(configuration config.Configuration, flags *pflag.FlagSet) erro
 
 			  %s
 
-			There is already a directory there.
+			There is already something there.
+			If it's a directory, that might be fine. If it's a file, you will need to move it first,
+			or choose a different location for the workspace.
+
 			You can choose the workspace location by rerunning this command with the --workspace flag.
 
 			  %s configure %s --workspace=%s


Test patch:
diff --git a/cmd/configure_test.go b/cmd/configure_test.go
index a029f89..adf30b7 100644
--- a/cmd/configure_test.go
+++ b/cmd/configure_test.go
@@ -8,6 +8,7 @@ import (
 	"net/http"
 	"net/http/httptest"
 	"os"
+	"path/filepath"
 	"testing"
 
 	"github.com/exercism/cli/config"
@@ -397,7 +398,47 @@ func TestConfigureDefaultWorkspaceWithoutClobbering(t *testing.T) {
 
 	err = runConfigure(cfg, flags)
 	assert.Error(t, err)
-	assert.Regexp(t, "already a directory", err.Error())
+	assert.Regexp(t, "already something", err.Error())
+}
+
+func TestConfigureExplicitWorkspaceWithoutClobberingNonDirectory(t *testing.T) {
+	oldOut := Out
+	oldErr := Err
+	Out = ioutil.Discard
+	Err = ioutil.Discard
+	defer func() {
+		Out = oldOut
+		Err = oldErr
+	}()
+
+	tmpDir, err := ioutil.TempDir("", "no-clobber")
+	assert.NoError(t, err)
+
+	v := viper.New()
+	v.Set("token", "abc123")
+
+	cfg := config.Configuration{
+		OS:              "linux",
+		DefaultDirName:  "workspace",
+		Home:            tmpDir,
+		Dir:             tmpDir,
+		UserViperConfig: v,
+		Persister:       config.InMemoryPersister{},
+	}
+
+	// Create a file at the workspace directory's location
+	err = ioutil.WriteFile(filepath.Join(tmpDir, "workspace"), []byte("This is not a directory"), os.FileMode(0755))
+	assert.NoError(t, err)
+
+	flags := pflag.NewFlagSet("fake", pflag.PanicOnError)
+	setupConfigureFlags(flags)
+	err = flags.Parse([]string{"--no-verify", "--workspace", config.DefaultWorkspaceDir(cfg)})
+	assert.NoError(t, err)
+
+	err = runConfigure(cfg, flags)
+	if assert.Error(t, err) {
+		assert.Regexp(t, "set a different workspace", err.Error())
+	}
 }
 
 func TestCommandifyFlagSet(t *testing.T) {