Skip to content
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

Fixes #22 to Detect anaconda from known locations #221

Merged
merged 19 commits into from
Nov 22, 2017
Merged
Show file tree
Hide file tree
Changes from 8 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
18 changes: 18 additions & 0 deletions .vscode/tasks.json
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,24 @@
"fileLocation": "relative"
}
]
},
{
"type": "gulp",
"task": "hygiene-watch",
"isBackground": true,
"presentation": {
"echo": true,
"reveal": "always",
"focus": false,
"panel": "shared"
},
"problemMatcher": [
"$tsc",
{
"base": "$tslint5",
"fileLocation": "relative"
}
]
}
]
}
48 changes: 27 additions & 21 deletions gulpfile.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ const tsfmt = require('typescript-formatter');
const tslint = require('tslint');
const relative = require('relative');
const ts = require('gulp-typescript');
const watch = require('gulp-watch');
const cp = require('child_process');

/**
* Hygiene works by creating cascading subsets of all our files and
Expand Down Expand Up @@ -212,47 +214,51 @@ const hygiene = exports.hygiene = (some, options) => {

gulp.task('hygiene', () => hygiene());

// this allows us to run hygiene as a git pre-commit hook.
if (require.main === module) {
const cp = require('child_process');
gulp.task('hygiene-watch', function () {
return watch(all, function () {
run(true, true);
});
});

function run(lintOnlyModifiedFiles, doNotExit) {
function exitProcessOnError(ex) {
console.error();
console.error(ex);
if (!doNotExit) {
process.exit(1);
}
}
process.on('unhandledRejection', (reason, p) => {
console.log('Unhandled Rejection at: Promise', p, 'reason:', reason);
process.exit(1);
exitProcessOnError();
});

cp.exec('git config core.autocrlf', (err, out) => {
const skipEOL = out.trim() === 'true';

if (process.argv.length > 2) {
if (!lintOnlyModifiedFiles && process.argv.length > 2) {
return hygiene(process.argv.slice(2), {
skipEOL: skipEOL
}).on('error', err => {
console.error();
console.error(err);
process.exit(1);
});
}).on('error', exitProcessOnError);
}

cp.exec('git diff --cached --name-only', {
const cmd = lintOnlyModifiedFiles ? 'git diff --name-only' : 'git diff --cached --name-only';
cp.exec(cmd, {
maxBuffer: 2000 * 1024
}, (err, out) => {
if (err) {
console.error();
console.error(err);
process.exit(1);
exitProcessOnError(err);
}

const some = out
.split(/\r?\n/)
.filter(l => !!l);

hygiene(some, {
skipEOL: skipEOL
}).on('error', err => {
console.error();
console.error(err);
process.exit(1);
});
}).on('error', exitProcessOnError);
});
});
}
// this allows us to run hygiene as a git pre-commit hook.
if (require.main === module) {
run();
}
5 changes: 3 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -1635,6 +1635,7 @@
"gulp": "^3.9.1",
"gulp-filter": "^5.0.1",
"gulp-typescript": "^3.2.2",
"gulp-watch": "^4.3.11",
"husky": "^0.14.3",
"ignore-loader": "^0.1.1",
"mocha": "^2.3.3",
Expand All @@ -1649,7 +1650,7 @@
"tslint-microsoft-contrib": "^5.0.1",
"typescript": "^2.5.2",
"typescript-formatter": "^6.0.0",
"webpack": "^1.13.2",
"vscode": "^1.1.5"
"vscode": "^1.1.5",
"webpack": "^1.13.2"
}
}
Binary file added python-0.7.0.vsix
Binary file not shown.
4 changes: 4 additions & 0 deletions src/client/interpreter/contracts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,10 @@ export interface IInterpreterLocatorService extends Disposable {
getInterpreters(resource?: Uri): Promise<PythonInterpreter[]>;
}

export interface ICondaLocatorService {
getCondaFile(): Promise<string>;
}

export type PythonInterpreter = {
path: string;
companyDisplayName?: string;
Expand Down
14 changes: 5 additions & 9 deletions src/client/interpreter/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { ConfigurationTarget, window, workspace } from 'vscode';
import { RegistryImplementation } from '../common/registry';
import { Is_64Bit, IS_WINDOWS } from '../common/utils';
import { WorkspacePythonPath } from './contracts';
import { CondaEnvService } from './locators/services/condaEnvService';
import { CondaLocatorService } from './locators/services/condaLocator';
import { WindowsRegistryService } from './locators/services/windowsRegistryService';

export function getFirstNonEmptyLineFromMultilineString(stdout: string) {
Expand All @@ -29,14 +29,10 @@ export function getActiveWorkspaceUri(): WorkspacePythonPath | undefined {
return undefined;
}
export async function getCondaVersion() {
let condaService: CondaEnvService;
if (IS_WINDOWS) {
const windowsRegistryProvider = new WindowsRegistryService(new RegistryImplementation(), Is_64Bit);
condaService = new CondaEnvService(windowsRegistryProvider);
} else {
condaService = new CondaEnvService();
}
return condaService.getCondaFile()
const windowsRegistryProvider = IS_WINDOWS ? new WindowsRegistryService(new RegistryImplementation(), Is_64Bit) : undefined;
const condaLocator = new CondaLocatorService(windowsRegistryProvider);

return condaLocator.getCondaFile()
.then(async condaFile => {
return new Promise<string>((resolve, reject) => {
child_process.execFile(condaFile, ['--version'], (_, stdout) => {
Expand Down
11 changes: 7 additions & 4 deletions src/client/interpreter/locators/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,12 @@ import { Disposable, Uri, workspace } from 'vscode';
import { RegistryImplementation } from '../../common/registry';
import { areBasePathsSame, arePathsSame, Is_64Bit, IS_WINDOWS } from '../../common/utils';
import { IInterpreterLocatorService, PythonInterpreter } from '../contracts';
import { IInterpreterVersionService, InterpreterVersionService } from '../interpreterVersion';
import { InterpreterVersionService } from '../interpreterVersion';
import { VirtualEnvironmentManager } from '../virtualEnvs';
import { fixInterpreterDisplayName, fixInterpreterPath } from './helpers';
import { CondaEnvFileService, getEnvironmentsFile as getCondaEnvFile } from './services/condaEnvFileService';
import { CondaEnvService } from './services/condaEnvService';
import { CondaLocatorService } from './services/condaLocator';
import { CurrentPathService } from './services/currentPathService';
import { getKnownSearchPathsForInterpreters, KnownPathsService } from './services/KnownPathsService';
import { getKnownSearchPathsForVirtualEnvs, VirtualEnvService } from './services/virtualEnvService';
Expand Down Expand Up @@ -46,7 +47,7 @@ export class PythonInterpreterLocatorService implements IInterpreterLocatorServi
}
private async getInterpretersPerResource(resource?: Uri) {
const locators = this.getLocators(resource);
const promises = locators.map(provider => provider.getInterpreters(resource));
const promises = locators.map(async provider => provider.getInterpreters(resource));
const listOfInterpreters = await Promise.all(promises);

// tslint:disable-next-line:underscore-consistent-invocation
Expand All @@ -67,10 +68,12 @@ export class PythonInterpreterLocatorService implements IInterpreterLocatorServi
// The order of the services is important.
if (IS_WINDOWS) {
const windowsRegistryProvider = new WindowsRegistryService(new RegistryImplementation(), Is_64Bit);
const condaLocator = new CondaLocatorService(windowsRegistryProvider);
locators.push(windowsRegistryProvider);
locators.push(new CondaEnvService(windowsRegistryProvider));
locators.push(new CondaEnvService(condaLocator));
} else {
locators.push(new CondaEnvService());
const condaLocator = new CondaLocatorService();
locators.push(new CondaEnvService(condaLocator));
}
// Supplements the above list of conda environments.
locators.push(new CondaEnvFileService(getCondaEnvFile(), versionService));
Expand Down
20 changes: 3 additions & 17 deletions src/client/interpreter/locators/services/condaEnvService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,33 +4,19 @@ import * as fs from 'fs-extra';
import * as path from 'path';
import { Uri } from 'vscode';
import { VersionUtils } from '../../../common/versionUtils';
import { IInterpreterLocatorService, PythonInterpreter } from '../../contracts';
import { ICondaLocatorService, IInterpreterLocatorService, PythonInterpreter } from '../../contracts';
import { AnacondaCompanyName, CONDA_RELATIVE_PY_PATH, CondaInfo } from './conda';
import { CondaHelper } from './condaHelper';

export class CondaEnvService implements IInterpreterLocatorService {
private readonly condaHelper = new CondaHelper();
constructor(private registryLookupForConda?: IInterpreterLocatorService) {
constructor(private condaLocator: ICondaLocatorService) {
}
public async getInterpreters(resource?: Uri) {
return this.getSuggestionsFromConda();
}
// tslint:disable-next-line:no-empty
public dispose() { }
public async getCondaFile() {
if (this.registryLookupForConda) {
return this.registryLookupForConda.getInterpreters()
.then(interpreters => interpreters.filter(this.isCondaEnvironment))
.then(condaInterpreters => this.getLatestVersion(condaInterpreters))
.then(condaInterpreter => {
return condaInterpreter ? path.join(path.dirname(condaInterpreter.path), 'conda.exe') : 'conda';
})
.then(async condaPath => {
return fs.pathExists(condaPath).then(exists => exists ? condaPath : 'conda');
});
}
return Promise.resolve('conda');
}
public isCondaEnvironment(interpreter: PythonInterpreter) {
return (interpreter.displayName ? interpreter.displayName : '').toUpperCase().indexOf('ANACONDA') >= 0 ||
(interpreter.companyDisplayName ? interpreter.companyDisplayName : '').toUpperCase().indexOf('CONTINUUM') >= 0;
Expand Down Expand Up @@ -73,7 +59,7 @@ export class CondaEnvService implements IInterpreterLocatorService {
.then(interpreters => interpreters.map(interpreter => interpreter!));
}
private async getSuggestionsFromConda(): Promise<PythonInterpreter[]> {
return this.getCondaFile()
return this.condaLocator.getCondaFile()
.then(async condaFile => {
return new Promise<PythonInterpreter[]>((resolve, reject) => {
// interrogate conda (if it's on the path) to find all environments.
Expand Down
66 changes: 66 additions & 0 deletions src/client/interpreter/locators/services/condaLocator.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
'use strict';
import * as child_process from 'child_process';
import * as fs from 'fs-extra';
import * as path from 'path';
import { VersionUtils } from '../../../common/versionUtils';
import { ICondaLocatorService, IInterpreterLocatorService, PythonInterpreter } from '../../contracts';
// tslint:disable-next-line:no-require-imports no-var-requires
const untildify: (value: string) => string = require('untildify');

const KNOWN_CONDA_LOCATIONS = ['~/anaconda3/bin/conda', '~/miniconda3/bin/conda'];
Copy link
Author

Choose a reason for hiding this comment

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

@brettcannon the more I look at the code, I feel its just not future proof.
What happens when Anaconda4 comes out? Do we add anaconda4, and so on?
Making it dynamic could be slow as well, i.e. looking for directories '~/anaconda*`

Choose a reason for hiding this comment

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

Over the time I've installed different versions of miniconda on my home and work computer, and now I have all these directories: .miniconda2, miniconda, miniconda3.

Copy link
Member

Choose a reason for hiding this comment

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

I agree about the brittleness going forward. I say just do the glob match; the number of matches for that should be rather small and the number of stat calls to resolve this won't be that high.

Copy link
Author

Choose a reason for hiding this comment

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

Will go with the hacky approach for now, created an issue to be resolve separately (as I'd need to add an npm package).
#256


export class CondaLocatorService implements ICondaLocatorService {
constructor(private registryLookupForConda?: IInterpreterLocatorService) {
}
// tslint:disable-next-line:no-empty
public dispose() { }
public async getCondaFile(): Promise<string> {
const isAvailable = await this.isCondaInCurrentPath();
if (isAvailable) {
return 'conda';
}
if (this.registryLookupForConda) {
return this.registryLookupForConda.getInterpreters()
.then(interpreters => interpreters.filter(this.isCondaEnvironment))
.then(condaInterpreters => this.getLatestVersion(condaInterpreters))
.then(condaInterpreter => {
return condaInterpreter ? path.join(path.dirname(condaInterpreter.path), 'conda.exe') : 'conda';
Copy link
Member

Choose a reason for hiding this comment

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

Isn't assuming conda.exe a Windows-specific thing?

Copy link
Author

Choose a reason for hiding this comment

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

Should probably change the condition. Currently its implied with the existence of registryLookupForConda

Copy link
Author

Choose a reason for hiding this comment

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

Fixed

})
.then(async condaPath => {
return fs.pathExists(condaPath).then(exists => exists ? condaPath : 'conda');
});
}
return this.getCondaFileFromKnownLocations();
}
public isCondaEnvironment(interpreter: PythonInterpreter) {
return (interpreter.displayName ? interpreter.displayName : '').toUpperCase().indexOf('ANACONDA') >= 0 ||
(interpreter.companyDisplayName ? interpreter.companyDisplayName : '').toUpperCase().indexOf('CONTINUUM') >= 0;
}
public getLatestVersion(interpreters: PythonInterpreter[]) {
const sortedInterpreters = interpreters.filter(interpreter => interpreter.version && interpreter.version.length > 0);
// tslint:disable-next-line:no-non-null-assertion
sortedInterpreters.sort((a, b) => VersionUtils.compareVersion(a.version!, b.version!));
if (sortedInterpreters.length > 0) {
return sortedInterpreters[sortedInterpreters.length - 1];
}
}
public async isCondaInCurrentPath() {
return new Promise<boolean>((resolve, reject) => {
child_process.execFile('conda', ['--version'], (_, stdout) => {
if (stdout && stdout.length > 0) {
resolve(true);
} else {
resolve(false);
}
});
});
}
private async getCondaFileFromKnownLocations(): Promise<string> {
const condaFiles = await Promise.all(KNOWN_CONDA_LOCATIONS
.map(untildify)
.map(async (condaPath: string) => fs.pathExists(condaPath).then(exists => exists ? condaPath : '')));

const validCondaFiles = condaFiles.filter(condaPath => condaPath.length > 0);
return validCondaFiles.length === 0 ? 'conda' : validCondaFiles[0];
}
}
Loading