Skip to content

Use a glob pattern to look for conda executables #1904

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 5 commits into from
Jun 8, 2018
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
2 changes: 1 addition & 1 deletion .appveyor.yml
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ init:

install:
- ps: Install-Product node $env:nodejs_version
- npm ci
- npm i
- "SET PATH=%PYTHON%;%PYTHON%\\Scripts;%PATH%"
- python -m pip install -U pip
- pip install -t ./pythonFiles/experimental/ptvsd git+https://github.com/Microsoft/ptvsd/
Expand Down
1 change: 1 addition & 0 deletions news/3 Code Health/256.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Use a glob pattern to look for `conda` executables.
15 changes: 4 additions & 11 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -1891,6 +1891,7 @@
"fs-extra": "4.0.3",
"fuzzy": "0.1.3",
"get-port": "3.2.0",
"glob": "^7.1.2",
"iconv-lite": "0.4.21",
"inversify": "4.11.1",
"line-by-line": "0.1.6",
Expand Down
11 changes: 11 additions & 0 deletions src/client/common/platform/fileSystem.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

import { createHash } from 'crypto';
import * as fs from 'fs-extra';
import * as glob from 'glob';
import { inject, injectable } from 'inversify';
import * as path from 'path';
import { createDeferred } from '../helpers';
Expand Down Expand Up @@ -130,4 +131,14 @@ export class FileSystem implements IFileSystem {
});
});
}
public search(globPattern: string): Promise<string[]> {
return new Promise<string[]>((resolve, reject) => {
glob(globPattern, (ex, files) => {
if (ex) {
return reject(ex);
}
resolve(Array.isArray(files) ? files : []);
});
});
}
}
1 change: 1 addition & 0 deletions src/client/common/platform/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,4 +47,5 @@ export interface IFileSystem {
copyFile(src: string, dest: string): Promise<void>;
deleteFile(filename: string): Promise<void>;
getFileHash(filePath: string): Promise<string | undefined>;
search(globPattern: string): Promise<string[]>;
}
11 changes: 5 additions & 6 deletions src/client/interpreter/locators/services/condaService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,9 @@ import { CondaHelper } from './condaHelper';
// tslint:disable-next-line:no-require-imports no-var-requires
const untildify: (value: string) => string = require('untildify');

export const KNOWN_CONDA_LOCATIONS = ['~/anaconda/bin/conda', '~/miniconda/bin/conda',
'~/anaconda2/bin/conda', '~/miniconda2/bin/conda',
'~/anaconda3/bin/conda', '~/miniconda3/bin/conda'];
// This glob pattern will match all of the following:
// ~/anaconda/bin/conda, ~/anaconda3/bin/conda, ~/miniconda/bin/conda, ~/miniconda3/bin/conda
export const CondaLocationsGlob = '~/*conda*/bin/conda';

@injectable()
export class CondaService implements ICondaService {
Expand Down Expand Up @@ -181,9 +181,8 @@ export class CondaService implements ICondaService {
return this.getCondaFileFromKnownLocations();
}
private async getCondaFileFromKnownLocations(): Promise<string> {
const condaFiles = await Promise.all(KNOWN_CONDA_LOCATIONS
.map(untildify)
.map(async (condaPath: string) => this.fileSystem.fileExists(condaPath).then(exists => exists ? condaPath : '')));
const condaFiles = await this.fileSystem.search(untildify(CondaLocationsGlob))
.catch<string[]>(() => []);

const validCondaFiles = condaFiles.filter(condaPath => condaPath.length > 0);
return validCondaFiles.length === 0 ? 'conda' : validCondaFiles[0];
Expand Down
8 changes: 8 additions & 0 deletions src/test/common/platform/filesystem.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -77,4 +77,12 @@ suite('FileSystem', () => {
const fileContents = await fileSystem.readFile(fileToAppendTo);
expect(fileContents).to.be.equal(dataToAppend);
});
test('Test searching for files', async () => {
const files = await fileSystem.search(path.join(__dirname, '*.js'));
expect(files).to.be.array();
expect(files.length).to.be.at.least(1);
const expectedFileName = __filename.replace(/\\/g, '/');
const fileName = files[0].replace(/\\/g, '/');
expect(fileName).to.equal(expectedFileName);
});
});
24 changes: 14 additions & 10 deletions src/test/interpreters/condaService.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import { Architecture, IFileSystem, IPlatformService } from '../../client/common
import { IProcessService, IProcessServiceFactory } from '../../client/common/process/types';
import { ILogger, IPersistentStateFactory } from '../../client/common/types';
import { IInterpreterLocatorService, InterpreterType, PythonInterpreter } from '../../client/interpreter/contracts';
import { CondaService, KNOWN_CONDA_LOCATIONS } from '../../client/interpreter/locators/services/condaService';
import { CondaService } from '../../client/interpreter/locators/services/condaService';
import { IServiceContainer } from '../../client/ioc/types';
import { MockState } from './mocks';

