Prepare for v8pipe support (livebud#116)

* package/js/v8server: migrate from using bud executable to render JS more quickly in dev to using a passed in re-write os.Pipe (WIP)
* tool/v8: rename 'bud tool v8 client' to 'bud tool v8 server'. 
* internal/extrafile: rework api to be less error-prone. 
* internal/imhash: improve performance by avoiding re-computing seen imports. 
* package/exe: fix restart command. 
* package/js: support os.Pipe for communicating between v8client and v8server. scripts: add some miscellaneous tests scripts
* bump up client timeout to fix tests

.github/workflows/test.yml

@@ -36,7 +36,7 @@ jobs:
       # Temporary fix to get the tests working until we pass the V8 client as a
       # os.Pipe
       - name: Install bud binary into $PATH
-        run: go install github.com/livebud/bud@latest
+        run: go install github.com/livebud/bud@v8pipe2
 
       - name: Install bud node_modules
         run: npm ci

internal/cli/cli.go

@@ -9,15 +9,16 @@ import (
 	"path/filepath"
 	"strings"
 
+	"github.com/livebud/bud/internal/extrafile"
+
 	"github.com/livebud/bud/internal/buildcache"
 	"github.com/livebud/bud/internal/cli/create"
 	tool_cache_clean "github.com/livebud/bud/internal/cli/tool/cache/clean"
 	tool_di "github.com/livebud/bud/internal/cli/tool/di"
 	tool_v8 "github.com/livebud/bud/internal/cli/tool/v8"
-	tool_v8_client "github.com/livebud/bud/internal/cli/tool/v8/client"
+	tool_v8_serve "github.com/livebud/bud/internal/cli/tool/v8/serve"
 	"github.com/livebud/bud/internal/cli/version"
 	"github.com/livebud/bud/internal/envs"
-	"github.com/livebud/bud/internal/extrafile"
 	"github.com/livebud/bud/internal/generator/command"
 	"github.com/livebud/bud/internal/generator/generator"
 	"github.com/livebud/bud/internal/generator/importfile"
@@ -79,19 +80,23 @@ func (c *CLI) Dir() string {
 }
 
 // Inject extra files into the command
-func (c *CLI) Inject(prefix string, files ...extrafile.File) error {
-	extras, env, err := extrafile.Prepare(prefix, len(c.ExtraFiles), files...)
-	if err != nil {
-		return err
+func (c *CLI) Inject(prefix string, files ...*os.File) error {
+	var env []string
+	extrafile.Inject(&c.ExtraFiles, &env, prefix, files...)
+	for _, ev := range env {
+		parts := strings.Split(ev, "=")
+		if len(parts) != 2 {
+			continue
+		}
+		c.Env[parts[0]] = parts[1]
 	}
-	c.ExtraFiles = append(c.ExtraFiles, extras...)
-	c.Env = c.Env.Append(env...)
 	return nil
 }
 
 // Run the CLI and wait for the command to finish
 func (c *CLI) Run(ctx context.Context, args ...string) error {
 	return c.parse(ctx, args, func(ctx context.Context) error {
+		// Generate and build the project CLI
 		cmd, err := c.compile(ctx)
 		if err != nil {
 			if errors.Is(err, fs.ErrNotExist) {
@@ -157,9 +162,9 @@ func (c *CLI) parse(ctx context.Context, args []string, fn func(ctx context.Cont
 			cli := cli.Command("v8", "Execute Javascript with V8 from stdin")
 			cli.Run(cmd.Run)
 
-			{ // $ bud tool v8 client
-				cmd := &tool_v8_client.Command{}
-				cli := cli.Command("client", "V8 client used during development")
+			{ // $ bud tool v8 serve
+				cmd := &tool_v8_serve.Command{}
+				cli := cli.Command("serve", "Serve from a V8 server during development")
 				cli.Run(cmd.Run)
 			}
 		}

internal/cli/testcli/testcli.go

@@ -87,7 +87,14 @@ func listen(path string) (socket.Listener, *http.Client, error) {
 		return nil, nil, err
 	}
 	client := &http.Client{
-		Timeout:   30 * time.Second,
+		// This is extra high right now because we don't currently have any signal
+		// that we've built the app and `bud run --embed` can take a long time. This
+		// is going to slow down legitimately failing requests, so it's a very
+		// temporary solution.
+		//
+		// TODO: support getting a signal that we've built the app, then lower this
+		// deadline.
+		Timeout:   60 * time.Second,
 		Transport: transport,
 		CheckRedirect: func(req *http.Request, via []*http.Request) error {
 			return http.ErrUseLastResponse
@@ -108,7 +115,12 @@ func (c *TestCLI) Start(ctx context.Context, args ...string) (app *App, stdout *
 	if err != nil {
 		return nil, nil, nil, fmt.Errorf("unable to listen on socket path %q: %s", appSocketPath, err)
 	}
-	if err := c.cli.Inject("APP", appListener); err != nil {
+	appFile, err := appListener.File()
+	if err != nil {
+		return nil, nil, nil, fmt.Errorf("unable to get the app file listener %q: %s", appSocketPath, err)
+	}
+	// extrafile.Inject()
+	if err := c.cli.Inject("APP", appFile); err != nil {
 		return nil, nil, nil, err
 	}
 	// Start listening on a unix domain socket for the hot
@@ -117,7 +129,11 @@ func (c *TestCLI) Start(ctx context.Context, args ...string) (app *App, stdout *
 	if err != nil {
 		return nil, nil, nil, fmt.Errorf("unable to listen on socket path %q: %s", hotSocketPath, err)
 	}
-	if err := c.cli.Inject("HOT", hotListener); err != nil {
+	hotFile, err := hotListener.File()
+	if err != nil {
+		return nil, nil, nil, fmt.Errorf("unable to get the hot file listener %q: %s", hotSocketPath, err)
+	}
+	if err := c.cli.Inject("HOT", hotFile); err != nil {
 		return nil, nil, nil, err
 	}
 	// Attach to stdout and stderr

internal/cli/tool/v8/client/client.go → internal/cli/tool/v8/serve/serve.go

@@ -1,7 +1,8 @@
-package client
+package serve
 
 import (
 	"context"
+	"os"
 
 	"github.com/livebud/bud/package/js/v8server"
 )
@@ -10,5 +11,5 @@ type Command struct {
 }
 
 func (c *Command) Run(ctx context.Context) error {
-	return v8server.Serve()
+	return v8server.New(os.Stdin, os.Stdout).Serve()
 }

internal/extrafile/extrafile.go

@@ -6,32 +6,13 @@ import (
 	"syscall"
 )
 
-type File interface {
-	File() (*os.File, error)
-}
-
-// Prepare is a low-level function for preparing files to be passed through a
-// subprocess via "os/exec".*Cmd.ExtraFiles. The prefix must be the same on both
-// sides. Use the offset to prepare multiple extra files to be passed through.
-// The offset should start at 0.
-func Prepare(prefix string, offset int, files ...File) ([]*os.File, []string, error) {
-	lenFiles := len(files)
-	osFiles := make([]*os.File, lenFiles)
-	for i, file := range files {
-		osFile, err := file.File()
-		if err != nil {
-			return nil, nil, err
-		}
-		osFiles[i] = osFile
-	}
-	env := prepareEnv(prefix, offset, osFiles)
-	return osFiles, env, nil
-}
-
-// Half-hearted attempt to support Systemd out of the box.
+// prepareEnv prepares the environment variables so the file descriptors can be
+// recovered in the subprocess.
+//
+// This is also a half-hearted attempt to support systemd out of the box.
 // https://github.com/coreos/go-systemd/blob/main/activation/files_unix.go
-// TODO: test this assumption
-func prepareEnv(prefix string, offset int, files []*os.File) []string {
+// TODO: test systemd support
+func prepareEnv(prefix string, offset int, files ...*os.File) []string {
 	if len(files) == 0 {
 		return nil
 	}
@@ -41,6 +22,27 @@ func prepareEnv(prefix string, offset int, files []*os.File) []string {
 	}
 }
 
+// Inject files and environment for a subprocess
+func Inject(extras *[]*os.File, env *[]string, prefix string, files ...*os.File) {
+	if len(files) == 0 {
+		return
+	}
+	offset := len(*extras)
+	environ := prepareEnv(prefix, offset, files...)
+	*extras = append(*extras, files...)
+	*env = append(*env, environ...)
+}
+
+// Forward an existing prefix into a subprocesses ExtraFiles and Env
+// parameters.
+func Forward(extras *[]*os.File, env *[]string, prefix string) {
+	files := Load(prefix)
+	if len(files) == 0 {
+		return
+	}
+	Inject(extras, env, prefix, files...)
+}
+
 // Loading extra file descriptors should start at 3 because the first 3 are:
 // stdin (0), stdout (1) and stderr (2).
 const startAt = 3
@@ -65,7 +67,7 @@ func Load(prefix string) []*os.File {
 	var files []*os.File
 	for fd := startAt + offset; fd < startAt+offset+len; fd++ {
 		syscall.CloseOnExec(fd)
-		name := prefix + "_FD_" + strconv.Itoa(fd)
+		name := prefix + "_FD_" + strconv.Itoa(fd-startAt)
 		files = append(files, os.NewFile(uintptr(fd), name))
 	}
 	return files

internal/extrafile/extrafile_test.go

@@ -7,12 +7,14 @@ import (
 	"net"
 	"net/http"
 	"os"
-	"os/exec"
 	"path/filepath"
 	"strings"
 	"testing"
 	"time"
 
+	"github.com/livebud/bud/package/exe"
+	"github.com/livebud/bud/package/js/v8client"
+	"github.com/livebud/bud/package/js/v8server"
 	"golang.org/x/sync/errgroup"
 
 	"github.com/livebud/bud/internal/extrafile"
@@ -23,10 +25,11 @@ import (
 
 func TestNoFiles(t *testing.T) {
 	is := is.New(t)
-	files, env, err := extrafile.Prepare("APP", 0)
-	is.NoErr(err)
-	is.Equal(len(files), 0)
-	is.Equal(len(env), 0)
+	ctx := context.Background()
+	cmd := exe.Command(ctx, "echo")
+	extrafile.Inject(&cmd.ExtraFiles, &cmd.Env, "APP")
+	is.Equal(len(cmd.Env), 0)
+	is.Equal(len(cmd.ExtraFiles), 0)
 }
 
 func listen(addr string) (socket.Listener, *http.Client, error) {
@@ -61,14 +64,6 @@ func TestUnixPassthrough(t *testing.T) {
 		hotSocket, hotClient, err := listen(filepath.Join(dir, "hot.sock"))
 		is.NoErr(err)
 		defer hotSocket.Close()
-		appFiles, appEnv, err := extrafile.Prepare("APP", 0, appSocket)
-		is.NoErr(err)
-		is.Equal(len(appFiles), 1)
-		is.Equal(len(appEnv), 2)
-		hotFiles, hotEnv, err := extrafile.Prepare("HOT", 1, hotSocket)
-		is.NoErr(err)
-		is.Equal(len(hotFiles), 1)
-		is.Equal(len(hotEnv), 2)
 		// Ignore -test.count otherwise this will continue recursively
 		var args []string
 		for _, arg := range os.Args[1:] {
@@ -77,18 +72,20 @@ func TestUnixPassthrough(t *testing.T) {
 			}
 			args = append(args, arg)
 		}
-		cmd := exec.CommandContext(ctx, os.Args[0], append(args, "-test.v=true", "-test.run=^"+t.Name()+"$")...)
+		cmd := exe.Command(ctx, os.Args[0], append(args, "-test.v=true", "-test.run=^"+t.Name()+"$")...)
 		listener, err := socket.Listen(":0")
 		is.NoErr(err)
 		is.Equal(listener.Addr().Network(), "tcp")
 		is.True(strings.HasPrefix(listener.Addr().String(), "127.0.0.1:"))
 		cmd.Stdout = os.Stdout
 		cmd.Stderr = os.Stderr
-		cmd.ExtraFiles = append(cmd.ExtraFiles, appFiles...)
-		cmd.ExtraFiles = append(cmd.ExtraFiles, hotFiles...)
 		cmd.Env = append(os.Environ(), "CHILD=1")
-		cmd.Env = append(cmd.Env, appEnv...)
-		cmd.Env = append(cmd.Env, hotEnv...)
+		appFile, err := appSocket.File()
+		is.NoErr(err)
+		hotFile, err := hotSocket.File()
+		is.NoErr(err)
+		extrafile.Inject(&cmd.ExtraFiles, &cmd.Env, "APP", appFile)
+		extrafile.Inject(&cmd.ExtraFiles, &cmd.Env, "HOT", hotFile)
 		is.NoErr(cmd.Start())
 
 		// Test app socket
@@ -203,14 +200,6 @@ func TestTCPPassthrough(t *testing.T) {
 		hotSocket, hotClient, err := listen(":0")
 		is.NoErr(err)
 		defer hotSocket.Close()
-		appFiles, appEnv, err := extrafile.Prepare("APP", 0, appSocket)
-		is.NoErr(err)
-		is.Equal(len(appFiles), 1)
-		is.Equal(len(appEnv), 2)
-		hotFiles, hotEnv, err := extrafile.Prepare("HOT", 1, hotSocket)
-		is.NoErr(err)
-		is.Equal(len(hotFiles), 1)
-		is.Equal(len(hotEnv), 2)
 		// Ignore -test.count otherwise this will continue recursively
 		var args []string
 		for _, arg := range os.Args[1:] {
@@ -219,18 +208,20 @@ func TestTCPPassthrough(t *testing.T) {
 			}
 			args = append(args, arg)
 		}
-		cmd := exec.CommandContext(ctx, os.Args[0], append(args, "-test.v=true", "-test.run=^"+t.Name()+"$")...)
+		cmd := exe.Command(ctx, os.Args[0], append(args, "-test.v=true", "-test.run=^"+t.Name()+"$")...)
 		listener, err := socket.Listen(":0")
 		is.NoErr(err)
 		is.Equal(listener.Addr().Network(), "tcp")
 		is.True(strings.HasPrefix(listener.Addr().String(), "127.0.0.1:"))
 		cmd.Stdout = os.Stdout
 		cmd.Stderr = os.Stderr
-		cmd.ExtraFiles = append(cmd.ExtraFiles, appFiles...)
-		cmd.ExtraFiles = append(cmd.ExtraFiles, hotFiles...)
 		cmd.Env = append(os.Environ(), "CHILD=1")
-		cmd.Env = append(cmd.Env, appEnv...)
-		cmd.Env = append(cmd.Env, hotEnv...)
+		appFile, err := appSocket.File()
+		is.NoErr(err)
+		hotFile, err := hotSocket.File()
+		is.NoErr(err)
+		extrafile.Inject(&cmd.ExtraFiles, &cmd.Env, "APP", appFile)
+		extrafile.Inject(&cmd.ExtraFiles, &cmd.Env, "HOT", hotFile)
 		is.NoErr(cmd.Start())
 
 		// Test app socket
@@ -332,3 +323,62 @@ func TestTCPPassthrough(t *testing.T) {
 		parent(t)
 	}
 }
+
+func TestV8Passthrough(t *testing.T) {
+	// Parent process
+	parent := func(t testing.TB) {
+		is := is.New(t)
+		ctx, cancel := context.WithCancel(context.Background())
+		defer cancel()
+		dir := t.TempDir()
+		v8server, err := v8server.Pipe()
+		is.NoErr(err)
+		defer v8server.Close()
+		// Ignore -test.count otherwise this will continue recursively
+		var args []string
+		for _, arg := range os.Args[1:] {
+			if strings.HasPrefix(arg, "-test.count=") {
+				continue
+			}
+			args = append(args, arg)
+		}
+		cmd := exe.Command(ctx, os.Args[0], append(args, "-test.v=true", "-test.run=^"+t.Name()+"$")...)
+		cmd.Stdout = os.Stdout
+		cmd.Stderr = os.Stderr
+		cmd.Dir = dir
+		cmd.Env = append(os.Environ(), "CHILD=1")
+		extrafile.Inject(&cmd.ExtraFiles, &cmd.Env, "V8", v8server.Files()...)
+		is.NoErr(cmd.Start())
+		// Create the V8 server
+		eg := new(errgroup.Group)
+		eg.Go(v8server.Serve)
+		// Wait for the command to finish
+		is.NoErr(cmd.Wait())
+		// Restart and ensure the V8 server is ready to serve clients
+		is.NoErr(cmd.Restart(ctx))
+		is.NoErr(cmd.Wait())
+		// Close the V8 server
+		is.NoErr(v8server.Close())
+		is.NoErr(eg.Wait())
+	}
+
+	// Child process
+	child := func(t testing.TB) {
+		is := is.New(t)
+		client, err := v8client.From("V8")
+		is.NoErr(err)
+		result, err := client.Eval("eval.js", "2+2")
+		is.NoErr(err)
+		is.Equal(result, "4")
+		// Test that console.log doesn't mess things up
+		result, err = client.Eval("eval.js", "console.log('hi')")
+		is.NoErr(err)
+		is.Equal(result, "undefined")
+	}
+
+	if value := os.Getenv("CHILD"); value != "" {
+		child(t)
+	} else {
+		parent(t)
+	}
+}