Added hidden console allocation for transport processes on Windows.

This commit updates the process attributes and flags used for transport
processes on Windows. Transport processes will now have a hidden console
allocated by default. They were previously created without a console and
would allocate one when required, creating a large number of pop-ups on
Windows when using the Docker transport.

This commit also takes the opportunity to reorganize the transport code
and attributes.

Fixes mutagen-io#233.

Signed-off-by: Jacob Howard <[email protected]>

Author: xenoscopic

cmd/mutagen/daemon/start_windows.go

@@ -3,10 +3,10 @@ package daemon
 import (
 	"syscall"
 
-	"github.com/mutagen-io/mutagen/pkg/process"
+	"golang.org/x/sys/windows"
 )
 
 // daemonProcessAttributes are the process attributes to use for the daemon.
 var daemonProcessAttributes = &syscall.SysProcAttr{
-	CreationFlags: process.DETACHED_PROCESS | syscall.CREATE_NEW_PROCESS_GROUP,
+	CreationFlags: windows.DETACHED_PROCESS | windows.CREATE_NEW_PROCESS_GROUP,
 }

pkg/agent/transport/doc.go

@@ -0,0 +1,2 @@
+// Package transport provides common functionality for agent transports.
+package transport

pkg/agent/transports/docker/doc.go renamed to pkg/agent/transport/docker/doc.go

