Add a pip install step

Summary:
We want to also install the wheels, not just generate them. To do that
we add a pip install -t <target> command. For this to work however, we
need to install into /opt/lib/python and amend the environment
accordingly to set PYTHONPATH and PATH in order for the libraries and
binaries to be accessible.

Reviewers: dduvall, thcipriani, #release-engineering-team

Tags: #release-engineering-team

Differential Revision: https://phabricator.wikimedia.org/D1057

2 files changed, 74 insertions, 64 deletions

diff --git a/config/python.go b/config/python.go
index 4e2f2ce..f63ce25 100644
--- a/config/python.go
+++ b/config/python.go
@@ -7,9 +7,17 @@ import (
 	"phabricator.wikimedia.org/source/blubber/build"
 )
 
-// PythonLocalLibPrefix is the path to installed dependency wheels.
+// PythonLibPrefix is the path to installed dependency wheels.
 //
-const PythonLocalLibPrefix = LocalLibPrefix + "/python"
+const PythonLibPrefix = LocalLibPrefix + "/python"
+
+// PythonSitePackages is the path to installed Python packages.
+//
+const PythonSitePackages = PythonLibPrefix + "/site-packages"
+
+// PythonSiteBin is the path to installed Python packages bin files.
+//
+const PythonSiteBin = PythonSitePackages + "/bin"
 
 // PythonConfig holds configuration fields related to pre-installation of project
 // dependencies via PIP.
