vpython: Add Python interpreter handling package.

vpython/python/interpreter.go

Index: vpython/python/interpreter.go
diff --git a/vpython/python/interpreter.go b/vpython/python/interpreter.go
new file mode 100644
index 0000000000000000000000000000000000000000..30c8e3968d8998b10c4174c49d70faef40d8ef3e
--- /dev/null
+++ b/vpython/python/interpreter.go
@@ -0,0 +1,180 @@
+// Copyright 2017 The LUCI Authors. All rights reserved.
+// Use of this source code is governed under the Apache License, Version 2.0
+// that can be found in the LICENSE file.
+
+package python
+
+import (
+	"os"
+	"os/exec"
+	"strings"
+	"sync"
+
+	"github.com/luci/luci-go/common/errors"
+	"github.com/luci/luci-go/common/logging"
+	"github.com/luci/luci-go/common/system/exitcode"
+
+	"golang.org/x/net/context"
+)
+
+type runnerFunc func(cmd *exec.Cmd, capture bool) (string, error)
+
+// Interpreter is a configured Python interpreter.

[iannucci, 2017/02/21 10:08:16]

maybe

Interpreter is a runnable Python interpreter binary found on the system. It can
be queried for it's version. Obtain one using the Find method.

The current comment isn't useful.

[dnj, 2017/02/21 23:21:35]

Done.

+type Interpreter struct {
+	// Python is the path to the Python interpreter.

[iannucci, 2017/02/21 10:08:16]

absolute?

[dnj, 2017/02/21 23:21:35]

Done.

+	Python string
+
+	// WorkDir is the working directory to use.

[iannucci, 2017/02/21 10:08:16]

use for what?

[dnj, 2017/02/21 23:21:35]

Done.

+	WorkDir string
+
+	// ConnectSTDIN, if true, says that STDIN should also be connected.

[iannucci, 2017/02/21 10:08:16]

ConnectSTDIN will cause stdin to be passed through. Otherwise it will be closed.

[dnj, 2017/02/21 23:21:35]

Done.

+	ConnectSTDIN bool
+
+	// Isolated means that the Python invocation should include flags to isolate
+	// it from local system modification.

[iannucci, 2017/02/21 10:08:16]

Like what? "Local system modification"? From the code, I would say this should
be something like


Isolated should be set to avoid picking up environmental queues (e.g.
PYTHONPATH, site.py), and avoid modifying the local filesystem (e.g. writing
.pyo files)

[dnj, 2017/02/21 23:21:35]

Done.

+	Isolated bool
+
+	// Env, if not nil, is the environment to supply.
+	Env []string
+
+	// runner is the runner function to use to run an exec.Cmd. This can be
+	// swapped out for testing.
+	runner runnerFunc

[iannucci, 2017/02/21 10:08:16]

lets prefix all testing members with 'testing'

[dnj, 2017/02/21 23:21:35]

Done.

+
+	// cachedVersion is the cached Version for this interpreter. It is populated
+	// on the first GetVersion call.
+	cachedVersion   *Version
+	cachedVersionMu sync.Mutex

[iannucci, 2017/02/21 10:08:16]

sync.Once?

[dnj, 2017/02/21 23:21:35]

Because there can be an error return code, I would prefer to use a Mutex.

+}
+
+func (i *Interpreter) getRunnerFunc() runnerFunc {
+	if i.runner != nil {
+		return i.runner
+	}
+
+	// Return default runner (actually run the process).
+	return func(cmd *exec.Cmd, capture bool) (string, error) {
+		// If we're capturing output, combine STDOUT and STDERR (see GetVersion
+		// for details).
+		if capture {
+			out, err := cmd.CombinedOutput()
+			if err != nil {
+				return "", errors.Annotate(err).Err()
+			}
+			return string(out), nil
+		}
+
+		// Non-capturing run.
+		if err := cmd.Run(); err != nil {
+			return "", errors.Annotate(err).Err()
+		}
+		return "", nil
+	}
+}
+
+// Run runs the configured Interpreter with the supplied arguments.

[iannucci, 2017/02/21 10:08:16]

Run executes the python interpreter with the supplied arguments.

