From dee6dafa3a5a751e62a10530c8fb3118600029d0 Mon Sep 17 00:00:00 2001
From: Arnaud Chong <a.chong@criteo.com>
Date: Tue, 27 Dec 2022 17:51:49 +0100
Subject: [PATCH] Handle better pip changes

* pip version was not used to converge the install resource
* when python was installed without pip, the resource did nothing, pip
  was not installed
* some code cleanup in the library
* make test cookbook to depend on the tested cookbook

Change-Id: Ide1bdab796c8b5608f37cb273f7535652d70025d
---
 Policyfile.rb                           |  2 +-
 libraries/helpers.rb                    | 37 ++++++++++++-------------
 resources/install.rb                    | 17 +++++++++---
 test/cookbooks/python3-test/metadata.rb |  2 ++
 4 files changed, 34 insertions(+), 24 deletions(-)

diff --git a/Policyfile.rb b/Policyfile.rb
index 673e0f9..a76d128 100644
--- a/Policyfile.rb
+++ b/Policyfile.rb
@@ -2,7 +2,7 @@
 
 run_list ['python3']
 
-named_run_list :test, run_list + ['recipe[python3-test]']
+named_run_list :test, ['recipe[python3-test]']
 
 cookbook 'python3', path: '.'
 cookbook 'python3-test', path: 'test/cookbooks/python3-test'
diff --git a/libraries/helpers.rb b/libraries/helpers.rb
index 41ec296..d2cd62a 100644
--- a/libraries/helpers.rb
+++ b/libraries/helpers.rb
@@ -13,23 +13,19 @@ def self.python_binary(resource)
     def self.python_path(resource = new_resource)
       return ::File.join(resource.virtualenv, 'bin/python3') if resource.respond_to?(:virtualenv) && !resource.virtualenv.nil?
 
-      if resource.python_provider == 'portable_pypy3'
-        "/usr/local/pypy#{resource.python_version}"
+      if (pypy = pypy_version(resource))
+        "/usr/local/pypy#{pypy}"
       else
         '/usr/local/'
       end
     end
 
     def self.python_system_path(resource = new_resource)
-      if resource.respond_to?(:python_provider)
-        provider = resource.python_provider
-        version = resource.python_version
+      if (pypy = pypy_version(resource))
+        ::File.join("/usr/local/pypy#{pypy}")
       else
-        provider = resource.source
-        version = resource.version
+        '/usr'
       end
-
-      provider == 'portable_pypy3' ? ::File.join("/usr/local/pypy#{version}") : '/usr'
     end
 
     def self.virtualenv(resource = new_resource)
@@ -41,23 +37,26 @@ def self.virtualenv(resource = new_resource)
     end
 
     def self.pip_path(resource = new_resource, system: false)
-      path = if resource.respond_to?(:virtualenv) && !resource.virtualenv.nil? && !system
-               resource.virtualenv
-             elsif resource.python_provider == 'portable_pypy3'
-               "/usr/local/pypy#{resource.python_version}"
-             else
-               '/usr/local/'
-             end
-
-      ::File.join(path)
+      if resource.respond_to?(:virtualenv) && !resource.virtualenv.nil? && !system
+        resource.virtualenv
+      elsif (pypy = pypy_version(resource))
+        "/usr/local/pypy#{pypy}"
+      else
+        '/usr/local/'
+      end
     end
 
     def self.pip_binary(resource = new_resource)
-      ::File.join(pip_path(resource), 'bin/pip3')
+      ::File.join(pip_path(resource, system: false), 'bin', resource.pip_binary_name)
     end
 
     def self.virtualenv_binary(resource = new_resource)
       ::File.join(pip_path(resource, system: true), 'bin/virtualenv')
     end
+
+    def self.pypy_version(resource = new_resource)
+      return resource.version if resource.respond_to?(:source) && resource.source == 'portable_pypy3'
+      return resource.python_version if resource.respond_to?(:python_provider) && resource.python_provider == 'portable_pypy3'
+    end
   end
 end
diff --git a/resources/install.rb b/resources/install.rb
index 9fe6d2a..7f88cd8 100644
--- a/resources/install.rb
+++ b/resources/install.rb
@@ -16,19 +16,28 @@
 
 load_current_value do |new_resource|
   python_binary = ::Python3::Path.python_binary(new_resource)
+  pip_binary = ::Python3::Path.pip_binary(new_resource)
 
-  current_value_does_not_exist! unless ::File.exist?(python_binary)
+  current_value_does_not_exist! unless ::File.exist?(python_binary) && ::File.exist?(pip_binary)
 
   if new_resource.source != 'portable_pypy3'
-    pyversion = ::Mixlib::ShellOut.new("#{python_binary} --version").run_command.stdout.match('\s+(^[0-9\.]+)')&.last_match(1)
-    version pyversion if new_resource.version
+    python_version = pyversion(python_binary)
+    pipv = pyversion(pip_binary)
+
+    version python_version if new_resource.version
+    pip_version pipv if new_resource.pip_version
   else
     version new_resource.version
+    pip_version new_resource.pip_version
   end
 end
 
+def pyversion(binary)
+  return Regexp.last_match(1) if ::Mixlib::ShellOut.new("#{binary} --version").run_command.stdout.match('\s+([0-9\.]+)')
+end
+
 action :install do
-  converge_if_changed :version do
+  converge_if_changed :version, :pip_version do
     valid_name_regex = /^python3\.?\d?$/
     if new_resource.source == 'portable_pypy3'
       package 'bzip2'
diff --git a/test/cookbooks/python3-test/metadata.rb b/test/cookbooks/python3-test/metadata.rb
index 184f014..a601760 100644
--- a/test/cookbooks/python3-test/metadata.rb
+++ b/test/cookbooks/python3-test/metadata.rb
@@ -9,3 +9,5 @@
 version          '0.1.0'
 supports         'centos'
 supports         'windows'
+
+depends          'python3'