Skip to content

Add API to identify pipenv #13762

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Sep 15, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions src/client/pythonEnvironments/common/environmentIdentifier.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// Licensed under the MIT License.

import { isCondaEnvironment } from '../discovery/locators/services/condaLocator';
import { isPipenvEnvironment } from '../discovery/locators/services/pipEnvHelper';
import { isVenvEnvironment } from '../discovery/locators/services/venvLocator';
import { isVirtualenvEnvironment } from '../discovery/locators/services/virtualenvLocator';
import { isWindowsStoreEnvironment } from '../discovery/locators/services/windowsStoreLocator';
Expand Down Expand Up @@ -39,6 +40,10 @@ export async function identifyEnvironment(interpreterPath: string): Promise<Envi
return EnvironmentType.WindowsStore;
}

if (await isPipenvEnvironment(interpreterPath)) {
return EnvironmentType.Pipenv;
}

if (await isVenvEnvironment(interpreterPath)) {
return EnvironmentType.Venv;
}
Expand Down
22 changes: 16 additions & 6 deletions src/client/pythonEnvironments/common/externalDependencies.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,9 @@
// Licensed under the MIT License.

import * as fsapi from 'fs-extra';
import * as path from 'path';
import { ExecutionResult, IProcessServiceFactory } from '../../common/process/types';
import { createDeferred } from '../../common/utils/async';
import { getOSType, OSType } from '../../common/utils/platform';
import { IServiceContainer } from '../../ioc/types';

let internalServiceContainer: IServiceContainer;
Expand All @@ -21,9 +22,18 @@ export async function shellExecute(command: string, timeout: number): Promise<Ex
}

export function pathExists(absPath: string): Promise<boolean> {
const deferred = createDeferred<boolean>();
fsapi.exists(absPath, (result) => {
deferred.resolve(result);
});
return deferred.promise;
return fsapi.pathExists(absPath);
}

export function readFile(filePath: string): Promise<string> {
return fsapi.readFile(filePath, 'utf-8');
}