[dnj, 2017/02/21 23:21:35]

I split this off into "Command()" (returns a bound command) and "Run()" (which
runs the configured command).

+//
+// Run returns wrapped errors. Use errors.Unwrap to get the main cause, if
+// needed. If an error occurs during setup or invocation, it will be returned
+// directly. If the interpreter runs and returns zero, nil will be returned. If
+// the interpreter runs and returns non-zero, an Error instance will be returned
+// containing that return code.
+func (i *Interpreter) Run(c context.Context, args ...string) error {
+	if i.Python == "" {
+		return errors.New("a Python interpreter must be supplied")

[iannucci, 2017/02/21 10:08:16]

please avoid passive voice everywhere

[dnj, 2017/02/21 23:21:35]

Done.

+	}
+
+	if i.Isolated {
+		args = append([]string{
+			"-B", // Don't compile "pyo" binaries.
+			"-E", // Don't use PYTHON* enviornment variables.
+			"-s", // Don't use user 'site.py'.
+		}, args...)
+	}
+
+	cmd := exec.CommandContext(c, i.Python, args...)
+	cmd.Stdout = os.Stdout
+	cmd.Stderr = os.Stderr
+	if i.ConnectSTDIN {
+		cmd.Stdin = os.Stdin
+	}
+	cmd.Dir = i.WorkDir
+	cmd.Env = i.Env
+
+	if logging.IsLogging(c, logging.Debug) {
+		logging.Debugf(c, "Running Python command (cwd=%s): %s",
+			cmd.Dir, strings.Join(cmd.Args, " "))
+	}
+
+	// Allow testing to supply an alternative runner function.
+	rf := i.getRunnerFunc()
+	if _, err := rf(cmd, false); err != nil {
+		// If the process failed because of a non-zero return value, return that
+		// as our error.
+		if rc, has := exitcode.Get(err); has {
+			return errors.Annotate(Error(rc)).Reason("Python bootstrap returned non-zero").Err()
+		}
+
+		return errors.Annotate(err).Reason("failed to run Python command").
+			D("python", i.Python).
+			D("args", args).
+			Err()
+	}
+	return nil
+}
+
+// GetVersion runs the specified Python interpreter with the "--version"
+// flag and maps it to a known specification verison.
+func (i *Interpreter) GetVersion(c context.Context) (v Version, err error) {
+	if i.Python == "" {
+		err = errors.New("a Python interpreter must be supplied")
+		return
+	}
+
+	i.cachedVersionMu.Lock()
+	defer i.cachedVersionMu.Unlock()
+
+	// Check again, under write-lock.
+	if i.cachedVersion != nil {
+		v = *i.cachedVersion
+		return
+	}
+
+	// We use CombinedOutput here becuase Python2 writes the version to STDERR,
+	// while Python3+ writes it to STDOUT.
+	cmd := exec.CommandContext(c, i.Python, "--version")
+
+	rf := i.getRunnerFunc()

[iannucci, 2017/02/21 10:08:16]

why not `i.run(cmd, capture)`? I think that might be clearer than returning a
function which you immediately invoke once.

[dnj, 2017/02/21 23:21:35]

Changed to a Command struct.

+	out, err := rf(cmd, true)
+	if err != nil {
+		err = errors.Annotate(err).Reason("failed to get Python version").Err()
+		return
+	}
+	if v, err = parseVersionOutput(strings.TrimSpace(out)); err != nil {
+		err = errors.Annotate(err).Err()
+		return
+	}
+
+	i.cachedVersion = &v
+	return
+}
+
+func parseVersionOutput(output string) (Version, error) {
+	// Expected output:
+	// Python X.Y.Z
+	parts := strings.SplitN(output, " ", 2)
+	if len(parts) != 2 || parts[0] != "Python" {
+		return Version{}, errors.Reason("unknown version output").
+			D("output", output).
+			Err()
+	}
+
+	v, err := ParseVersion(parts[1])
+	if err != nil {
+		err = errors.Annotate(err).Reason("failed to parse version from: %(value)q").
+			D("value", parts[1]).
+			Err()
+		return v, err
+	}
+	return v, nil
+}