Refactoring and comments

cmbrose · web-flow · commit a03051f295de · 2022-03-22T14:49:58.000Z
diff --git a/internal/codespaces/ssh.go b/internal/codespaces/ssh.go
@@ -29,47 +29,18 @@ func Shell(ctx context.Context, p printer, sshArgs []string, port int, destinati
 	return cmd.Run()
 }
 
-// Copy runs an scp command over the specified port. The arguments may
-// include flags and non-flags, optionally separated by "--".
+// Copy runs an scp command over the specified port. scpArgs should contain both scp flags
+// as well as the list of files to copy, with the flags first.
 //
 // Remote files indicated by a "remote:" prefix are resolved relative
 // to the remote user's home directory, and are subject to shell expansion
 // on the remote host; see https://lwn.net/Articles/835962/.
 func Copy(ctx context.Context, scpArgs []string, port int, destination string) error {
-	// Beware: invalid syntax causes scp to exit 1 with
-	// no error message, so don't let that happen.
-	connArgs := []string{
-		"-P", strconv.Itoa(port),
-		"-o", "NoHostAuthenticationForLocalhost=yes",
-		"-C", // compression
-	}
-
-	cmdArgs, command, err := parseSCPArgs(scpArgs)
+	cmd, err := newSCPCommand(ctx, port, destination, scpArgs)
 	if err != nil {
-		return err
-	}
-
-	cmdArgs = append(cmdArgs, connArgs...)
-
-	if len(command) > 0 {
-		cmdArgs = append(cmdArgs, "--")
-
-		for _, arg := range command {
-			// Replace "remote:" prefix with (e.g.) "root@localhost:".
-			if rest := strings.TrimPrefix(arg, "remote:"); rest != arg {
-				arg = destination + ":" + rest
-			}
-			cmdArgs = append(cmdArgs, arg)
-		}
+		return fmt.Errorf("failed to create scp command: %w", err)
 	}
 
-	fmt.Println(cmdArgs)
-
-	cmd := exec.CommandContext(ctx, "scp", cmdArgs...)
-
-	cmd.Stdin = nil
-	cmd.Stdout = os.Stderr
-	cmd.Stderr = os.Stderr
 	return cmd.Run()
 }
 
@@ -116,11 +87,49 @@ func parseSSHArgs(args []string) (cmdArgs, command []string, err error) {
 	return parseArgs(args, "bcDeFIiLlmOopRSWw")
 }
 
+// newSCPCommand populates an exec.Cmd to run an scp command for the files specified in cmdArgs.
+// cmdArgs is parsed such that scp flags and the files to copy are separated by a "--" in the command.
+// For example: scp -F ./config -- local/file remote:file
+func newSCPCommand(ctx context.Context, port int, dst string, cmdArgs []string) (*exec.Cmd, error) {
+	// Beware: invalid syntax causes scp to exit 1 with
+	// no error message, so don't let that happen.
+	connArgs := []string{
+		"-P", strconv.Itoa(port),
+		"-o", "NoHostAuthenticationForLocalhost=yes",
+		"-C", // compression
+	}
+
+	cmdArgs, command, err := parseSCPArgs(cmdArgs)
+	if err != nil {
+		return nil, err
+	}
+
+	cmdArgs = append(cmdArgs, connArgs...)
+
+	cmdArgs = append(cmdArgs, "--")
+
+	for _, arg := range command {
+		// Replace "remote:" prefix with (e.g.) "root@localhost:".
+		if rest := strings.TrimPrefix(arg, "remote:"); rest != arg {
+			arg = dst + ":" + rest
+		}
+		cmdArgs = append(cmdArgs, arg)
+	}
+
+	cmd := exec.CommandContext(ctx, "scp", cmdArgs...)
+
+	cmd.Stdin = nil
+	cmd.Stdout = os.Stderr
+	cmd.Stderr = os.Stderr
+
+	return cmd, nil
+}
+
 func parseSCPArgs(args []string) (cmdArgs, command []string, err error) {
 	return parseArgs(args, "cFiJloPS")
 }
 
-// parseArgs parses arguments into two distinct slices of flags and command.  Parsing stops
+// parseArgs parses arguments into two distinct slices of flags and command. Parsing stops
 // as soon as a non-flag argument is found assuming the remaining arguments are the command.
 // It returns an error if a unary flag is provided without an argument.
 func parseArgs(args []string, unaryFlags string) (cmdArgs, command []string, err error) {
