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 <[email protected]>
Date: Thursday, March 19, 2020 at 12:44 PM
To: Ashley E Desimone <[email protected]>, "Desimone, Nathaniel L"
<[email protected]>, "[email protected]" <[email protected]>
Cc: "Pandya, Puja" <[email protected]>, "Bjorge, Erik C"
<[email protected]>
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:[email protected]>
Sent: Thursday, March 19, 2020 12:32 PM
To: Desimone, Nathaniel L<mailto:[email protected]>;
[email protected]<mailto:[email protected]>
Cc: Pandya, Puja<mailto:[email protected]>; Bjorge, Erik
C<mailto:[email protected]>; Bret
Barkelew<mailto:[email protected]>
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 <[email protected]>
Sent: Wednesday, March 18, 2020 8:17 PM
To: [email protected]
Cc: Desimone, Ashley E <[email protected]>; Pandya, Puja
<[email protected]>; Bjorge, Erik C <[email protected]>; Bret
Barkelew <[email protected]>
Subject: [PATCH 1/4] EdkRepo: Installer should remove obsolete dependencies
Cc: Ashley DeSimone <[email protected]>
Cc: Puja Pandya <[email protected]>
Cc: Erik Bjorge <[email protected]>
Cc: Bret Barkelew <[email protected]>
Signed-off-by: Nate DeSimone <[email protected]>
---
.../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 (#56037): https://edk2.groups.io/g/devel/message/56037
Mute This Topic: https://groups.io/mt/72067009/21656
Group Owner: [email protected]
Unsubscribe: https://edk2.groups.io/g/devel/unsub [[email protected]]
-=-=-=-=-=-=-=-=-=-=-=-
