From 7a60cf468d7993ee63a541dd03440c9309e805b7 Mon Sep 17 00:00:00 2001 From: Kim-Adeline Miguel Date: Tue, 2 Nov 2021 14:42:57 -0700 Subject: [PATCH 1/5] Do not process system Python 2 on macOS Monterey. --- news/2 Fixes/17870.md | 1 + .../lowLevel/posixKnownPathsLocator.ts | 24 ++++++++- .../posixKnownPathsLocator.unit.test.ts | 53 ++++++++++++++++++- 3 files changed, 76 insertions(+), 2 deletions(-) create mode 100644 news/2 Fixes/17870.md diff --git a/news/2 Fixes/17870.md b/news/2 Fixes/17870.md new file mode 100644 index 000000000000..a43aba3398bb --- /dev/null +++ b/news/2 Fixes/17870.md @@ -0,0 +1 @@ +Do not process system Python 2 installs on macOS Monterey. diff --git a/src/client/pythonEnvironments/base/locators/lowLevel/posixKnownPathsLocator.ts b/src/client/pythonEnvironments/base/locators/lowLevel/posixKnownPathsLocator.ts index 74070386355f..a1e91d0a3d95 100644 --- a/src/client/pythonEnvironments/base/locators/lowLevel/posixKnownPathsLocator.ts +++ b/src/client/pythonEnvironments/base/locators/lowLevel/posixKnownPathsLocator.ts @@ -1,22 +1,44 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. +import * as os from 'os'; +import { gte } from 'semver'; import { traceError } from '../../../../common/logger'; import { PythonEnvKind, PythonEnvSource } from '../../info'; import { BasicEnvInfo, IPythonEnvsIterator, Locator } from '../../locator'; import { commonPosixBinPaths, getPythonBinFromPosixPaths } from '../../../common/posixUtils'; import { isPyenvShimDir } from '../../../common/environmentManagers/pyenv'; +import { getOSType, OSType } from '../../../../common/utils/platform'; export class PosixKnownPathsLocator extends Locator { private kind: PythonEnvKind = PythonEnvKind.OtherGlobal; public iterEnvs(): IPythonEnvsIterator { + // Flag to remove system installs of Python 2 from the list of discovered interpreters + // If on macOS Monterey or later. + let isMacPython2Deprecated = false; + if (getOSType() === OSType.OSX && gte(os.release(), '21.0.0')) { + isMacPython2Deprecated = true; + } + const iterator = async function* (kind: PythonEnvKind) { // Filter out pyenv shims. They are not actual python binaries, they are used to launch // the binaries specified in .python-version file in the cwd. We should not be reporting // those binaries as environments. const knownDirs = (await commonPosixBinPaths()).filter((dirname) => !isPyenvShimDir(dirname)); - const pythonBinaries = await getPythonBinFromPosixPaths(knownDirs); + let pythonBinaries = await getPythonBinFromPosixPaths(knownDirs); + + // Filter out MacOS system installs of Python 2 if necessary. + if (isMacPython2Deprecated) { + pythonBinaries = pythonBinaries.filter((binary) => { + if (binary.startsWith('/usr/bin/python')) { + return binary.startsWith('/usr/bin/python3'); + } + + return true; + }); + } + for (const bin of pythonBinaries) { try { yield { executablePath: bin, kind, source: [PythonEnvSource.PathEnvVar] }; diff --git a/src/test/pythonEnvironments/base/locators/lowLevel/posixKnownPathsLocator.unit.test.ts b/src/test/pythonEnvironments/base/locators/lowLevel/posixKnownPathsLocator.unit.test.ts index 3dd97884f9a5..38640dd80400 100644 --- a/src/test/pythonEnvironments/base/locators/lowLevel/posixKnownPathsLocator.unit.test.ts +++ b/src/test/pythonEnvironments/base/locators/lowLevel/posixKnownPathsLocator.unit.test.ts @@ -1,9 +1,12 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. +import * as assert from 'assert'; import * as path from 'path'; import * as sinon from 'sinon'; +import * as semver from 'semver'; import * as executablesAPI from '../../../../../client/common/utils/exec'; +import * as osUtils from '../../../../../client/common/utils/platform'; import { PythonEnvKind, PythonEnvSource } from '../../../../../client/pythonEnvironments/base/info'; import { BasicEnvInfo } from '../../../../../client/pythonEnvironments/base/locator'; import { getEnvs } from '../../../../../client/pythonEnvironments/base/locatorUtils'; @@ -33,7 +36,7 @@ suite('Posix Known Path Locator', () => { locator = new PosixKnownPathsLocator(); }); teardown(() => { - getPathEnvVar.restore(); + sinon.restore(); }); test('iterEnvs(): get python bin from known test roots', async () => { @@ -56,4 +59,52 @@ suite('Posix Known Path Locator', () => { const actualEnvs = (await getEnvs(locator.iterEnvs())).filter((e) => e.executablePath.indexOf('posixroot') > 0); assertBasicEnvsEqual(actualEnvs, expectedEnvs); }); + + test('iterEnvs(): Do not return Python 2 installs when on macOS Monterey', async function () { + if (osUtils.getOSType() === osUtils.OSType.Windows) { + this.skip(); + } + + const getOSTypeStub = sinon.stub(osUtils, 'getOSType'); + const gteStub = sinon.stub(semver, 'gte'); + + getOSTypeStub.returns(osUtils.OSType.OSX); + gteStub.returns(true); + + const actualEnvs = await getEnvs(locator.iterEnvs()); + + const globalPython2Envs = actualEnvs.filter((env) => { + if (env.executablePath.startsWith('/usr/bin/python')) { + return !env.executablePath.startsWith('/usr/bin/python3'); + } + + return false; + }); + + assert.strictEqual(globalPython2Envs.length, 0); + }); + + test('iterEnvs(): Return Python 2 installs when not macOS Monterey', async function () { + if (osUtils.getOSType() === osUtils.OSType.Windows) { + this.skip(); + } + + const getOSTypeStub = sinon.stub(osUtils, 'getOSType'); + const gteStub = sinon.stub(semver, 'gte'); + + getOSTypeStub.returns(osUtils.OSType.OSX); + gteStub.returns(false); + + const actualEnvs = await getEnvs(locator.iterEnvs()); + + const globalPython2Envs = actualEnvs.filter((env) => { + if (env.executablePath.startsWith('/usr/bin/python')) { + return !env.executablePath.startsWith('/usr/bin/python3'); + } + + return false; + }); + + assert.notStrictEqual(globalPython2Envs.length, 0); + }); }); From 1755b50cb6c4566034adfbb753f8a76a1780f11e Mon Sep 17 00:00:00 2001 From: Kim-Adeline Miguel Date: Tue, 2 Nov 2021 14:46:43 -0700 Subject: [PATCH 2/5] Missing word --- .../base/locators/lowLevel/posixKnownPathsLocator.unit.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/pythonEnvironments/base/locators/lowLevel/posixKnownPathsLocator.unit.test.ts b/src/test/pythonEnvironments/base/locators/lowLevel/posixKnownPathsLocator.unit.test.ts index 38640dd80400..62ab65a86f48 100644 --- a/src/test/pythonEnvironments/base/locators/lowLevel/posixKnownPathsLocator.unit.test.ts +++ b/src/test/pythonEnvironments/base/locators/lowLevel/posixKnownPathsLocator.unit.test.ts @@ -84,7 +84,7 @@ suite('Posix Known Path Locator', () => { assert.strictEqual(globalPython2Envs.length, 0); }); - test('iterEnvs(): Return Python 2 installs when not macOS Monterey', async function () { + test('iterEnvs(): Return Python 2 installs when not on macOS Monterey', async function () { if (osUtils.getOSType() === osUtils.OSType.Windows) { this.skip(); } From ef8dd02219f71adea15961334bf86b780f48a9f5 Mon Sep 17 00:00:00 2001 From: Kim-Adeline Miguel Date: Wed, 3 Nov 2021 08:51:14 -0700 Subject: [PATCH 3/5] Update isMacDefaultPythonPath --- .../base/locators/lowLevel/macDefaultLocator.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/client/pythonEnvironments/base/locators/lowLevel/macDefaultLocator.ts b/src/client/pythonEnvironments/base/locators/lowLevel/macDefaultLocator.ts index 4204eccb7411..9baf6d2affd3 100644 --- a/src/client/pythonEnvironments/base/locators/lowLevel/macDefaultLocator.ts +++ b/src/client/pythonEnvironments/base/locators/lowLevel/macDefaultLocator.ts @@ -12,5 +12,8 @@ export function isMacDefaultPythonPath(pythonPath: string): boolean { if (getOSType() !== OSType.OSX) { return false; } - return pythonPath === 'python' || pythonPath === '/usr/bin/python'; + + const defaultPaths = ['python', '/usr/bin/python']; + + return defaultPaths.includes(pythonPath) || pythonPath.startsWith('/usr/bin/python2'); } From b438320ddf7fe95c6b867418a2b3b8457803126c Mon Sep 17 00:00:00 2001 From: Kim-Adeline Miguel Date: Wed, 3 Nov 2021 08:51:29 -0700 Subject: [PATCH 4/5] Remove code in posix locator --- .../lowLevel/posixKnownPathsLocator.ts | 10 +++------ .../posixKnownPathsLocator.unit.test.ts | 21 +++++-------------- 2 files changed, 8 insertions(+), 23 deletions(-) diff --git a/src/client/pythonEnvironments/base/locators/lowLevel/posixKnownPathsLocator.ts b/src/client/pythonEnvironments/base/locators/lowLevel/posixKnownPathsLocator.ts index a1e91d0a3d95..1b945c7617ad 100644 --- a/src/client/pythonEnvironments/base/locators/lowLevel/posixKnownPathsLocator.ts +++ b/src/client/pythonEnvironments/base/locators/lowLevel/posixKnownPathsLocator.ts @@ -9,6 +9,7 @@ import { BasicEnvInfo, IPythonEnvsIterator, Locator } from '../../locator'; import { commonPosixBinPaths, getPythonBinFromPosixPaths } from '../../../common/posixUtils'; import { isPyenvShimDir } from '../../../common/environmentManagers/pyenv'; import { getOSType, OSType } from '../../../../common/utils/platform'; +import { isMacDefaultPythonPath } from './macDefaultLocator'; export class PosixKnownPathsLocator extends Locator { private kind: PythonEnvKind = PythonEnvKind.OtherGlobal; @@ -16,6 +17,7 @@ export class PosixKnownPathsLocator extends Locator { public iterEnvs(): IPythonEnvsIterator { // Flag to remove system installs of Python 2 from the list of discovered interpreters // If on macOS Monterey or later. + // See https://github.com/microsoft/vscode-python/issues/17870. let isMacPython2Deprecated = false; if (getOSType() === OSType.OSX && gte(os.release(), '21.0.0')) { isMacPython2Deprecated = true; @@ -30,13 +32,7 @@ export class PosixKnownPathsLocator extends Locator { // Filter out MacOS system installs of Python 2 if necessary. if (isMacPython2Deprecated) { - pythonBinaries = pythonBinaries.filter((binary) => { - if (binary.startsWith('/usr/bin/python')) { - return binary.startsWith('/usr/bin/python3'); - } - - return true; - }); + pythonBinaries = pythonBinaries.filter((binary) => !isMacDefaultPythonPath(binary)); } for (const bin of pythonBinaries) { diff --git a/src/test/pythonEnvironments/base/locators/lowLevel/posixKnownPathsLocator.unit.test.ts b/src/test/pythonEnvironments/base/locators/lowLevel/posixKnownPathsLocator.unit.test.ts index 62ab65a86f48..89ab4402b3b6 100644 --- a/src/test/pythonEnvironments/base/locators/lowLevel/posixKnownPathsLocator.unit.test.ts +++ b/src/test/pythonEnvironments/base/locators/lowLevel/posixKnownPathsLocator.unit.test.ts @@ -14,6 +14,7 @@ import { PosixKnownPathsLocator } from '../../../../../client/pythonEnvironments import { createBasicEnv } from '../../common'; import { TEST_LAYOUT_ROOT } from '../../../common/commonTestConstants'; import { assertBasicEnvsEqual } from '../envTestUtils'; +import { isMacDefaultPythonPath } from '../../../../../client/pythonEnvironments/base/locators/lowLevel/macDefaultLocator'; suite('Posix Known Path Locator', () => { let getPathEnvVar: sinon.SinonStub; @@ -61,7 +62,7 @@ suite('Posix Known Path Locator', () => { }); test('iterEnvs(): Do not return Python 2 installs when on macOS Monterey', async function () { - if (osUtils.getOSType() === osUtils.OSType.Windows) { + if (osUtils.getOSType() !== osUtils.OSType.OSX) { this.skip(); } @@ -73,19 +74,13 @@ suite('Posix Known Path Locator', () => { const actualEnvs = await getEnvs(locator.iterEnvs()); - const globalPython2Envs = actualEnvs.filter((env) => { - if (env.executablePath.startsWith('/usr/bin/python')) { - return !env.executablePath.startsWith('/usr/bin/python3'); - } - - return false; - }); + const globalPython2Envs = actualEnvs.filter((env) => isMacDefaultPythonPath(env.executablePath)); assert.strictEqual(globalPython2Envs.length, 0); }); test('iterEnvs(): Return Python 2 installs when not on macOS Monterey', async function () { - if (osUtils.getOSType() === osUtils.OSType.Windows) { + if (osUtils.getOSType() !== osUtils.OSType.OSX) { this.skip(); } @@ -97,13 +92,7 @@ suite('Posix Known Path Locator', () => { const actualEnvs = await getEnvs(locator.iterEnvs()); - const globalPython2Envs = actualEnvs.filter((env) => { - if (env.executablePath.startsWith('/usr/bin/python')) { - return !env.executablePath.startsWith('/usr/bin/python3'); - } - - return false; - }); + const globalPython2Envs = actualEnvs.filter((env) => isMacDefaultPythonPath(env.executablePath)); assert.notStrictEqual(globalPython2Envs.length, 0); }); From db995e4428b24836af17ac749df228ca0cf9a9f0 Mon Sep 17 00:00:00 2001 From: Kim-Adeline Miguel Date: Wed, 3 Nov 2021 09:11:00 -0700 Subject: [PATCH 5/5] Add isMacDefaultPythonPath tests --- .../lowLevel/macDefaultLocator.unit.test.ts | 44 +++++++++++++++++++ 1 file changed, 44 insertions(+) create mode 100644 src/test/pythonEnvironments/base/locators/lowLevel/macDefaultLocator.unit.test.ts diff --git a/src/test/pythonEnvironments/base/locators/lowLevel/macDefaultLocator.unit.test.ts b/src/test/pythonEnvironments/base/locators/lowLevel/macDefaultLocator.unit.test.ts new file mode 100644 index 000000000000..3847869f5a2b --- /dev/null +++ b/src/test/pythonEnvironments/base/locators/lowLevel/macDefaultLocator.unit.test.ts @@ -0,0 +1,44 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +import * as assert from 'assert'; +import * as sinon from 'sinon'; +import * as osUtils from '../../../../../client/common/utils/platform'; +import { isMacDefaultPythonPath } from '../../../../../client/pythonEnvironments/base/locators/lowLevel/macDefaultLocator'; + +suite('isMacDefaultPythonPath', () => { + let getOSTypeStub: sinon.SinonStub; + + setup(() => { + getOSTypeStub = sinon.stub(osUtils, 'getOSType'); + }); + + teardown(() => { + sinon.restore(); + }); + + const testCases: { path: string; os: osUtils.OSType; expected: boolean }[] = [ + { path: 'python', os: osUtils.OSType.OSX, expected: true }, + { path: 'python', os: osUtils.OSType.Windows, expected: false }, + { path: '/usr/bin/python', os: osUtils.OSType.OSX, expected: true }, + { path: '/usr/bin/python', os: osUtils.OSType.Linux, expected: false }, + { path: '/usr/bin/python2', os: osUtils.OSType.OSX, expected: true }, + { path: '/usr/local/bin/python2', os: osUtils.OSType.OSX, expected: false }, + { path: '/usr/bin/python3', os: osUtils.OSType.OSX, expected: false }, + { path: '/usr/bin/python3', os: osUtils.OSType.Linux, expected: false }, + ]; + + testCases.forEach(({ path, os, expected }) => { + const testName = `If the Python path is ${path} on ${os}, it is${ + expected ? '' : ' not' + } a macOS default Python path`; + + test(testName, () => { + getOSTypeStub.returns(os); + + const result = isMacDefaultPythonPath(path); + + assert.strictEqual(result, expected); + }); + }); +});