@@ -61,58 +69,68 @@ func (pc *PythonConfig) Merge(pc2 PythonConfig) {
 // network requests from said commands.
 //
 func (pc PythonConfig) InstructionsForPhase(phase build.Phase) []build.Instruction {
-	if pc.Requirements != nil || pc.Version != "" {
+	if pc.Version != "" {
 		switch phase {
 		case build.PhasePrivileged:
-			return []build.Instruction{
-				build.RunAll{[]build.Run{
-					{pc.version(), []string{"-m", "easy_install", "pip"}},
-					{pc.version(), []string{"-m", "pip", "install", "-U", "setuptools", "wheel", "tox"}},
-				}},
+			if pc.Requirements != nil {
+				return []build.Instruction{
+					build.RunAll{[]build.Run{
+						{pc.version(), []string{"-m", "easy_install", "pip"}},
+						{pc.version(), []string{"-m", "pip", "install", "-U", "setuptools", "wheel", "tox"}},
+					}},
+				}
 			}
 
 		case build.PhasePreInstall:
-			envs := build.Env{map[string]string{
-				"PIP_WHEEL_DIR":  PythonLocalLibPrefix,
-				"PIP_FIND_LINKS": "file://" + PythonLocalLibPrefix,
-			}}
+			if pc.Requirements != nil {
+				envs := build.Env{map[string]string{
+					"PIP_WHEEL_DIR":  PythonLibPrefix,
+					"PIP_FIND_LINKS": "file://" + PythonLibPrefix,
+				}}
+
+				mkdirs := build.RunAll{
+					Runs: []build.Run{
+						build.CreateDirectory(PythonLibPrefix),
+					},
+				}
 
-			mkdirs := build.RunAll{
-				Runs: []build.Run{
-					build.CreateDirectory(PythonLocalLibPrefix),
-				},
-			}
+				dirs, bydir := pc.RequirementsByDir()
+				copies := make([]build.Instruction, len(dirs))
 
-			dirs, bydir := pc.RequirementsByDir()
-			copies := make([]build.Instruction, len(dirs))
+				// make project subdirectories for requirements files if necessary, and
+				// copy in requirements files
+				for i, dir := range dirs {
+					if dir != "./" {
+						mkdirs.Runs = append(mkdirs.Runs, build.CreateDirectory(dir))
+					}
 
-			// make project subdirectories for requirements files if necessary, and
-			// copy in requirements files
-			for i, dir := range dirs {
-				if dir != "./" {
-					mkdirs.Runs = append(mkdirs.Runs, build.CreateDirectory(dir))
+					copies[i] = build.Copy{bydir[dir], dir}
 				}
 
-				copies[i] = build.Copy{bydir[dir], dir}
-			}
+				ins := []build.Instruction{envs, mkdirs}
+				ins = append(ins, copies...)
 
-			ins := []build.Instruction{envs, mkdirs}
-			ins = append(ins, copies...)
+				if args := pc.RequirementsArgs(); len(args) > 0 {
+					ins = append(ins, build.RunAll{[]build.Run{
+						{pc.version(), append([]string{"-m", "pip", "wheel"}, args...)},
+						{pc.version(), append([]string{"-m", "pip", "install", "--target", PythonSitePackages}, args...)},
+					}})
+				}
 
-			if args := pc.RequirementsArgs(); len(args) > 0 {
-				ins = append(ins, build.Run{
-					pc.version(), append([]string{"-m", "pip", "wheel"}, args...),
-				})
+				return ins
 			}
 
-			return ins
-
 		case build.PhasePostInstall:
-			return []build.Instruction{
-				build.Env{map[string]string{
-					"PIP_NO_INDEX": "1",
-				}},
+			env := build.Env{map[string]string{
+				"PYTHONPATH": PythonSitePackages,
+				"PATH":       PythonSiteBin + ":${PATH}",
+			}}
+
+			if pc.Requirements != nil {
+				env.Definitions["PIP_NO_INDEX"] = "1"
 			}
+
+			return []build.Instruction{env}
 		}
 	}
 
diff --git a/config/python_test.go b/config/python_test.go
index 5c07644..ee4dfc0 100644
--- a/config/python_test.go
+++ b/config/python_test.go
@@ -85,15 +85,7 @@ func TestPythonConfigInstructionsNoRequirementsWithVersion(t *testing.T) {
 	}
 
 	t.Run("PhasePrivileged", func(t *testing.T) {
-		assert.Equal(t,
-			[]build.Instruction{
-				build.RunAll{[]build.Run{
-					{"python2.7", []string{"-m", "easy_install", "pip"}},
-					{"python2.7", []string{"-m", "pip", "install", "-U", "setuptools", "wheel", "tox"}},
-				}},
-			},
-			cfg.InstructionsForPhase(build.PhasePrivileged),
-		)
+		assert.Empty(t, cfg.InstructionsForPhase(build.PhasePrivileged))
 	})
 
 	t.Run("PhasePrivilegeDropped", func(t *testing.T) {
@@ -101,25 +93,15 @@ func TestPythonConfigInstructionsNoRequirementsWithVersion(t *testing.T) {
 	})
 
 	t.Run("PhasePreInstall", func(t *testing.T) {
-		assert.Equal(t,
-			[]build.Instruction{
-				build.Env{map[string]string{
-					"PIP_WHEEL_DIR":  "/opt/lib/python",
-					"PIP_FIND_LINKS": "file:///opt/lib/python",
-				}},
-				build.RunAll{[]build.Run{
-					{"mkdir -p", []string{"/opt/lib/python"}},
-				}},
-			},
-			cfg.InstructionsForPhase(build.PhasePreInstall),
-		)
+		assert.Empty(t, cfg.InstructionsForPhase(build.PhasePreInstall))
 	})
 
 	t.Run("PhasePostInstall", func(t *testing.T) {
 		assert.Equal(t,
 			[]build.Instruction{
 				build.Env{map[string]string{
-					"PIP_NO_INDEX": "1",
+					"PYTHONPATH": "/opt/lib/python/site-packages",
+					"PATH":       "/opt/lib/python/site-packages/bin:${PATH}",
 				}},
 			},
 			cfg.InstructionsForPhase(build.PhasePostInstall),
@@ -182,10 +164,18 @@ func TestPythonConfigInstructionsWithRequirements(t *testing.T) {
 				}},
 				build.Copy{[]string{"requirements.txt", "requirements-test.txt"}, "./"},
 				build.Copy{[]string{"docs/requirements.txt"}, "docs/"},
-				build.Run{"python2.7", []string{"-m", "pip", "wheel",
-					"-r", "requirements.txt",
-					"-r", "requirements-test.txt",
-					"-r", "docs/requirements.txt",
+				build.RunAll{[]build.Run{
+					{"python2.7", []string{"-m", "pip", "wheel",
+						"-r", "requirements.txt",
+						"-r", "requirements-test.txt",
+						"-r", "docs/requirements.txt",
+					}},
+					{"python2.7", []string{"-m", "pip", "install",
+						"--target", "/opt/lib/python/site-packages",
+						"-r", "requirements.txt",
+						"-r", "requirements-test.txt",
+						"-r", "docs/requirements.txt",
+					}},
 				}},
 			},
 			cfg.InstructionsForPhase(build.PhasePreInstall),
@@ -197,6 +187,8 @@ func TestPythonConfigInstructionsWithRequirements(t *testing.T) {
 			[]build.Instruction{
 				build.Env{map[string]string{
 					"PIP_NO_INDEX": "1",
+					"PYTHONPATH":   "/opt/lib/python/site-packages",
+					"PATH":         "/opt/lib/python/site-packages/bin:${PATH}",
 				}},
 			},
 			cfg.InstructionsForPhase(build.PhasePostInstall),