Expand Down Expand Up @@ -356,21 +356,25 @@ suite('Interpreters Conda Service', () => {
processService.verify(p => p.exec(TypeMoq.It.isAny(), TypeMoq.It.isAny(), TypeMoq.It.isAny()), TypeMoq.Times.once());
});

KNOWN_CONDA_LOCATIONS.forEach(knownLocation => {
test(`Must return conda path from known location '${knownLocation}' (non windows)`, async () => {
const expectedCondaLocation = untildify(knownLocation);
platformService.setup(p => p.isWindows).returns(() => false);
processService.setup(p => p.exec(TypeMoq.It.isValue('conda'), TypeMoq.It.isValue(['--version']), TypeMoq.It.isAny())).returns(() => Promise.reject(new Error('Not Found')));
fileSystem.setup(fs => fs.fileExists(TypeMoq.It.isAny())).returns((file: string) => Promise.resolve(file === expectedCondaLocation));
['~/anaconda/bin/conda', '~/miniconda/bin/conda', '~/anaconda2/bin/conda',
'~/miniconda2/bin/conda', '~/anaconda3/bin/conda', '~/miniconda3/bin/conda']
.forEach(knownLocation => {
test(`Must return conda path from known location '${knownLocation}' (non windows)`, async () => {
const expectedCondaLocation = untildify(knownLocation);
platformService.setup(p => p.isWindows).returns(() => false);
processService.setup(p => p.exec(TypeMoq.It.isValue('conda'), TypeMoq.It.isValue(['--version']), TypeMoq.It.isAny())).returns(() => Promise.reject(new Error('Not Found')));
fileSystem.setup(fs => fs.search(TypeMoq.It.isAny())).returns(() => Promise.resolve([expectedCondaLocation]));
fileSystem.setup(fs => fs.fileExists(TypeMoq.It.isValue(expectedCondaLocation))).returns(() => Promise.resolve(true));

const condaExe = await condaService.getCondaFile();
assert.equal(condaExe, expectedCondaLocation, 'Failed to identify');
const condaExe = await condaService.getCondaFile();
assert.equal(condaExe, expectedCondaLocation, 'Failed to identify');
});
});
});

test('Must return \'conda\' if conda could not be found in known locations', async () => {
platformService.setup(p => p.isWindows).returns(() => false);
processService.setup(p => p.exec(TypeMoq.It.isValue('conda'), TypeMoq.It.isValue(['--version']), TypeMoq.It.isAny())).returns(() => Promise.reject(new Error('Not Found')));
fileSystem.setup(fs => fs.search(TypeMoq.It.isAny())).returns(() => Promise.resolve([]));
fileSystem.setup(fs => fs.fileExists(TypeMoq.It.isAny())).returns((file: string) => Promise.resolve(false));

const condaExe = await condaService.getCondaFile();
Expand Down