[edk2-devel] [PATCH 1/4] EdkRepo: Installer should remove obsolete dependencies
Bret Barkelew via Groups.Io
bret.barkelew=microsoft.com at groups.io
Fri Mar 20 22:06:50 UTC 2020
Spiffy. Agreed; thanks for the background.
My only other concern would be the function name and if some future developer thought that was permission to add other modules to it.
Then again, maybe I’m just bikeshedding. Feel free to ignore.
- Bret
From: Desimone, Ashley E<mailto:ashley.e.desimone at intel.com>
Sent: Friday, March 20, 2020 1:21 PM
To: Desimone, Nathaniel L<mailto:nathaniel.l.desimone at intel.com>; Bret Barkelew<mailto:Bret.Barkelew at microsoft.com>; devel at edk2.groups.io<mailto:devel at edk2.groups.io>
Cc: Pandya, Puja<mailto:puja.pandya at intel.com>; Bjorge, Erik C<mailto:erik.c.bjorge at intel.com>
Subject: [EXTERNAL] RE: [PATCH 1/4] EdkRepo: Installer should remove obsolete dependencies
Thanks for the explanation.
Reviewed-by: Ashley DeSimone <ashley.e.deismone at intel.com>
From: Desimone, Nathaniel L <nathaniel.l.desimone at intel.com>
Sent: Thursday, March 19, 2020 2:01 PM
To: Bret Barkelew <Bret.Barkelew at microsoft.com>; Desimone, Ashley E <ashley.e.desimone at intel.com>; devel at edk2.groups.io
Cc: Pandya, Puja <puja.pandya at intel.com>; Bjorge, Erik C <erik.c.bjorge at intel.com>
Subject: Re: [PATCH 1/4] EdkRepo: Installer should remove obsolete dependencies
Hi Bret,
I alluded to this a little bit in the cover letter, completely agreed a more detailed explanation is needed.
There is an odd situation with the way GitPython’s dependencies are evolved over time. Initially, they had the following dependency graph:
* GitPython
* gitdb
* smmap
At some point in the past, they decided to make backwards incompatible API changes to gitdb and smmap, so the dependencies where updated as follows:
* GitPython
* gitdb2
* smmap2
However, now that Python 2.x is deprecated, any code that was using the old API will need updates anyway. So, the GitPython maintainers decided to go back to the original dependency tree. The issue is, gitdb2 and smmap2 wheels create packages in site-packages with the names gitdb and smmap, so installing gitdb2 and gitdb at the same time results in the gitdb folder in site-packages being overwritten. End result pretty crappy, whichever wheel was most recently installed is the one that will prevail in site-packages.
The way the GitPython maintainers addressed this problem is by converting gitdb2 and smmap2 into wheels with no content other than a dependency on gitdb and smmap. So when one does a pip upgrade those packages remain and you get a dependency graph that looks like this:
* GitPython
* gitdb2
* smmap2
* gitdb2
* gitdb
* smmap
* smmap2
* smmap
So, the net result for a regular pip upgrade leaves a fair amount of detritus sitting on the system which would not be present for a new user doing a clean install from pip. Obviously, we can do better than that which is why I’ve automated the uninstallation of smmap2 and gitdb2 before proceeding to upgrade GitPython. This also guarantees that the duplicate folder in site-packages situation does not happen at the same time.
The fact that the GitPython maintainers created this situation in the first place is rather annoying.
Thanks,
Nate
From: Bret Barkelew <Bret.Barkelew at microsoft.com<mailto:Bret.Barkelew at microsoft.com>>
Date: Thursday, March 19, 2020 at 12:44 PM
To: Ashley E Desimone <ashley.e.desimone at intel.com<mailto:ashley.e.desimone at intel.com>>, "Desimone, Nathaniel L" <nathaniel.l.desimone at intel.com<mailto:nathaniel.l.desimone at intel.com>>, "devel at edk2.groups.io<mailto:devel at edk2.groups.io>" <devel at edk2.groups.io<mailto:devel at edk2.groups.io>>
Cc: "Pandya, Puja" <puja.pandya at intel.com<mailto:puja.pandya at intel.com>>, "Bjorge, Erik C" <erik.c.bjorge at intel.com<mailto:erik.c.bjorge at intel.com>>
Subject: RE: [PATCH 1/4] EdkRepo: Installer should remove obsolete dependencies
Naïve question: why isn’t this upgrade handled when pip installs the new wheel? Are the dependencies not matched up correctly? I would think that pip would uninstall the old version on its own.
- Bret
From: Desimone, Ashley E<mailto:ashley.e.desimone at intel.com>
Sent: Thursday, March 19, 2020 12:32 PM
To: Desimone, Nathaniel L<mailto:nathaniel.l.desimone at intel.com>; devel at edk2.groups.io<mailto:devel at edk2.groups.io>
Cc: Pandya, Puja<mailto:puja.pandya at intel.com>; Bjorge, Erik C<mailto:erik.c.bjorge at intel.com>; Bret Barkelew<mailto:Bret.Barkelew at microsoft.com>
Subject: [EXTERNAL] RE: [PATCH 1/4] EdkRepo: Installer should remove obsolete dependencies
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 at intel.com<mailto:nathaniel.l.desimone at intel.com>>
Sent: Wednesday, March 18, 2020 8:17 PM
To: devel at edk2.groups.io<mailto:devel at edk2.groups.io>
Cc: Desimone, Ashley E <ashley.e.desimone at intel.com<mailto:ashley.e.desimone at intel.com>>; Pandya, Puja <puja.pandya at intel.com<mailto:puja.pandya at intel.com>>; Bjorge, Erik C <erik.c.bjorge at intel.com<mailto:erik.c.bjorge at intel.com>>; Bret Barkelew <Bret.Barkelew at microsoft.com<mailto:Bret.Barkelew at microsoft.com>>
Subject: [PATCH 1/4] EdkRepo: Installer should remove obsolete dependencies
Cc: Ashley DeSimone <ashley.e.desimone at intel.com<mailto:ashley.e.desimone at intel.com>>
Cc: Puja Pandya <puja.pandya at intel.com<mailto:puja.pandya at intel.com>>
Cc: Erik Bjorge <erik.c.bjorge at intel.com<mailto:erik.c.bjorge at intel.com>>
Cc: Bret Barkelew <Bret.Barkelew at microsoft.com<mailto:Bret.Barkelew at microsoft.com>>
Signed-off-by: Nate DeSimone <nathaniel.l.desimone at intel.com<mailto:nathaniel.l.desimone at 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
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#56060): https://edk2.groups.io/g/devel/message/56060
Mute This Topic: https://groups.io/mt/72067009/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/edk2-devel-archive/attachments/20200320/657a6dc7/attachment.htm>
More information about the edk2-devel-archive
mailing list