Fix bug in order to make pipenv install deterministic

kardolus · Danny Joyce · kardolus · commit a45d3256ad1f · 2018-11-20T14:23:28.000-05:00
[#161267508]

Co-authored-by: Danny Joyce &lt;djoyce@pivotal.io&gt;
diff --git a/fixtures/flask_python_3_pipenv/Pipfile b/fixtures/flask_python_3_pipenv/Pipfile
@@ -3,11 +3,10 @@ url = "https://pypi.python.org/simple"
 verify_ssl = true
 
 [packages]
-Flask = "==0.10.1"
-gunicorn = "==19.3.0"
+Flask = "==1.0.2"
+gunicorn = "*"
 itsdangerous = "==0.24"
 Jinja2 = "==2.7.2"
-MarkupSafe = "==0.21"
 Werkzeug = "==0.10.4"
 
 [requires]
diff --git a/fixtures/flask_python_3_pipenv/server.py b/fixtures/flask_python_3_pipenv/server.py
@@ -1,5 +1,7 @@
 from flask import Flask, request
 import subprocess
+import gunicorn
+
 
 app = Flask(__name__)
 
@@ -13,4 +15,9 @@ def execute():
         f.write(request.values.get('code'))
     return subprocess.check_output(["python", "runtime.py"])
 
+@app.route('/versions')
+def versions():
+    version = gunicorn.__version__
+    return "Gunicorn version: " + version
+
 app.debug=True
diff --git a/fixtures/flask_python_3_pipenv_vendored_incomplete/Pipfile b/fixtures/flask_python_3_pipenv_vendored_incomplete/Pipfile
@@ -3,7 +3,7 @@ url = "https://pypi.python.org/simple"
 verify_ssl = true
 
 [packages]
-Flask = "==0.10.1"
+Flask = "==1.0.2"
 gunicorn = "==19.3.0"
 itsdangerous = "==0.24"
 Jinja2 = "==2.9.6"
diff --git a/fixtures/flask_python_3_pipenv_vendored_incomplete/Pipfile.lock b/fixtures/flask_python_3_pipenv_vendored_incomplete/Pipfile.lock
diff --git a/src/python/brats/brats_suite_test.go b/src/python/brats/brats_suite_test.go
@@ -77,4 +77,4 @@ func ApiHasStackAssociation() bool {
 	supported, err := cutlass.ApiGreaterThan("2.113.0")
 	Expect(err).NotTo(HaveOccurred())
 	return supported
-}
+}
diff --git a/src/python/hooks/appdynamics_test.go b/src/python/hooks/appdynamics_test.go
@@ -8,11 +8,11 @@ import (
 
 	"path/filepath"
 
+	"fmt"
 	"github.com/cloudfoundry/libbuildpack"
 	"github.com/cloudfoundry/libbuildpack/ansicleaner"
 	. "github.com/onsi/ginkgo"
 	. "github.com/onsi/gomega"
-	"fmt"
 )
 
 func createFile(dir, filename, command string, perm os.FileMode) error {
diff --git a/src/python/integration/deploy_python_app_pipenv_test.go b/src/python/integration/deploy_python_app_pipenv_test.go
@@ -25,15 +25,15 @@ var _ = Describe("deploying a flask web app", func() {
 			PushAppAndConfirm(app)
 		})
 
-		It("gets the python version from pipfile.lock and generates a runtime.txt", func() {
+		It("deploys a pipenv app", func() {
+			By("getting the python version from pipfile.lock")
 			Expect(app.Stdout.String()).To(ContainSubstring("Installing python 3.6."))
-			Expect(app.Stdout.String()).To(ContainSubstring("Installing pipenv"))
 			Expect(app.GetBody("/")).To(ContainSubstring("Hello, World with pipenv!"))
-			Expect(app.Stdout.String()).To(ContainSubstring("Dir checksum unchanged"))
-		})
 
-		It("generates a requirements.txt", func() {
-			Expect(app.Stdout.String()).To(ContainSubstring("Generating 'requirements.txt' with pipenv"))
+			By("generating a requirements.txt without updating the pipfile.lock packages")
+			Expect(app.Stdout.String()).To(ContainSubstring("Generating 'requirements.txt' from Pipfile.lock"))
+			Expect(app.GetBody("/versions")).To(ContainSubstring("Gunicorn version: 19.3.0"))
+			Expect(app.Stdout.String()).To(ContainSubstring("Dir checksum unchanged"))
 		})
 	})
 
diff --git a/src/python/integration/deploy_python_app_with_appdynamics_test.go b/src/python/integration/deploy_python_app_with_appdynamics_test.go
@@ -6,10 +6,10 @@ import (
 
 	"github.com/cloudfoundry/libbuildpack/cutlass"
 
+	"encoding/json"
 	"fmt"
 	. "github.com/onsi/ginkgo"
 	. "github.com/onsi/gomega"
-	"encoding/json"
 )
 
 var _ = Describe("appdynamics", func() {
diff --git a/src/python/integration/fake_supply_before_python_test.go b/src/python/integration/fake_supply_before_python_test.go
@@ -62,17 +62,17 @@ var _ = Describe("running supply buildpacks before the python buildpack", func()
 
 		It("pushes successfully both times", func() {
 			app.Buildpacks = []string{
-					"https://buildpacks.cloudfoundry.org/fixtures/supply-cache-new.zip",
-					"python_buildpack",
-				}
+				"https://buildpacks.cloudfoundry.org/fixtures/supply-cache-new.zip",
+				"python_buildpack",
+			}
 			PushAppAndConfirm(app)
 			Expect(app.GetBody("/")).To(ContainSubstring("Hello, World!"))
 
 			app.Buildpacks = []string{
-					"https://github.com/cloudfoundry/binary-buildpack#develop",
-					"https://buildpacks.cloudfoundry.org/fixtures/supply-cache-new.zip",
-					"python_buildpack",
-				}
+				"https://github.com/cloudfoundry/binary-buildpack#develop",
+				"https://buildpacks.cloudfoundry.org/fixtures/supply-cache-new.zip",
+				"python_buildpack",
+			}
 			PushAppAndConfirm(app)
 			Expect(app.GetBody("/")).To(ContainSubstring("Hello, World!"))
 		})
diff --git a/src/python/supply/supply.go b/src/python/supply/supply.go
@@ -2,6 +2,7 @@ package supply
 
 import (
 	"bytes"
+	"encoding/json"
 	"fmt"
 	"io"
 	"io/ioutil"
@@ -47,14 +48,14 @@ type Command interface {
 }
 
 type Supplier struct {
-	PythonVersion string
-	Manifest      Manifest
-	Installer     Installer
-	Stager        Stager
-	Command       Command
-	Log           *libbuildpack.Logger
-	Logfile       *os.File
-	HasNltkData   bool
+	PythonVersion          string
+	Manifest               Manifest
+	Installer              Installer
+	Stager                 Stager
+	Command                Command
+	Log                    *libbuildpack.Logger
+	Logfile                *os.File
+	HasNltkData            bool
 	removeRequirementsText bool
 }
 
@@ -340,55 +341,103 @@ func (s *Supplier) InstallPipEnv() error {
 	requirementstxtExists, err := libbuildpack.FileExists(filepath.Join(s.Stager.BuildDir(), "requirements.txt"))
 	if err != nil {
 		return err
+	} else if requirementstxtExists {
+		return nil
 	}
 
 	pipfileExists, err := libbuildpack.FileExists(filepath.Join(s.Stager.BuildDir(), "Pipfile"))
 	if err != nil {
 		return err
+	} else if !pipfileExists {
+		return nil
 	}
 
-	if pipfileExists && !requirementstxtExists {
-		s.Log.Info("Installing pipenv")
-		if err := s.Installer.InstallOnlyVersion("pipenv", filepath.Join("/tmp", "pipenv")); err != nil {
-			return err
+	hasLockFile, err := libbuildpack.FileExists(filepath.Join(s.Stager.BuildDir(), "Pipfile.lock"))
+	if err != nil {
+		return fmt.Errorf("could not check Pipfile.lock existence: %v", err)
+	} else if hasLockFile {
+		s.Log.Info("Generating 'requirements.txt' from Pipfile.lock")
+		requirementsContents, err := pipfileToRequirements(filepath.Join(s.Stager.BuildDir(), "Pipfile.lock"))
+		if err != nil {
+			return fmt.Errorf("failed to write `requirement.txt` from Pipfile.lock: %s", err.Error())
 		}
 
-		if err := s.installFfi(); err != nil {
-			return err
-		}
+		return s.writeTempRequirementsTxt(requirementsContents)
+	}
 
-		for _, dep := range []string{"setuptools_scm", "pytest-runner", "parver", "invoke", "pipenv", "wheel"} {
-			s.Log.Info("Installing %s", dep)
-			out := &bytes.Buffer{}
-			stderr := &bytes.Buffer{}
-			if err := s.Command.Execute(s.Stager.BuildDir(), out, stderr, "pip", "install", dep, "--exists-action=w", "--no-index", fmt.Sprintf("--find-links=%s", filepath.Join("/tmp", "pipenv"))); err != nil {
-				return fmt.Errorf("Failed to install %s: %v.\nStdout: %v\nStderr: %v", dep, err, out, stderr)
-			}
+	s.Log.Info("Installing pipenv")
+	if err := s.Installer.InstallOnlyVersion("pipenv", filepath.Join("/tmp", "pipenv")); err != nil {
+		return err
+	}
+
+	if err := s.installFfi(); err != nil {
+		return err
+	}
+
+	for _, dep := range []string{"setuptools_scm", "pytest-runner", "parver", "invoke", "pipenv", "wheel"} {
+		s.Log.Info("Installing %s", dep)
+		out := &bytes.Buffer{}
+		stderr := &bytes.Buffer{}
+		if err := s.Command.Execute(s.Stager.BuildDir(), out, stderr, "pip", "install", dep, "--exists-action=w", "--no-index", fmt.Sprintf("--find-links=%s", filepath.Join("/tmp", "pipenv"))); err != nil {
+			return fmt.Errorf("Failed to install %s: %v.\nStdout: %v\nStderr: %v", dep, err, out, stderr)
 		}
-		s.Stager.LinkDirectoryInDepDir(filepath.Join(s.Stager.DepDir(), "python", "bin"), "bin")
-		s.Log.Info("Generating 'requirements.txt' with pipenv")
+	}
+	s.Stager.LinkDirectoryInDepDir(filepath.Join(s.Stager.DepDir(), "python", "bin"), "bin")
 
-		cmd := exec.Command("pipenv", "lock", "--requirements")
-		cmd.Dir = s.Stager.BuildDir()
-		cmd.Env = append(os.Environ(), "VIRTUALENV_NEVER_DOWNLOAD=true")
-		output, err := s.Command.RunWithOutput(cmd)
-		outputString := string(output)
+	s.Log.Info("Generating 'requirements.txt' with pipenv")
+	cmd := exec.Command("pipenv", "lock", "--requirements")
+	cmd.Dir = s.Stager.BuildDir()
+	cmd.Env = append(os.Environ(), "VIRTUALENV_NEVER_DOWNLOAD=true")
+	output, err := s.Command.RunWithOutput(cmd)
+	if err != nil {
+		return err
+	}
+	outputString := string(output)
 
-		if err != nil {
-			return err
+	// Remove output due to virtualenv
+	if strings.HasPrefix(outputString, "Using ") {
+		reqs := strings.SplitN(outputString, "\n", 2)
+		if len(reqs) > 0 {
+			outputString = reqs[1]
 		}
+	}
 
-		// Remove output due to virtualenv
-		if strings.HasPrefix(outputString, "Using ") {
-			reqs := strings.SplitN(outputString, "\n", 2)
-			if len(reqs) > 0 {
-				outputString = reqs[1]
+	return s.writeTempRequirementsTxt(outputString)
+}
+
+func pipfileToRequirements(lockFilePath string) (string, error) {
+	var lockFile struct {
+		Meta struct {
+			Sources []struct {
+				URL string
 			}
+		} `json:"_meta"`
+		Default map[string]struct {
+			Version string
 		}
+	}
 
-		return s.writeTempRequirementsTxt(outputString)
+	lockContents, err := ioutil.ReadFile(lockFilePath)
+	if err != nil {
+		return "", err
 	}
-	return nil
+
+	err = json.Unmarshal(lockContents, &lockFile)
+	if err != nil {
+		return "", err
+	}
+
+	buf := &bytes.Buffer{}
+
+	for _, source := range lockFile.Meta.Sources {
+		fmt.Fprintf(buf, "-i %s\n", source.URL)
+	}
+
+	for pkg, obj := range lockFile.Default {
+		fmt.Fprintf(buf, "%s%s\n", pkg, obj.Version)
+	}
+
+	return buf.String(), nil
 }
 
 func (s *Supplier) HandlePylibmc() error {
diff --git a/src/python/supply/supply_test.go b/src/python/supply/supply_test.go
@@ -12,7 +12,7 @@ import (
 
 	"github.com/cloudfoundry/libbuildpack"
 	"github.com/cloudfoundry/libbuildpack/ansicleaner"
-	gomock "github.com/golang/mock/gomock"
+	"github.com/golang/mock/gomock"
 	. "github.com/onsi/ginkgo"
 	. "github.com/onsi/gomega"
 )
@@ -201,9 +201,10 @@ var _ = Describe("Supply", func() {
 		BeforeEach(func() {
 			Expect(os.MkdirAll(depDir, 0755)).To(Succeed())
 		})
+
 		Context("when Pipfile.lock and requirements.txt both exist", func() {
 			BeforeEach(func() {
-				Expect(ioutil.WriteFile(filepath.Join(buildDir, "Pipfile"), []byte("This is pipfile"), 0644)).To(Succeed())
+				Expect(ioutil.WriteFile(filepath.Join(buildDir, "Pipfile.lock"), []byte("This is pipfile"), 0644)).To(Succeed())
 				Expect(ioutil.WriteFile(filepath.Join(buildDir, "requirements.txt"), []byte("blah"), 0644)).To(Succeed())
 			})
 
@@ -214,22 +215,27 @@ var _ = Describe("Supply", func() {
 
 		Context("when Pipfile.lock exists but requirements.txt does not exist", func() {
 			BeforeEach(func() {
-				Expect(ioutil.WriteFile(filepath.Join(buildDir, "Pipfile"), []byte("This is pipfile"), 0644)).To(Succeed())
+				const lockFileContnet string = `{"_meta":{"sources":[{"url":"https://pypi.org/simple"}]},"default":{"test":{"version":"==1.2.3"}}}`
+				Expect(ioutil.WriteFile(filepath.Join(buildDir, "Pipfile"), []byte("some content"), 0644)).To(Succeed())
+				Expect(ioutil.WriteFile(filepath.Join(buildDir, "Pipfile.lock"), []byte(lockFileContnet), 0644)).To(Succeed())
 			})
 
-			It("installs pipenv and generates requirements.txt", func() {
-				expectInstallPipEnv()
-
-				mockCommand.EXPECT().RunWithOutput(gomock.Any()).Return([]byte("test"), nil)
-
+			It("manually generates the requirements.txt", func() {
 				Expect(supplier.InstallPipEnv()).To(Succeed())
 
 				requirementsContents, err := ioutil.ReadFile(filepath.Join(buildDir, "requirements.txt"))
 				Expect(err).ToNot(HaveOccurred())
-				Expect(requirementsContents).To(ContainSubstring("test"))
+				Expect(requirementsContents).To(ContainSubstring("-i https://pypi.org/simple"))
+				Expect(requirementsContents).To(ContainSubstring("test==1.2.3"))
 			})
+		})
 
-			It("removes extraneous pipenv lock output", func() {
+		Context("when Pipfile exists but requirements.txt and Pipfile.lock do not exist", func() {
+			BeforeEach(func() {
+				Expect(ioutil.WriteFile(filepath.Join(buildDir, "Pipfile"), []byte("some content"), 0644)).To(Succeed())
+			})
+
+			It("manually generates the requirements.txt", func() {
 				expectInstallPipEnv()
 
 				mockCommand.EXPECT().RunWithOutput(gomock.Any()).Return([]byte("Using /tmp/deps/0/bin/python3.6m to create virtualenv…\nline 1\nline 2\n"), nil)
@@ -238,6 +244,8 @@ var _ = Describe("Supply", func() {
 
 				requirementsContents, err := ioutil.ReadFile(filepath.Join(buildDir, "requirements.txt"))
 				Expect(err).ToNot(HaveOccurred())
+
+				By("removes extraneous pipenv lock output")
 				Expect(string(requirementsContents)).To(Equal("line 1\nline 2\n"))
 			})
 		})

Original file line number	Diff line number	Diff line change
`@@ -6,10 +6,10 @@ import (`
`6`	`6`
`7`	`7`	`"github.com/cloudfoundry/libbuildpack/cutlass"`
`8`	`8`
	`9`	`+ "encoding/json"`
`9`	`10`	`"fmt"`
`10`	`11`	`. "github.com/onsi/ginkgo"`
`11`	`12`	`. "github.com/onsi/gomega"`
`12`		`- "encoding/json"`
`13`	`13`	`)`
`14`	`14`
`15`	`15`	`var _ = Describe("appdynamics", func() {`