From 409477b951c62dd475913c88f91b11559554400c Mon Sep 17 00:00:00 2001 From: Matt Ellis Date: Mon, 30 Apr 2018 16:10:01 -0700 Subject: [PATCH] Invoke node directly from the language host Instead of using a shell script to jump from the language host into node, just invoke node directly. This makes our start-up path a little simpler to understand and indirectly fixes pulumi/home#156, where we would fail on Windows if the `-exec` script was in a folder that had spaces in it (due to a subtle interaction between how go launches cmd files and how cmd.exe parses arguments). --- build.proj | 20 ++--- scripts/make_release.ps1 | 1 - scripts/make_release.sh | 1 - sdk/nodejs/Makefile | 1 - sdk/nodejs/cmd/pulumi-language-nodejs/main.go | 86 +++++++++++-------- sdk/nodejs/dist/pulumi-language-nodejs-exec | 12 --- .../dist/pulumi-language-nodejs-exec-test | 2 - .../dist/pulumi-language-nodejs-exec.cmd | 10 --- sdk/nodejs/tests/runtime/langhost/run.spec.ts | 35 +++----- 9 files changed, 66 insertions(+), 102 deletions(-) delete mode 100755 sdk/nodejs/dist/pulumi-language-nodejs-exec delete mode 100755 sdk/nodejs/dist/pulumi-language-nodejs-exec-test delete mode 100644 sdk/nodejs/dist/pulumi-language-nodejs-exec.cmd diff --git a/build.proj b/build.proj index 8fc70edac..0032dc6c7 100644 --- a/build.proj +++ b/build.proj @@ -22,9 +22,9 @@ - - - + + + @@ -33,8 +33,8 @@ - - + + @@ -75,7 +75,6 @@ - @@ -106,11 +105,6 @@ - - - - @@ -118,11 +112,11 @@ - + + Condition="$(WhereLangHostExitCode) != 0 Or $(WhereDynamicProviderExitCode) != 0"/> diff --git a/scripts/make_release.ps1 b/scripts/make_release.ps1 index 0df0f04d6..7f38d198f 100644 --- a/scripts/make_release.ps1 +++ b/scripts/make_release.ps1 @@ -32,7 +32,6 @@ RunGoBuild "github.com/pulumi/pulumi" RunGoBuild "github.com/pulumi/pulumi/sdk/nodejs/cmd/pulumi-language-nodejs" CopyPackage "$Root\sdk\nodejs\bin" "pulumi" -Copy-Item "$Root\sdk\nodejs\dist\pulumi-language-nodejs-exec.cmd" "$PublishDir\bin" Copy-Item "$Root\sdk\nodejs\dist\pulumi-resource-pulumi-nodejs.cmd" "$PublishDir\bin" New-Item -ItemType Directory -Path "$PublishDir\bin\v6.10.2" | Out-Null diff --git a/scripts/make_release.sh b/scripts/make_release.sh index e98281fa0..9b9b3e7b2 100755 --- a/scripts/make_release.sh +++ b/scripts/make_release.sh @@ -49,7 +49,6 @@ run_go_build "${ROOT}/sdk/nodejs/cmd/pulumi-language-nodejs" run_go_build "${ROOT}/sdk/python/cmd/pulumi-language-python" # Copy over the language and dynamic resource providers. -cp ${ROOT}/sdk/nodejs/dist/pulumi-language-nodejs-exec ${PUBDIR}/bin/ cp ${ROOT}/sdk/nodejs/dist/pulumi-resource-pulumi-nodejs ${PUBDIR}/bin/ cp ${ROOT}/sdk/python/cmd/pulumi-language-python-exec ${PUBDIR}/bin/ diff --git a/sdk/nodejs/Makefile b/sdk/nodejs/Makefile index 58953e0e3..4a1ee1ce3 100644 --- a/sdk/nodejs/Makefile +++ b/sdk/nodejs/Makefile @@ -36,7 +36,6 @@ build:: install:: GOBIN=$(PULUMI_BIN) go install -ldflags "-X github.com/pulumi/pulumi/pkg/version.Version=${VERSION}" ${LANGUAGE_HOST} - cp dist/pulumi-language-nodejs-exec "$(PULUMI_BIN)" cp dist/pulumi-resource-pulumi-nodejs "$(PULUMI_BIN)" mkdir -p "$(PULUMI_BIN)/v6.10.2" cp ./prebuilt/*.node "$(PULUMI_BIN)/v6.10.2/" diff --git a/sdk/nodejs/cmd/pulumi-language-nodejs/main.go b/sdk/nodejs/cmd/pulumi-language-nodejs/main.go index a376cbf03..2235f5432 100644 --- a/sdk/nodejs/cmd/pulumi-language-nodejs/main.go +++ b/sdk/nodejs/cmd/pulumi-language-nodejs/main.go @@ -39,9 +39,9 @@ import ( ) const ( - // By convention, the executor is the name of the current program - // (pulumi-language-nodejs) plus this suffix. - nodeExecSuffix = "-exec" // the exec shim for Pulumi to run Node programs. + // The path to the "run" program which will spawn the rest of the language host. This may be overriden with + // PULUMI_LANGUAGE_NODEJS_RUN_PATH, which we do in some testing cases. + defaultRunPath = "./node_modules/@pulumi/pulumi/cmd/run" // The runtime expects the config object to be saved to this environment variable. pulumiConfigVar = "PULUMI_CONFIG" @@ -52,40 +52,27 @@ const ( // endpoint. func main() { var tracing string - var givenExecutor string flag.StringVar(&tracing, "tracing", "", "Emit tracing to a Zipkin-compatible tracing endpoint") - // You can use the below flag to request that the language host load - // a specific executor instead of probing the PATH. This is used specifically - // in run.spec.ts to work around some unfortunate Node module loading behavior. - flag.StringVar(&givenExecutor, "use-executor", "", - "Use the given program as the executor instead of looking for one on PATH") - flag.Parse() args := flag.Args() cmdutil.InitLogging(false, 0, false) cmdutil.InitTracing(os.Args[0], tracing) - var nodeExec string - if givenExecutor == "" { - // The -exec binary is the same name as the current language host, except that we must trim off - // the file extension (if any) and then append -exec to it. - bin := os.Args[0] - if ext := filepath.Ext(bin); ext != "" { - bin = bin[:len(bin)-len(ext)] - } - bin += nodeExecSuffix - pathExec, err := exec.LookPath(bin) - if err != nil { - err = errors.Wrapf(err, "could not find `%s` on the $PATH", bin) - cmdutil.Exit(err) - } - glog.V(3).Infof("language host identified executor from path: `%s`", pathExec) - nodeExec = pathExec - } else { - glog.V(3).Infof("language host asked to use specific executor: `%s`", givenExecutor) - nodeExec = givenExecutor + nodePath, err := exec.LookPath("node") + if err != nil { + cmdutil.Exit(errors.Wrapf(err, "could not find node on the $PATH")) + } + + runPath := os.Getenv("PULUMI_LANGUAGE_NODEJS_RUN_PATH") + if runPath == "" { + runPath = defaultRunPath + } + + if _, err = os.Stat(runPath); err != nil { + cmdutil.ExitError( + "It looks like the Pulumi SDK has not been installed. Have you run npm install or yarn install?") } // Optionally pluck out the engine so we can do logging, etc. @@ -97,7 +84,7 @@ func main() { // Fire up a gRPC server, letting the kernel choose a free port. port, done, err := rpcutil.Serve(0, nil, []func(*grpc.Server) error{ func(srv *grpc.Server) error { - host := newLanguageHost(nodeExec, engineAddress, tracing) + host := newLanguageHost(nodePath, runPath, engineAddress, tracing) pulumirpc.RegisterLanguageRuntimeServer(srv, host) return nil }, @@ -118,14 +105,16 @@ func main() { // nodeLanguageHost implements the LanguageRuntimeServer interface // for use as an API endpoint. type nodeLanguageHost struct { - exec string + nodeBin string + runPath string engineAddress string tracing string } -func newLanguageHost(exec, engineAddress, tracing string) pulumirpc.LanguageRuntimeServer { +func newLanguageHost(nodePath, runPath, engineAddress, tracing string) pulumirpc.LanguageRuntimeServer { return &nodeLanguageHost{ - exec: exec, + nodeBin: nodePath, + runPath: runPath, engineAddress: engineAddress, tracing: tracing, } @@ -263,17 +252,40 @@ func (host *nodeLanguageHost) Run(ctx context.Context, req *pulumirpc.RunRequest return nil, err } + ourCmd, err := os.Executable() + if err != nil { + err = errors.Wrap(err, "failed to find our working directory") + return nil, err + } + + // Older versions of the pulumi runtime used a custom node module (which only worked on node 6.10.X) to support + // closure serialization. While we no longer use this, we continue to ship this module with the language host in + // the SDK, so we can deploy programs using older versions of the Pulumi framework. So, for now, let's add this + // folder with our native modules to the NODE_PATH so Node can find it. + // + // TODO(ellismg)[pulumi/pulumi#1298]: Remove this block of code when we no longer need to support older + // @pulumi/pulumi versions. + env := os.Environ() + existingNodePath := os.Getenv("NODE_PATH") + if existingNodePath != "" { + env = append(env, fmt.Sprintf("NODE_PATH=%s/v6.10.2:%s", filepath.Dir(ourCmd), existingNodePath)) + } else { + env = append(env, "NODE_PATH="+filepath.Dir(ourCmd)+"/v6.10.2") + } + + env = append(env, pulumiConfigVar+"="+string(config)) + if glog.V(5) { commandStr := strings.Join(args, " ") - glog.V(5).Infoln("Language host launching process: ", host.exec, commandStr) + glog.V(5).Infoln("Language host launching process: ", host.nodeBin, commandStr) } // Now simply spawn a process to execute the requested program, wiring up stdout/stderr directly. var errResult string - cmd := exec.Command(host.exec, args...) // nolint: gas, intentionally running dynamic program name. + cmd := exec.Command(host.nodeBin, args...) // nolint: gas, intentionally running dynamic program name. cmd.Stdout = os.Stdout cmd.Stderr = os.Stderr - cmd.Env = append(os.Environ(), pulumiConfigVar+"="+string(config)) + cmd.Env = env if err := cmd.Run(); err != nil { if exiterr, ok := err.(*exec.ExitError); ok { // If the program ran, but exited with a non-zero error code. This will happen often, since user @@ -299,7 +311,7 @@ func (host *nodeLanguageHost) Run(ctx context.Context, req *pulumirpc.RunRequest // by enumerating all of the optional and non-optional arguments present // in a RunRequest. func (host *nodeLanguageHost) constructArguments(req *pulumirpc.RunRequest) []string { - var args []string + args := []string{host.runPath} maybeAppendArg := func(k, v string) { if v != "" { args = append(args, "--"+k, v) diff --git a/sdk/nodejs/dist/pulumi-language-nodejs-exec b/sdk/nodejs/dist/pulumi-language-nodejs-exec deleted file mode 100755 index 15c0d4794..000000000 --- a/sdk/nodejs/dist/pulumi-language-nodejs-exec +++ /dev/null @@ -1,12 +0,0 @@ -#!/bin/sh -# we exploit the fact that the cwd when `pulumi-language-nodejs-exec` is -# run is the root of the node program we want to run and use a relative -# path here. -export NODE_PATH="$NODE_PATH:`dirname $0`/v6.10.2" -PULUMI_RUN=./node_modules/@pulumi/pulumi/cmd/run -if [ ! -e $PULUMI_RUN ]; then - echo "It looks like the Pulumi SDK has not been installed. Have you run npm install or yarn install?" - exit 1 -fi - -node $PULUMI_RUN $@ diff --git a/sdk/nodejs/dist/pulumi-language-nodejs-exec-test b/sdk/nodejs/dist/pulumi-language-nodejs-exec-test deleted file mode 100755 index 3b4401517..000000000 --- a/sdk/nodejs/dist/pulumi-language-nodejs-exec-test +++ /dev/null @@ -1,2 +0,0 @@ -#!/bin/sh -node ./bin/cmd/run $@ diff --git a/sdk/nodejs/dist/pulumi-language-nodejs-exec.cmd b/sdk/nodejs/dist/pulumi-language-nodejs-exec.cmd deleted file mode 100644 index c3120079b..000000000 --- a/sdk/nodejs/dist/pulumi-language-nodejs-exec.cmd +++ /dev/null @@ -1,10 +0,0 @@ -@echo off - -set NODE_PATH=%NODE_PATH%;%~dp0\v6.10.2 -set PULUMI_RUN=./node_modules/@pulumi/pulumi/cmd/run -if not exist %PULUMI_RUN% ( - echo It looks like the Pulumi SDK has not been installed. Have you run npm install or yarn install? - exit /b 1 -) - -node %PULUMI_RUN% %* diff --git a/sdk/nodejs/tests/runtime/langhost/run.spec.ts b/sdk/nodejs/tests/runtime/langhost/run.spec.ts index ff2cc1088..eb6e5372a 100644 --- a/sdk/nodejs/tests/runtime/langhost/run.spec.ts +++ b/sdk/nodejs/tests/runtime/langhost/run.spec.ts @@ -544,32 +544,17 @@ function createMockResourceMonitor( function serveLanguageHostProcess(): { proc: childProcess.ChildProcess, addr: Promise } { // A quick note about this: // - // Normally, pulumi-language-nodejs probes the path in order to - // find the nodejs executor, pulumi-language-nodejs-exec. This works - // great in all scenarios other than testing within this file. If the executor - // that it founds resides in the Pulumi install dir (which it will, if these tests - // are being executed by `make`), then Node will execute it by resolving our relative - // path requires to the Pulumi install directory. However, the programs being evaluated - // by the language host are using the current directory to resolve relative path requires. + // Normally, `pulumi-language-nodejs` launches `./node-modules/@pulumi/pulumi/cmd/run` which is responsible + // for setting up some state and then running the actual user program. However, in this case, we don't + // have a folder structure like the above because we are seting the package as we've built it, not it installed + // in another application. // - // Normally, this isn't a problem - ostensibly the stuff in the Pulumi install directory - // is the same as the stuff that we are currently testing. However, the "settings.ts" module - // contains some global state that is expected to be shared between the language host stub - // that is connecting to our RPC endpoints (run/index.ts) and the Pulumi program being - // evaluated. Node, when resolving the require, will pick different modules to load depending - // on which context the require occured; if it happened in run/index.ts, it'll load - // from Pulumi install directory, while requires coming from anywhere else will load - // from the source directory. Because these are two different files, Node instantiates two - // separate module objects and the state that we are expecting to share is not actually shared, - // ultimately resulting in extremely wacky errors. - // - // In order to work around this problem, the langhost is explicitly instructed - // (through --use-executor) to use a specific executor which will load modules from - // the source directory and not the install directory. - const proc = childProcess.spawn("pulumi-language-nodejs", [ - "--use-executor", - path.join(__filename, "..", "..", "..", "..", "pulumi-language-nodejs-exec-test"), - ]); + // `pulumi-language-nodejs` allows us to set `PULUMI_LANGUAGE_NODEJS_RUN_PATH` in the environment, and when + // set, it will use that path instead of the default value. For our tests here, we set it and point at the + // just built version of run. + process.env.PULUMI_LANGUAGE_NODEJS_RUN_PATH = "./bin/cmd/run"; + const proc = childProcess.spawn("pulumi-language-nodejs"); + // Hook the first line so we can parse the address. Then we hook the rest to print for debugging purposes, and // hand back the resulting process object plus the address we plucked out. let addrResolve: ((addr: string) => void) | undefined;