Skip to content

Commit

Permalink
Ensure editable install only when [build-system] is present `pyprojec…
Browse files Browse the repository at this point in the history
…t.toml` (#20625)

Fixes #20620
  • Loading branch information
karthiknadig authored Feb 2, 2023
1 parent b43bc25 commit e743037
Show file tree
Hide file tree
Showing 2 changed files with 46 additions and 18 deletions.
39 changes: 26 additions & 13 deletions src/client/pythonEnvironments/creation/provider/venvUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import { CancellationToken, QuickPickItem, RelativePattern, WorkspaceFolder } fr
import { CreateEnv } from '../../../common/utils/localize';
import { showQuickPick } from '../../../common/vscodeApis/windowApis';
import { findFiles } from '../../../common/vscodeApis/workspaceApis';
import { traceError, traceVerbose } from '../../../logging';
import { traceError, traceInfo, traceVerbose } from '../../../logging';

const exclude = '**/{.venv*,.git,.nox,.tox,.conda,site-packages,__pypackages__}/**';
async function getPipRequirementsFiles(
Expand All @@ -25,20 +25,27 @@ async function getPipRequirementsFiles(
return files;
}

async function getTomlOptionalDeps(tomlPath: string): Promise<string[]> {
const content = await fs.readFile(tomlPath, 'utf-8');
const extras: string[] = [];
function tomlParse(content: string): tomljs.JsonMap {
try {
const toml = tomljs.parse(content);
if (toml.project && (toml.project as Record<string, Array<string>>)['optional-dependencies']) {
const deps = (toml.project as Record<string, Record<string, Array<string>>>)['optional-dependencies'];
for (const key of Object.keys(deps)) {
extras.push(key);
}
}
return tomljs.parse(content);
} catch (err) {
traceError('Failed to parse `pyproject.toml`:', err);
}
return {};
}

function tomlHasBuildSystem(toml: tomljs.JsonMap): boolean {
return toml['build-system'] !== undefined;
}

function getTomlOptionalDeps(toml: tomljs.JsonMap): string[] {
const extras: string[] = [];
if (toml.project && (toml.project as tomljs.JsonMap)['optional-dependencies']) {
const deps = (toml.project as tomljs.JsonMap)['optional-dependencies'];
for (const key of Object.keys(deps)) {
extras.push(key);
}
}
return extras;
}

Expand Down Expand Up @@ -109,12 +116,15 @@ export async function pickPackagesToInstall(

let extras: string[] = [];
let tomlExists = false;
let hasBuildSystem = false;
if (await fs.pathExists(tomlPath)) {
tomlExists = true;
extras = await getTomlOptionalDeps(tomlPath);
const toml = tomlParse(await fs.readFile(tomlPath, 'utf-8'));
extras = getTomlOptionalDeps(toml);
hasBuildSystem = tomlHasBuildSystem(toml);
}

if (tomlExists) {
if (tomlExists && hasBuildSystem) {
if (extras.length === 0) {
return { installType: 'toml', installList: [], source: tomlPath };
}
Expand All @@ -125,6 +135,9 @@ export async function pickPackagesToInstall(
}
return undefined;
}
if (tomlExists) {
traceInfo('Create env: Found toml without optional dependencies or build system.');
}

traceVerbose('Looking for pip requirements.');
const requirementFiles = (await getPipRequirementsFiles(workspaceFolder, token))?.map((p) =>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,11 +46,26 @@ suite('Venv Utils test', () => {
});
});

test('Toml found with no optional deps', async () => {
test('Toml found with no build system', async () => {
findFilesStub.resolves([]);
pathExistsStub.resolves(true);
readFileStub.resolves('[project]\nname = "spam"\nversion = "2020.0.0"\n');

const actual = await pickPackagesToInstall(workspace1);
assert.isTrue(showQuickPickStub.notCalled);
assert.deepStrictEqual(actual, {
installType: 'none',
installList: [],
});
});

test('Toml found with no optional deps', async () => {
findFilesStub.resolves([]);
pathExistsStub.resolves(true);
readFileStub.resolves(
'[project]\nname = "spam"\nversion = "2020.0.0"\n[build-system]\nrequires = ["setuptools ~= 58.0", "cython ~= 0.29.0"]',
);

const actual = await pickPackagesToInstall(workspace1);
assert.isTrue(showQuickPickStub.notCalled);
assert.deepStrictEqual(actual, {
Expand All @@ -64,7 +79,7 @@ suite('Venv Utils test', () => {
findFilesStub.resolves([]);
pathExistsStub.resolves(true);
readFileStub.resolves(
'[project]\nname = "spam"\nversion = "2020.0.0"\n[project.optional-dependencies]\ntest = ["pytest"]\ndoc = ["sphinx", "furo"]',
'[project]\nname = "spam"\nversion = "2020.0.0"\n[build-system]\nrequires = ["setuptools ~= 58.0", "cython ~= 0.29.0"]\n[project.optional-dependencies]\ntest = ["pytest"]\ndoc = ["sphinx", "furo"]',
);

showQuickPickStub.resolves(undefined);
Expand All @@ -88,7 +103,7 @@ suite('Venv Utils test', () => {
findFilesStub.resolves([]);
pathExistsStub.resolves(true);
readFileStub.resolves(
'[project]\nname = "spam"\nversion = "2020.0.0"\n[project.optional-dependencies]\ntest = ["pytest"]\ndoc = ["sphinx", "furo"]',
'[project]\nname = "spam"\nversion = "2020.0.0"\n[build-system]\nrequires = ["setuptools ~= 58.0", "cython ~= 0.29.0"]\n[project.optional-dependencies]\ntest = ["pytest"]\ndoc = ["sphinx", "furo"]',
);

showQuickPickStub.resolves([]);
Expand Down Expand Up @@ -116,7 +131,7 @@ suite('Venv Utils test', () => {
findFilesStub.resolves([]);
pathExistsStub.resolves(true);
readFileStub.resolves(
'[project]\nname = "spam"\nversion = "2020.0.0"\n[project.optional-dependencies]\ntest = ["pytest"]\ndoc = ["sphinx", "furo"]',
'[project]\nname = "spam"\nversion = "2020.0.0"\n[build-system]\nrequires = ["setuptools ~= 58.0", "cython ~= 0.29.0"]\n[project.optional-dependencies]\ntest = ["pytest"]\ndoc = ["sphinx", "furo"]',
);

showQuickPickStub.resolves([{ label: 'doc' }]);
Expand Down Expand Up @@ -144,7 +159,7 @@ suite('Venv Utils test', () => {
findFilesStub.resolves([]);
pathExistsStub.resolves(true);
readFileStub.resolves(
'[project]\nname = "spam"\nversion = "2020.0.0"\n[project.optional-dependencies]\ntest = ["pytest"]\ndoc = ["sphinx", "furo"]\ncov = ["pytest-cov"]',
'[project]\nname = "spam"\nversion = "2020.0.0"\n[build-system]\nrequires = ["setuptools ~= 58.0", "cython ~= 0.29.0"]\n[project.optional-dependencies]\ntest = ["pytest"]\ndoc = ["sphinx", "furo"]\ncov = ["pytest-cov"]',
);

showQuickPickStub.resolves([{ label: 'test' }, { label: 'cov' }]);
Expand Down

0 comments on commit e743037

Please sign in to comment.