export function arePathsSame(path1: string, path2: string): boolean {
path1 = path.normalize(path1);
path2 = path.normalize(path2);
if (getOSType() === OSType.Windows) {
return path1.toUpperCase() === path2.toUpperCase();
}
return path1 === path2;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,152 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.

import * as path from 'path';
import { traceError } from '../../../../common/logger';
import { getEnvironmentVariable } from '../../../../common/utils/platform';
import { arePathsSame, pathExists, readFile } from '../../../common/externalDependencies';

function getSearchHeight() {
// PIPENV_MAX_DEPTH tells pipenv the maximum number of directories to recursively search for
// a Pipfile, defaults to 3: https://pipenv.pypa.io/en/latest/advanced/#pipenv.environments.PIPENV_MAX_DEPTH
const maxDepthStr = getEnvironmentVariable('PIPENV_MAX_DEPTH');
if (maxDepthStr === undefined) {
return 3;
}
const maxDepth = parseInt(maxDepthStr, 10);
// eslint-disable-next-line no-restricted-globals
if (isNaN(maxDepth)) {
traceError(`PIPENV_MAX_DEPTH is incorrectly set. Converting value '${maxDepthStr}' to number results in NaN`);
return 1;
}
return maxDepth;
}

/**
* Returns the path to Pipfile associated with the provided directory.
* @param searchDir the directory to look into
* @param lookIntoParentDirectories set to true if we should also search for Pipfile in parent directory
*/
export async function _getAssociatedPipfile(

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm personally not opposed to exporting internal functions for the sake of unit testing. However, IIRC as a team we had decided to avoid doing this. You may want to double-check with everyone if that is still the case.

Copy link
Author

@karrtikr karrtikr Sep 10, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have avoided it for most functions, however I felt this one was sophisticated enough so I had to single it out to test it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there no way to exercise it indirectly so you don't have to export it?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see how this function can be "unit" tested cleanly, if we don't export.

If we test it along with testing the exported APIs, it won't be so readable and it'll be difficult to test all branches.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Karthik will deal with it separately.

searchDir: string,
options: {lookIntoParentDirectories: boolean},
): Promise<string | undefined> {
const pipFileName = getEnvironmentVariable('PIPENV_PIPFILE') || 'Pipfile';
let heightToSearch = options.lookIntoParentDirectories ? getSearchHeight() : 1;
while (heightToSearch > 0 && !arePathsSame(searchDir, path.dirname(searchDir))) {
const pipFile = path.join(searchDir, pipFileName);
// eslint-disable-next-line no-await-in-loop
if (await pathExists(pipFile)) {
return pipFile;
}
searchDir = path.dirname(searchDir);
heightToSearch -= 1;
}
return undefined;
}

/**
* If interpreter path belongs to a pipenv environment which is located inside a project, return associated Pipfile,
* otherwise return `undefined`.
* @param interpreterPath Absolute path to any python interpreter.
*/
async function getPipfileIfLocal(interpreterPath: string): Promise<string | undefined> {
// Local pipenv environments are created by setting PIPENV_VENV_IN_PROJECT to 1, which always names the environment
// folder '.venv': https://pipenv.pypa.io/en/latest/advanced/#pipenv.environments.PIPENV_VENV_IN_PROJECT
// This is the layout we wish to verify.
// project
// |__ Pipfile <--- check if Pipfile exists here
// |__ .venv <--- check if name of the folder is '.venv'
// |__ Scripts/bin
// |__ python <--- interpreterPath
const venvFolder = path.dirname(path.dirname(interpreterPath));
if (path.basename(venvFolder) !== '.venv') {
return undefined;
}
const directoryWhereVenvResides = path.dirname(venvFolder);
return _getAssociatedPipfile(directoryWhereVenvResides, { lookIntoParentDirectories: false });
}

/**
* Returns the project directory for pipenv environments given the environment folder
* @param envFolder Path to the environment folder
*/
async function getProjectDir(envFolder: string): Promise<string | undefined> {
// Global pipenv environments have a .project file with the absolute path to the project
// See https://github.com/pypa/pipenv/blob/v2018.6.25/CHANGELOG.rst#features--improvements
// This is the layout we expect
// <Environment folder>
// |__ .project <--- check if .project exists here
// |__ Scripts/bin
// |__ python <--- interpreterPath
// We get the project by reading the .project file
const dotProjectFile = path.join(envFolder, '.project');
if (!(await pathExists(dotProjectFile))) {
return undefined;
}
const projectDir = await readFile(dotProjectFile);
if (!(await pathExists(projectDir))) {
traceError(`The .project file inside environment folder: ${envFolder} doesn't contain a valid path to the project`);
return undefined;
}
return projectDir;
}

/**
* If interpreter path belongs to a global pipenv environment, return associated Pipfile, otherwise return `undefined`.
* @param interpreterPath Absolute path to any python interpreter.
*/
async function getPipfileIfGlobal(interpreterPath: string): Promise<string | undefined> {
const envFolder = path.dirname(path.dirname(interpreterPath));
const projectDir = await getProjectDir(envFolder);
if (projectDir === undefined) {
return undefined;
}

// This is the layout we expect to see.
// project
// |__ Pipfile <--- check if Pipfile exists here and return it
// The name of the project (directory where Pipfile resides) is used as a prefix in the environment folder
const envFolderName = path.basename(envFolder);
if (!envFolderName.startsWith(`${path.basename(projectDir)}-`)) {
return undefined;
}

return _getAssociatedPipfile(projectDir, { lookIntoParentDirectories: false });
}

/**
* Checks if the given interpreter path belongs to a pipenv environment, by locating the Pipfile which was used to
* create the environment.
* @param interpreterPath: Absolute path to any python interpreter.
*/
export async function isPipenvEnvironment(interpreterPath: string): Promise<boolean> {
if (await getPipfileIfLocal(interpreterPath)) {
return true;
}
if (await getPipfileIfGlobal(interpreterPath)) {
return true;
}
return false;
}

/**
* Returns true if interpreter path belongs to a global pipenv environment which is associated with a particular folder,
* false otherwise.
* @param interpreterPath Absolute path to any python interpreter.
*/
export async function isPipenvEnvironmentRelatedToFolder(interpreterPath: string, folder: string): Promise<boolean> {
const pipFileAssociatedWithEnvironment = await getPipfileIfGlobal(interpreterPath);
if (!pipFileAssociatedWithEnvironment) {
return false;
}

// PIPENV_NO_INHERIT is used to tell pipenv not to look for Pipfile in parent directories
// https://pipenv.pypa.io/en/latest/advanced/#pipenv.environments.PIPENV_NO_INHERIT
const lookIntoParentDirectories = (getEnvironmentVariable('PIPENV_NO_INHERIT') === undefined);
const pipFileAssociatedWithFolder = await _getAssociatedPipfile(folder, { lookIntoParentDirectories });
if (!pipFileAssociatedWithFolder) {
return false;
}
return arePathsSame(pipFileAssociatedWithEnvironment, pipFileAssociatedWithFolder);
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import * as path from 'path';
import * as sinon from 'sinon';
import * as platformApis from '../../../client/common/utils/platform';
import { identifyEnvironment } from '../../../client/pythonEnvironments/common/environmentIdentifier';
import * as externalDependencies from '../../../client/pythonEnvironments/common/externalDependencies';
import { EnvironmentType } from '../../../client/pythonEnvironments/info';
import { TEST_LAYOUT_ROOT } from './commonTestConstants';

Expand All @@ -23,6 +24,40 @@ suite('Environment Identifier', () => {
});
});

