Revert "Generalize console control"

Signed-off-by: Michael Smith <[email protected]>

plugin/cache.go

@@ -164,11 +164,7 @@ func cachedList(ctx context.Context, p Parent) (*EntryMap, error) {
 		// Including the entry's ID allows plugin authors to use any Cached* methods defined on the
 		// children after their creation. This is necessary when the child's Cached* methods are used
 		// to calculate its attributes. Note that the child's ID is set in cachedOp.
-		var entries []Entry
-		err := withConsole(ctx, func(c context.Context) (err error) {
-			entries, err = p.List(context.WithValue(c, parentID, p.id()))
-			return
-		})
+		entries, err := p.List(context.WithValue(ctx, parentID, p.id()))
 		if err != nil {
 			return nil, err
 		}
@@ -217,12 +213,8 @@ func cachedList(ctx context.Context, p Parent) (*EntryMap, error) {
 // such as ReadAt or wrap it in a SectionReader. Using Read operations on the cached
 // reader will change it and make subsequent uses of the cached reader invalid.
 func cachedOpen(ctx context.Context, r Readable) (SizedReader, error) {
-	cachedContent, err := cachedDefaultOp(ctx, OpenOp, r, func() (rdr interface{}, err error) {
-		err = withConsole(ctx, func(c context.Context) (err error) {
-			rdr, err = r.Open(c)
-			return
-		})
-		return
+	cachedContent, err := cachedDefaultOp(ctx, OpenOp, r, func() (interface{}, error) {
+		return r.Open(ctx)
 	})
 
 	if err != nil {
@@ -234,12 +226,8 @@ func cachedOpen(ctx context.Context, r Readable) (SizedReader, error) {
 
 // cachedMetadata caches an entry's Metadata method
 func cachedMetadata(ctx context.Context, e Entry) (JSONObject, error) {
-	cachedMetadata, err := cachedDefaultOp(ctx, MetadataOp, e, func() (obj interface{}, err error) {
-		err = withConsole(ctx, func(c context.Context) (err error) {
-			obj, err = e.Metadata(c)
-			return
-		})
-		return
+	cachedMetadata, err := cachedDefaultOp(ctx, MetadataOp, e, func() (interface{}, error) {
+		return e.Metadata(ctx)
 	})
 
 	if err != nil {

plugin/interactive.go

@@ -1,12 +1,9 @@
 package plugin
 
 import (
-	"context"
 	"fmt"
 	"os"
-	"os/signal"
 	"sync"
-	"syscall"
 	"unsafe"
 
 	"github.com/mattn/go-isatty"
@@ -38,15 +35,19 @@ func tcSetpgrp(fd int, pgrp int) (err error) {
 	return unix.IoctlSetInt(fd, unix.TIOCSPGRP, int(uintptr(unsafe.Pointer(&v))))
 }
 
-// withConsole invokes the specified function while Wash has control of console input.
-// The function should save results by writing to captured variables.
-func withConsole(ctx context.Context, fn func(context.Context) error) error {
+// Only allow one Prompt call at a time. This prevents multiple plugins loading concurrently
+// from messing things up by calling Prompt concurrently.
+var promptMux sync.Mutex
+
+// Prompt prints the supplied message, then waits for input on stdin.
+func Prompt(msg string) (string, error) {
 	if !IsInteractive() {
-		// If not interactive, all we can do is call the function and return.
-		// If the function prompts for input without checking whether it can, then it may crash.
-		return fn(ctx)
+		return "", fmt.Errorf("not an interactive session")
 	}
 
+	promptMux.Lock()
+	defer promptMux.Unlock()
+
 	// Even if Wash is running interactively, it will not have control of STDIN while another command
 	// is running within the shell environment. If it doesn't have control and tries to read from it,
 	// the read will fail. If we have control, read normally. If not, temporarily acquire control for
@@ -55,63 +56,27 @@ func withConsole(ctx context.Context, fn func(context.Context) error) error {
 	inFd := int(os.Stdin.Fd())
 	inGrp, err := tcGetpgrp(inFd)
 	if err != nil {
-		return fmt.Errorf("error getting process group controlling stdin: %v", err)
+		return "", fmt.Errorf("error getting process group controlling stdin: %v", err)
 	}
 	curGrp := unix.Getpgrp()
 
+	var v string
 	if inGrp == curGrp {
-		return fn(ctx)
-	}
-
-	// Catch Ctrl-C while we have input control. Otherwise the shell exits.
-	sigCh := make(chan os.Signal, 1)
-	signal.Notify(sigCh, syscall.SIGINT)
-
-	// On Ctrl-C, cancel the function call.
-	cancelCtx, cancel := context.WithCancel(ctx)
-	go func() {
-		<-sigCh
-		cancel()
-	}()
-
-	// Cleanup when the function exits.
-	defer func() {
-		// Reset the signal watch first so we know there won't be any more attempts to
-		// write to sigCh after we close it.
-		signal.Reset(syscall.SIGINT)
-		close(sigCh)
-	}()
-
-	// Need to get control, call the function, then return control.
-	if err := tcSetpgrp(inFd, curGrp); err != nil {
-		return fmt.Errorf("error getting control of stdin: %v", err)
-	}
-
-	// Restore input control when we return.
-	defer func() {
+		// We control stdin
+		fmt.Fprintf(os.Stderr, "%s: ", msg)
+		_, err = fmt.Scanln(&v)
+	} else {
+		// Need to get control, prompt, then return control.
+		if err := tcSetpgrp(inFd, curGrp); err != nil {
+			return "", fmt.Errorf("error getting control of stdin: %v", err)
+		}
+		fmt.Fprintf(os.Stderr, "%s: ", msg)
+		_, err = fmt.Scanln(&v)
 		if err := tcSetpgrp(inFd, inGrp); err != nil {
 			// Panic if we can't return control. A messed up environment that they 'kill -9' is worse.
 			panic(err.Error())
 		}
-	}()
-
-	return fn(cancelCtx)
-}
-
-// Only allow one Prompt call at a time. This prevents multiple plugins loading concurrently
-// from messing things up by calling Prompt concurrently.
-var promptMux sync.Mutex
-
-// Prompt prints the supplied message, then waits for input on stdin.
-func Prompt(msg string) (v string, err error) {
-	if IsInteractive() {
-		promptMux.Lock()
-		defer promptMux.Unlock()
-
-		fmt.Fprintf(os.Stderr, "%s: ", msg)
-		_, err = fmt.Scanln(&v)
-	} else {
-		err = fmt.Errorf("not an interactive session")
 	}
-	return
+	// Return the error set by Scanln.
+	return v, err
 }

plugin/interactive_test.go

@@ -1,17 +1,12 @@
 package plugin
 
 import (
-	"context"
-	"fmt"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
 )
 
 func TestInteractive(t *testing.T) {
-	saveInteractive := isInteractive
-	defer func() { isInteractive = saveInteractive }()
-
 	if !IsInteractive() {
 		// If tests are not run interactively we can only test the false case, so override to true.
 		isInteractive = true
@@ -26,33 +21,3 @@ func TestInteractive(t *testing.T) {
 	InitInteractive(true)
 	assert.False(t, IsInteractive())
 }
-
-func TestWithConsole(t *testing.T) {
-	saveInteractive := isInteractive
-	defer func() { isInteractive = saveInteractive }()
-
-	called := false
-	passing := func(context.Context) error { called = true; return nil }
-	failing := func(context.Context) error { called = true; return fmt.Errorf("failed") }
-
-	// Test non-interactive first.
-	isInteractive = false
-	assert.NoError(t, withConsole(context.Background(), passing))
-	assert.True(t, called)
-
-	called = false
-	assert.Error(t, withConsole(context.Background(), failing), "failed")
-	assert.True(t, called)
-
-	// Interactive tests.
-	isInteractive = true
-
-	called = false
-	if saveInteractive {
-		assert.NoError(t, withConsole(context.Background(), passing))
-		assert.True(t, called)
-	} else {
-		assert.Error(t, withConsole(context.Background(), passing))
-		assert.False(t, called)
-	}
-}

plugin/methodWrappers.go

@@ -145,30 +145,18 @@ func Metadata(ctx context.Context, e Entry) (JSONObject, error) {
 }
 
 // Exec execs the command on the given entry.
-func Exec(ctx context.Context, e Execable, cmd string, args []string, opts ExecOptions) (result ExecCommand, err error) {
-	err = withConsole(ctx, func(c context.Context) (err error) {
-		result, err = e.Exec(ctx, cmd, args, opts)
-		return
-	})
-	return
+func Exec(ctx context.Context, e Execable, cmd string, args []string, opts ExecOptions) (ExecCommand, error) {
+	return e.Exec(ctx, cmd, args, opts)
 }
 
 // Stream streams the entry's content for updates.
-func Stream(ctx context.Context, s Streamable) (rdr io.ReadCloser, err error) {
-	err = withConsole(ctx, func(c context.Context) (err error) {
-		rdr, err = s.Stream(c)
-		return
-	})
-	return
+func Stream(ctx context.Context, s Streamable) (io.ReadCloser, error) {
+	return s.Stream(ctx)
 }
 
 // Write sends the supplied buffer to the entry.
-func Write(ctx context.Context, a Writable, offset int64, b []byte) (n int, err error) {
-	err = withConsole(ctx, func(c context.Context) (err error) {
-		n, err = a.Write(c, offset, b)
-		return
-	})
-	return
+func Write(ctx context.Context, a Writable, offset int64, b []byte) (int, error) {
+	return a.Write(ctx, offset, b)
 }
 
 // Signal signals the entry with the specified signal
@@ -210,10 +198,7 @@ func Signal(ctx context.Context, s Signalable, signal string) error {
 	}
 
 	// Go ahead and send the signal
-	err = withConsole(ctx, func(c context.Context) (err error) {
-		err = s.Signal(c, signal)
-		return
-	})
+	err = s.Signal(ctx, signal)
 	if err != nil {
 		return err
 	}
@@ -233,10 +218,7 @@ func Signal(ctx context.Context, s Signalable, signal string) error {
 
 // Delete deletes the given entry.
 func Delete(ctx context.Context, d Deletable) (deleted bool, err error) {
-	err = withConsole(ctx, func(c context.Context) (err error) {
-		deleted, err = d.Delete(c)
-		return
-	})
+	deleted, err = d.Delete(ctx)
 	if err != nil {
 		return
 	}