Re: [PATCH 1/4] EdkRepo: Installer should remove obsolete dependencies

["Desimone, Ashley E" <ashley.e.desimone@intel.com>]

For the following section why are we singling out gitdb2 and smmap2 when deleting obsolete dependencies? Is there an alternative to hardcoding these values?

+                if (DeleteObsoletePackages)
+                {
+                    //
+                    // Delete obsolete dependencies
+                    //
+                    foreach (string PackageName in new string[] { "smmap2", "gitdb2" })
+                    {
+                        if (InstalledPackages.Where(p => p.Name == PackageName).FirstOrDefault() != null)
+                        {
+                            InstallLogger.Log(string.Format("Uninstalling {0}", PackageName));
+                            PythonOperations.UninstallPythonPackage(PythonPath, PackageName);
+                        }
+                    }
+                }

Thanks,
Ashley

-----Original Message-----
From: Desimone, Nathaniel L <nathaniel.l.desimone@intel.com> 
Sent: Wednesday, March 18, 2020 8:17 PM
To: devel@edk2.groups.io
Cc: Desimone, Ashley E <ashley.e.desimone@intel.com>; Pandya, Puja <puja.pandya@intel.com>; Bjorge, Erik C <erik.c.bjorge@intel.com>; Bret Barkelew <Bret.Barkelew@microsoft.com>
Subject: [PATCH 1/4] EdkRepo: Installer should remove obsolete dependencies

Cc: Ashley DeSimone <ashley.e.desimone@intel.com>
Cc: Puja Pandya <puja.pandya@intel.com>
Cc: Erik Bjorge <erik.c.bjorge@intel.com>
Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>
Signed-off-by: Nate DeSimone <nathaniel.l.desimone@intel.com>
---
 .../EdkRepoInstaller/InstallWorker.cs         | 30 +++++++++-
 edkrepo_installer/linux-scripts/install.py    | 58 ++++++++++++-------
 2 files changed, 66 insertions(+), 22 deletions(-)

diff --git a/edkrepo_installer/EdkRepoInstaller/InstallWorker.cs b/edkrepo_installer/EdkRepoInstaller/InstallWorker.cs
index 8d824f2..b44e8fa 100644
--- a/edkrepo_installer/EdkRepoInstaller/InstallWorker.cs
+++ b/edkrepo_installer/EdkRepoInstaller/InstallWorker.cs
@@ -710,6 +710,7 @@ namespace TianoCore.EdkRepoInstaller
                 string PythonPath;
                 bool Has32Bit;
                 bool Has64Bit;
+                bool DeleteObsoletePackages = false;
                 PythonOperations.GetPythonBitness(PyInstance.Version.Major, PyInstance.Version.Minor, out Has32Bit, out Has64Bit);
                 if (Has32Bit && Has64Bit && PyInstance.Architecture == CpuArchitecture.IA32)
                 {
@@ -720,14 +721,39 @@ namespace TianoCore.EdkRepoInstaller
                     PythonPath = PythonOperations.FindPython(PyInstance.Version.Major, PyInstance.Version.Minor, false);
                 }
                 List<PythonPackage> InstalledPackages = PythonOperations.GetInstalledPythonPackages(PythonPath);