suite('Pipenv', () => {
let getEnvVar: sinon.SinonStub;
let readFile: sinon.SinonStub;
setup(() => {
getEnvVar = sinon.stub(platformApis, 'getEnvironmentVariable');
readFile = sinon.stub(externalDependencies, 'readFile');
});

teardown(() => {
readFile.restore();
getEnvVar.restore();
});

test('Path to a global pipenv environment', async () => {
const expectedDotProjectFile = path.join(TEST_LAYOUT_ROOT, 'pipenv', 'globalEnvironments', 'project2-vnNIWe9P', '.project');
const expectedProjectFile = path.join(TEST_LAYOUT_ROOT, 'pipenv', 'project2');
readFile.withArgs(expectedDotProjectFile).resolves(expectedProjectFile);
const interpreterPath: string = path.join(TEST_LAYOUT_ROOT, 'pipenv', 'globalEnvironments', 'project2-vnNIWe9P', 'bin', 'python');

const envType: EnvironmentType = await identifyEnvironment(interpreterPath);

assert.equal(envType, EnvironmentType.Pipenv);
});

test('Path to a local pipenv environment with a custom Pipfile name', async () => {
getEnvVar.withArgs('PIPENV_PIPFILE').returns('CustomPipfileName');
const interpreterPath: string = path.join(TEST_LAYOUT_ROOT, 'pipenv', 'project1', '.venv', 'Scripts', 'python.exe');

const envType: EnvironmentType = await identifyEnvironment(interpreterPath);

assert.equal(envType, EnvironmentType.Pipenv);
});
});

suite('Windows Store', () => {
let getEnvVar: sinon.SinonStub;
const fakeLocalAppDataPath = path.join(TEST_LAYOUT_ROOT, 'storeApps');
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Absolute path to \src\test\pythonEnvironments\common\envlayouts\pipenv\project2
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Not a real python binary
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rather than creating these artifacts here, could you use an in-memory filesystem fake? That way the tests that use these can create these (e.g. from declarative JSON/YAML) and it would be clear what the test is doing. See for example https://dev.to/julienp/node-js-testing-using-a-virtual-filesystem-as-a-mock-2jln

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. There're other such tests as well which has already went in. We'll address fixing all these tests separately.

Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Absolute path to \src\test\pythonEnvironments\common\envlayouts\pipenv\project3
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Not real python exe
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Not real python exe
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
[[source]]
name = "pypi"
url = "https://pypi.org/simple"
verify_ssl = true

[dev-packages]

[packages]

[requires]
python_version = "3.7"
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
[[source]]
name = "pypi"
url = "https://pypi.org/simple"
verify_ssl = true

[dev-packages]

[packages]

[requires]
python_version = "3.7"
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
[[source]]
name = "pypi"
url = "https://pypi.org/simple"
verify_ssl = true

[dev-packages]

[packages]

[requires]
python_version = "3.7"
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
import * as assert from 'assert';
import * as path from 'path';
import * as sinon from 'sinon';
import * as platformApis from '../../../../client/common/utils/platform';
import * as externalDependencies from '../../../../client/pythonEnvironments/common/externalDependencies';
import { isPipenvEnvironmentRelatedToFolder } from '../../../../client/pythonEnvironments/discovery/locators/services/pipEnvHelper';
import { TEST_LAYOUT_ROOT } from '../../common/commonTestConstants';

suite('Pipenv utils', () => {
let readFile: sinon.SinonStub;
let getEnvVar: sinon.SinonStub;
setup(() => {
getEnvVar = sinon.stub(platformApis, 'getEnvironmentVariable');
readFile = sinon.stub(externalDependencies, 'readFile');
});

teardown(() => {
readFile.restore();
getEnvVar.restore();
});

test('Global pipenv environment is associated with a project whose Pipfile lies at 3 levels above the project', async () => {
getEnvVar.withArgs('PIPENV_MAX_DEPTH').returns('5');
const expectedDotProjectFile = path.join(TEST_LAYOUT_ROOT, 'pipenv', 'globalEnvironments', 'project3-2s1eXEJ2', '.project');
const project = path.join(TEST_LAYOUT_ROOT, 'pipenv', 'project3');
readFile.withArgs(expectedDotProjectFile).resolves(project);
const interpreterPath: string = path.join(TEST_LAYOUT_ROOT, 'pipenv', 'globalEnvironments', 'project3-2s1eXEJ2', 'Scripts', 'python.exe');
const folder = path.join(project, 'parent', 'child', 'folder');

const isRelated = await isPipenvEnvironmentRelatedToFolder(interpreterPath, folder);

assert.equal(isRelated, true);
});
});
Loading