@@ -11,7 +11,8 @@ import (
 	"github.com/pkg/errors"
 
 	"github.com/mutagen-io/mutagen/pkg/agent"
-	"github.com/mutagen-io/mutagen/pkg/agent/transports/ssh"
+	"github.com/mutagen-io/mutagen/pkg/agent/transport"
+	"github.com/mutagen-io/mutagen/pkg/agent/transport/ssh"
 	"github.com/mutagen-io/mutagen/pkg/docker"
 	"github.com/mutagen-io/mutagen/pkg/process"
 	"github.com/mutagen-io/mutagen/pkg/prompting"
@@ -26,8 +27,8 @@ Hyper-V doesn't support copying files into running containers.
 
 Would you like to continue? (yes/no)? `
 
-// transport implements the agent.Transport interface using Docker.
-type transport struct {
+// dockerTransport implements the agent.Transport interface using Docker.
+type dockerTransport struct {
 	// container is the target container name.
 	container string
 	// user is the container user under which agents should be invoked.
@@ -73,7 +74,7 @@ func NewTransport(container, user string, environment, parameters map[string]str
 	}
 
 	// Success.
-	return &transport{
+	return &dockerTransport{
 		container:             container,
 		user:                  user,
 		environment:           environment,
@@ -86,7 +87,7 @@ func NewTransport(container, user string, environment, parameters map[string]str
 // specification of the working directory inside the container, as well as an
 // override of the executing user. An empty user specification means to use the
 // username specified in the remote URL, if any.
-func (t *transport) command(command, workingDirectory, user string) (*exec.Cmd, error) {
+func (t *dockerTransport) command(command, workingDirectory, user string) (*exec.Cmd, error) {
 	// Set up top-level command-line flags.
 	var dockerArguments []string
 	dockerArguments = append(dockerArguments, t.daemonConnectionFlags...)
@@ -125,8 +126,8 @@ func (t *transport) command(command, workingDirectory, user string) (*exec.Cmd,
 		return nil, err
 	}
 
-	// Force it to run detached.
-	dockerCommand.SysProcAttr = process.DetachedProcessAttributes()
+	// Set the process attributes.
+	dockerCommand.SysProcAttr = transport.ProcessAttributes()
 
 	// Create a copy of the current environment.
 	environment := os.Environ()
@@ -152,7 +153,7 @@ func (t *transport) command(command, workingDirectory, user string) (*exec.Cmd,
 // probeContainer ensures that the containerIsWindows and containerHomeDirectory
 // fields are populated. It is idempotent. If probing previously failed, probing
 // will simply return an error indicating the previous failure.
-func (t *transport) probeContainer() error {
+func (t *dockerTransport) probeContainer() error {
 	// Watch for previous errors.
 	if t.containerProbeError != nil {
 		return errors.Wrap(t.containerProbeError, "previous container probing failed")
@@ -286,7 +287,7 @@ func (t *transport) probeContainer() error {
 
 // changeContainerStatus stops or starts the container. It is required for
 // copying files on Windows when using Hyper-V.
-func (t *transport) changeContainerStatus(stop bool) error {
+func (t *dockerTransport) changeContainerStatus(stop bool) error {
 	// Set up top-level command-line flags.
 	var dockerArguments []string
 	dockerArguments = append(dockerArguments, t.daemonConnectionFlags...)
@@ -304,8 +305,8 @@ func (t *transport) changeContainerStatus(stop bool) error {
 		return errors.Wrap(err, "unable to set up Docker invocation")
 	}
 
-	// Force it to run detached.
-	dockerCommand.SysProcAttr = process.DetachedProcessAttributes()
+	// Set the process attributes.
+	dockerCommand.SysProcAttr = transport.ProcessAttributes()
 
 	// Create a copy of the current environment.
 	environment := os.Environ()
@@ -329,7 +330,7 @@ func (t *transport) changeContainerStatus(stop bool) error {
 }
 
 // Copy implements the Copy method of agent.Transport.
-func (t *transport) Copy(localPath, remoteName string) error {
+func (t *dockerTransport) Copy(localPath, remoteName string) error {
 	// Ensure that the container has been probed.
 	if err := t.probeContainer(); err != nil {
 		return errors.Wrap(err, "unable to probe container")
@@ -390,8 +391,8 @@ func (t *transport) Copy(localPath, remoteName string) error {
 		return errors.Wrap(err, "unable to set up Docker invocation")
 	}
 
-	// Force it to run detached.
-	dockerCommand.SysProcAttr = process.DetachedProcessAttributes()
+	// Set the process attributes.
+	dockerCommand.SysProcAttr = transport.ProcessAttributes()
 
 	// Create a copy of the current environment.
 	environment := os.Environ()
@@ -464,7 +465,7 @@ func (t *transport) Copy(localPath, remoteName string) error {
 }
 
 // Command implements the Command method of agent.Transport.
-func (t *transport) Command(command string) (*exec.Cmd, error) {
+func (t *dockerTransport) Command(command string) (*exec.Cmd, error) {
 	// Ensure that the container has been probed.
 	if err := t.probeContainer(); err != nil {
 		return nil, errors.Wrap(err, "unable to probe container")
@@ -475,7 +476,7 @@ func (t *transport) Command(command string) (*exec.Cmd, error) {
 }
 
 // ClassifyError implements the ClassifyError method of agent.Transport.
-func (t *transport) ClassifyError(processState *os.ProcessState, errorOutput string) (bool, bool, error) {
+func (t *dockerTransport) ClassifyError(processState *os.ProcessState, errorOutput string) (bool, bool, error) {
 	// Ensure that the container has been probed.
 	if err := t.probeContainer(); err != nil {
 		return false, false, errors.Wrap(err, "unable to probe container")

pkg/agent/transports/docker/environment.go renamed to pkg/agent/transport/docker/environment.go

@@ -2,15 +2,15 @@
 
 // TODO: Figure out what to do for Plan 9. It doesn't support Setsid.
 
-package process
+package transport
 
 import (
 	"syscall"
 )
 
-// DetachedProcessAttributes returns the process attributes to use for starting
-// detached processes.
-func DetachedProcessAttributes() *syscall.SysProcAttr {
+// ProcessAttributes returns the process attributes to use for starting
+// transport processes from the daemon.
+func ProcessAttributes() *syscall.SysProcAttr {
 	return &syscall.SysProcAttr{
 		// There's also a Noctty field, but it only detaches standard input from
 		// the controlling terminal (not standard output or error), and if

pkg/agent/transports/docker/environment_test.go renamed to pkg/agent/transport/docker/environment_test.go

@@ -0,0 +1,12 @@
+package transport
+
+import (
+	"testing"
+)
+
+// TestProcessAttributes tests ProcessAttributes.
+func TestProcessAttributes(t *testing.T) {
+	if ProcessAttributes() == nil {
+		t.Error("nil transport process attributes returned")
+	}
+}

pkg/agent/transports/docker/transport.go renamed to pkg/agent/transport/docker/transport.go

@@ -0,0 +1,16 @@
+package transport
+
+import (
+	"syscall"
+
+	"golang.org/x/sys/windows"
+)
+
+// ProcessAttributes returns the process attributes to use for starting
+// transport processes from the daemon.
+func ProcessAttributes() *syscall.SysProcAttr {
+	return &syscall.SysProcAttr{
+		HideWindow:    true,
+		CreationFlags: windows.CREATE_NEW_CONSOLE,
+	}
+}

pkg/agent/transports/docker/transport_test.go renamed to pkg/agent/transport/docker/transport_test.go

@@ -10,6 +10,7 @@ import (
 	"github.com/pkg/errors"
 
 	"github.com/mutagen-io/mutagen/pkg/agent"
+	"github.com/mutagen-io/mutagen/pkg/agent/transport"
 	"github.com/mutagen-io/mutagen/pkg/process"
 	"github.com/mutagen-io/mutagen/pkg/ssh"
 )
@@ -27,8 +28,8 @@ const (
 	serverAliveCountMax = 1
 )
 
-// transport implements the agent.Transport interface using SSH.
-type transport struct {
+// sshTransport implements the agent.Transport interface using SSH.
+type sshTransport struct {
 	// user is the SSH user under which agents should be invoked.
 	user string
 	// host is the target host.
@@ -41,7 +42,7 @@ type transport struct {
 
 // NewTransport creates a new SSH transport using the specified parameters.
 func NewTransport(user, host string, port uint16, prompter string) (agent.Transport, error) {
-	return &transport{
+	return &sshTransport{
 		user:     user,
 		host:     host,
 		port:     port,
@@ -50,7 +51,7 @@ func NewTransport(user, host string, port uint16, prompter string) (agent.Transp
 }
 
 // Copy implements the Copy method of agent.Transport.
-func (t *transport) Copy(localPath, remoteName string) error {
+func (t *sshTransport) Copy(localPath, remoteName string) error {
 	// HACK: On Windows, we attempt to use SCP executables that might not
 	// understand Windows paths because they're designed to run inside a POSIX-
 	// style environment (e.g. MSYS or Cygwin). To work around this, we run them
@@ -100,8 +101,8 @@ func (t *transport) Copy(localPath, remoteName string) error {
 	// Set the working directory.
 	scpCommand.Dir = workingDirectory
 
-	// Force it to run detached.
-	scpCommand.SysProcAttr = process.DetachedProcessAttributes()
+	// Set the process attributes.
+	scpCommand.SysProcAttr = transport.ProcessAttributes()
 
 	// Create a copy of the current environment.
 	environment := os.Environ()
@@ -128,7 +129,7 @@ func (t *transport) Copy(localPath, remoteName string) error {
 }
 
 // Command implements the Command method of agent.Transport.
-func (t *transport) Command(command string) (*exec.Cmd, error) {
+func (t *sshTransport) Command(command string) (*exec.Cmd, error) {
 	// Compute the target.
 	target := t.host
 	if t.user != "" {
@@ -154,7 +155,7 @@ func (t *transport) Command(command string) (*exec.Cmd, error) {
 	}
 
 	// Force it to run detached.
-	sshCommand.SysProcAttr = process.DetachedProcessAttributes()
+	sshCommand.SysProcAttr = transport.ProcessAttributes()
 
 	// Create a copy of the current environment.
 	environment := os.Environ()
@@ -176,7 +177,7 @@ func (t *transport) Command(command string) (*exec.Cmd, error) {
 }
 
 // ClassifyError implements the ClassifyError method of agent.Transport.
-func (t *transport) ClassifyError(processState *os.ProcessState, errorOutput string) (bool, bool, error) {
+func (t *sshTransport) ClassifyError(processState *os.ProcessState, errorOutput string) (bool, bool, error) {
 	// SSH faithfully returns exit codes and error output, so we can use direct
 	// methods for testing and classification. Note that we may get POSIX-like
 	// error codes back even from Windows remotes, but that indicates a POSIX

pkg/process/attributes_posix.go renamed to pkg/agent/transport/process_posix.go

@@ -36,7 +36,7 @@ func TestCopy(t *testing.T) {
 	}
 
 	// Create a transport.
-	transport := &transport{
+	transport := &sshTransport{
 		user: user.Username,
 		host: "localhost",
 		port: 22,
@@ -86,7 +86,7 @@ func TestCommandOutput(t *testing.T) {
 	}
 
 	// Create a transport.
-	transport := &transport{
+	transport := &sshTransport{
 		user: user.Username,
 		host: "localhost",
 		port: 22,

pkg/agent/transport/process_test.go

@@ -7,7 +7,7 @@ import (
 	"net"
 
 	"github.com/mutagen-io/mutagen/pkg/agent"
-	"github.com/mutagen-io/mutagen/pkg/agent/transports/docker"
+	"github.com/mutagen-io/mutagen/pkg/agent/transport/docker"
 	"github.com/mutagen-io/mutagen/pkg/forwarding"
 	"github.com/mutagen-io/mutagen/pkg/forwarding/endpoint/remote"
 	"github.com/mutagen-io/mutagen/pkg/logging"

pkg/agent/transport/process_windows.go

@@ -7,7 +7,7 @@ import (
 	"net"
 
 	"github.com/mutagen-io/mutagen/pkg/agent"
-	"github.com/mutagen-io/mutagen/pkg/agent/transports/ssh"
+	"github.com/mutagen-io/mutagen/pkg/agent/transport/ssh"
 	"github.com/mutagen-io/mutagen/pkg/forwarding"
 	"github.com/mutagen-io/mutagen/pkg/forwarding/endpoint/remote"
 	"github.com/mutagen-io/mutagen/pkg/logging"

pkg/agent/transports/ssh/doc.go renamed to pkg/agent/transport/ssh/doc.go

@@ -7,7 +7,7 @@ import (
 	"net"
 
 	"github.com/mutagen-io/mutagen/pkg/agent"
-	"github.com/mutagen-io/mutagen/pkg/agent/transports/docker"
+	"github.com/mutagen-io/mutagen/pkg/agent/transport/docker"
 	"github.com/mutagen-io/mutagen/pkg/logging"
 	"github.com/mutagen-io/mutagen/pkg/synchronization"
 	"github.com/mutagen-io/mutagen/pkg/synchronization/endpoint/remote"

pkg/agent/transports/ssh/locale.go renamed to pkg/agent/transport/ssh/locale.go

@@ -7,7 +7,7 @@ import (
 	"net"
 
 	"github.com/mutagen-io/mutagen/pkg/agent"
-	"github.com/mutagen-io/mutagen/pkg/agent/transports/ssh"
+	"github.com/mutagen-io/mutagen/pkg/agent/transport/ssh"
 	"github.com/mutagen-io/mutagen/pkg/logging"
 	"github.com/mutagen-io/mutagen/pkg/synchronization"
 	"github.com/mutagen-io/mutagen/pkg/synchronization/endpoint/remote"