-                foreach(PythonWheel Wheel in PyInstance.Wheels)
+                foreach (PythonWheel Wheel in PyInstance.Wheels)
+                {
+                    //If a package is already installed, check if we have a newer version bundled in the installer
+                    //If yes, the package will be upgraded, make sure obsolete packages are uninstalled first
+                    PythonPackage InstalledPackage = InstalledPackages.Where(p => p.Name == Wheel.Package.Name.Replace('_', '-')).FirstOrDefault();
+                    if (InstalledPackage != null && InstalledPackage.Version < Wheel.Package.Version)
+                    {
+                        DeleteObsoletePackages = true;
+                        break;
+                    }
+                }
+                if (DeleteObsoletePackages)
+                {
+                    //
+                    // Delete obsolete dependencies
+                    //
+                    foreach (string PackageName in new string[] { "smmap2", "gitdb2" })
+                    {
+                        if (InstalledPackages.Where(p => p.Name == PackageName).FirstOrDefault() != null)
+                        {
+                            InstallLogger.Log(string.Format("Uninstalling {0}", PackageName));
+                            PythonOperations.UninstallPythonPackage(PythonPath, PackageName);
+                        }
+                    }
+                }
+                foreach (PythonWheel Wheel in PyInstance.Wheels)
                 {
                     //PythonPackage InstalledPackage = (from package in InstalledPackages
                     //                                  where package.Name == Wheel.Package.Name
                     //                                  select package).FirstOrDefault();
                     //If the package is already installed, check if we have a newer version bundled in the installer
                     //If so, upgrade the package
-                    PythonPackage InstalledPackage = InstalledPackages.Where(p => p.Name == Wheel.Package.Name).FirstOrDefault();
+                    PythonPackage InstalledPackage = 
+ InstalledPackages.Where(p => p.Name == Wheel.Package.Name.Replace('_', 
+ '-')).FirstOrDefault();
                     if (InstalledPackage != null)
                     {
                         if (InstalledPackage.Version < Wheel.Package.Version) diff --git a/edkrepo_installer/linux-scripts/install.py b/edkrepo_installer/linux-scripts/install.py
index 0ea486d..b2cdfed 100755
--- a/edkrepo_installer/linux-scripts/install.py
+++ b/edkrepo_installer/linux-scripts/install.py
@@ -3,7 +3,7 @@
 ## @file
 # install.py
 #
-# Copyright (c) 2018 - 2019, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2018 - 2020, Intel Corporation. All rights 
+reserved.<BR>
 # SPDX-License-Identifier: BSD-2-Clause-Patent  #
 
@@ -53,6 +53,26 @@ def get_args():
     parser.add_argument('-v', '--verbose', action='store_true', default=False, help='Enables verbose output')
     return parser.parse_args()
 
+def get_installed_packages(python_command):
+    pip_cmd = [def_python, '-m', 'pip', 'list', '--legacy']
+    try:
+        res = default_run(pip_cmd)
+    except:
+        pip_cmd.pop(-1)
+        try:
+            res = default_run(pip_cmd)
+        except:
+            return ret_val
+    installed_packages = {}
+    for pip_mod in res.stdout.split('\n'):
+        try:
+            name, version = pip_mod.split()
+            version = version.strip().strip('()')
+        except:
+            continue
+        installed_packages[name] = version
+    return installed_packages
+
 def get_required_wheels():
     ret_val = collections.OrderedDict()
     if platform.machine() == 'x86_64':
@@ -79,27 +99,11 @@ def get_required_wheels():
                                                  'version':wheel.attrib['Version'],
                                                  'install':True}
             break
-    pip_cmd = [def_python, '-m', 'pip', 'list', '--legacy']
-    try:
-        res = default_run(pip_cmd)
-    except:
-        pip_cmd.pop(-1)
-        try:
-            res = default_run(pip_cmd)
-        except:
-            return ret_val
-    installed_modules = {}
-    for pip_mod in res.stdout.split('\n'):
-        try:
-            name, version = pip_mod.split()
-            version = version.strip().strip('()')
-        except:
-            continue
-        installed_modules[name] = version
+    installed_packages = get_installed_packages(def_python)
     for name in ret_val:
         #pip doesn't understand the difference between '_' and '-'
-        if name.replace('_','-') in installed_modules:
-            version = installed_modules[name.replace('_','-')]
+        if name.replace('_','-') in installed_packages:
+            version = installed_packages[name.replace('_','-')]
             if _check_version(version, ret_val[name]['version']) >= 0 and not ret_val[name]['uninstall']:
                 ret_val[name]['install'] = False
             else:
@@ -328,16 +332,30 @@ def do_install():
         if not os.path.isdir(whl_src_dir):
             log.info('- Missing wheel file directory')
             return 1
+        updating_edkrepo = False
         for whl_name in wheels_to_install:
             uninstall_whl = wheels_to_install[whl_name]['uninstall']
             whl_name = whl_name.replace('_','-')  #pip doesn't understand the difference between '_' and '-'
             if uninstall_whl:
+                updating_edkrepo = True
                 try:
                     res = default_run([def_python, '-m', 'pip', 'uninstall', '--yes', whl_name])
                 except:
                     log.info('- Failed to uninstall {}'.format(whl_name))
                     return 1
                 log.info('+ Uninstalled {}'.format(whl_name))
+
+        #Delete obsolete dependencies
+        if updating_edkrepo:
+            installed_packages = get_installed_packages(def_python)
+            for whl_name in ['smmap2', 'gitdb2']:
+                if whl_name in installed_packages:
+                    try:
+                        res = default_run([def_python, '-m', 'pip', 'uninstall', '--yes', whl_name])
+                    except:
+                        log.info('- Failed to uninstall {}'.format(whl_name))
+                        return 1
+                    log.info('+ Uninstalled {}'.format(whl_name))
         for whl_name in wheels_to_install:
             whl = wheels_to_install[whl_name]['wheel']
             install_whl = wheels_to_install[whl_name]['install']
--
2.24.0.windows.2