Fixes to JUPYTER_PATH (#12960)

Author: IanMatthewHuff

src/client/datascience/kernel-launcher/kernelFinder.ts

@@ -234,27 +234,23 @@ export class KernelFinder implements IKernelFinder {
         return uniqueInterpreterPrefixPaths.map((prefixPath) => path.join(prefixPath, baseKernelPath));
     }
 
-    private async getDiskPaths(): Promise<string[]> {
-        let paths: string[] = [];
+    // Find any paths associated with the JUPYTER_PATH env var. Can be a list of dirs.
+    // We need to look at the 'kernels' sub-directory and these paths are supposed to come first in the searching
+    // https://jupyter.readthedocs.io/en/latest/projects/jupyter-directories.html#envvar-JUPYTER_PATH
+    private async getJupyterPathPaths(): Promise<string[]> {
+        const paths: string[] = [];
         const vars = await this.envVarsProvider.getEnvironmentVariables();
-        const jupyterPathVar = vars.JUPYTER_PATH || '';
-
-        if (this.platformService.isWindows) {
-            const activeInterpreter = await this.interpreterService.getActiveInterpreter();
-            if (activeInterpreter) {
-                const winPath = await getRealPath(
-                    this.file,
-                    this.exeFactory,
-                    activeInterpreter.path,
-                    path.join(this.pathUtils.home, winJupyterPath)
-                );
-                if (winPath) {
-                    paths = [winPath];
-                }
-
-                if (jupyterPathVar !== '') {
-                    const jupyterPaths = jupyterPathVar.split(path.delimiter);
-                    jupyterPaths.forEach(async (jupyterPath) => {
+        const jupyterPathVars = vars.JUPYTER_PATH
+            ? vars.JUPYTER_PATH.split(path.delimiter).map((jupyterPath) => {
+                  return path.join(jupyterPath, 'kernels');
+              })
+            : [];
+
+        if (jupyterPathVars.length > 0) {
+            if (this.platformService.isWindows) {
+                const activeInterpreter = await this.interpreterService.getActiveInterpreter();
+                if (activeInterpreter) {
+                    jupyterPathVars.forEach(async (jupyterPath) => {
                         const jupyterWinPath = await getRealPath(
                             this.file,
                             this.exeFactory,
@@ -266,13 +262,36 @@ export class KernelFinder implements IKernelFinder {
                             paths.push(jupyterWinPath);
                         }
                     });
+                } else {
+                    paths.push(...jupyterPathVars);
                 }
             } else {
-                paths = [path.join(this.pathUtils.home, winJupyterPath)];
-                if (jupyterPathVar !== '') {
-                    const jupyterPaths = jupyterPathVar.split(path.delimiter);
-                    paths.push(...jupyterPaths);
+                // Unix based
+                paths.push(...jupyterPathVars);
+            }
+        }
+
+        return paths;
+    }
+
+    private async getDiskPaths(): Promise<string[]> {
+        // Paths specified in JUPYTER_PATH are supposed to come first in searching
+        const paths: string[] = await this.getJupyterPathPaths();
+
+        if (this.platformService.isWindows) {
+            const activeInterpreter = await this.interpreterService.getActiveInterpreter();
+            if (activeInterpreter) {
+                const winPath = await getRealPath(
+                    this.file,
+                    this.exeFactory,
+                    activeInterpreter.path,
+                    path.join(this.pathUtils.home, winJupyterPath)
+                );
+                if (winPath) {
+                    paths.push(winPath);
                 }
+            } else {
+                paths.push(path.join(this.pathUtils.home, winJupyterPath));
             }
 
             if (process.env.ALLUSERSPROFILE) {
@@ -282,16 +301,11 @@ export class KernelFinder implements IKernelFinder {
             // Unix based
             const secondPart = this.platformService.isMac ? macJupyterPath : linuxJupyterPath;
 
-            paths = [
+            paths.push(
                 path.join('usr', 'share', 'jupyter', 'kernels'),
                 path.join('usr', 'local', 'share', 'jupyter', 'kernels'),
                 path.join(this.pathUtils.home, secondPart)
-            ];
-
-            if (jupyterPathVar !== '') {
-                const jupyterPaths = jupyterPathVar.split(path.delimiter);
-                paths.push(...jupyterPaths);
-            }
+            );
         }
 
         return paths;

src/test/datascience/kernelFinder.unit.test.ts

@@ -51,7 +51,7 @@ suite('Kernel Finder', () => {
         specFile: path.join('1', 'share', 'jupyter', 'kernels', kernelName, 'kernel.json')
     };
     // Change this to your actual JUPYTER_PATH value and see it appearing on the paths in the kernelFinder
-    const JupyterPathEnvVar = '';
+    let JupyterPathEnvVar = '';
 
     function setupFileSystem() {
         fileSystem
@@ -103,8 +103,12 @@ suite('Kernel Finder', () => {
         let interpreter0Kernel: IJupyterKernelSpec;
         let interpreter1Kernel: IJupyterKernelSpec;
         let globalKernel: IJupyterKernelSpec;
+        let jupyterPathKernelA: IJupyterKernelSpec;
+        let jupyterPathKernelB: IJupyterKernelSpec;
         let loadError = false;
         setup(() => {
+            JupyterPathEnvVar = `Users/testuser/jupyterPathDirA${path.delimiter}Users/testuser/jupyterPathDirB`;
+
             activeInterpreter = {
                 path: context.object.globalStoragePath,
                 displayName: 'activeInterpreter',
@@ -191,6 +195,26 @@ suite('Kernel Finder', () => {
                 argv: ['<python path>', '-m', 'ipykernel_launcher', '-f', '{connection_file}']
             };
 
+            jupyterPathKernelA = {
+                name: 'jupyterPathKernelA',
+                language: 'python',
+                path: '<python path>',
+                display_name: 'Python 3',
+                metadata: {},
+                env: {},
+                argv: ['<python path>', '-m', 'ipykernel_launcher', '-f', '{connection_file}']
+            };
+
+            jupyterPathKernelB = {
+                name: 'jupyterPathKernelB',
+                language: 'python',
+                path: '<python path>',
+                display_name: 'Python 3',
+                metadata: {},
+                env: {},
+                argv: ['<python path>', '-m', 'ipykernel_launcher', '-f', '{connection_file}']
+            };
+
             platformService.reset();
             platformService.setup((ps) => ps.isWindows).returns(() => false);
             platformService.setup((ps) => ps.isMac).returns(() => true);
@@ -229,6 +253,7 @@ suite('Kernel Finder', () => {
                 .setup((fs) => fs.search(typemoq.It.isAnyString(), interpreter1Path, typemoq.It.isAny()))
                 .returns(() => Promise.resolve([path.join(interpreter1Kernel.name, 'kernel.json')]));
 
+            // Global path setup
             const globalPath = path.join('usr', 'share', 'jupyter', 'kernels');
             const globalFullPath = path.join(globalPath, globalKernel.name, 'kernel.json');
             fileSystem
@@ -245,6 +270,26 @@ suite('Kernel Finder', () => {
                 .setup((fs) => fs.search(typemoq.It.isAnyString(), globalBPath, typemoq.It.isAny()))
                 .returns(() => Promise.resolve([]));
 
+            // Jupyter path setup
+            const jupyterPathKernelAPath = path.join('Users', 'testuser', 'jupyterPathDirA', 'kernels');
+            const jupyterPathKernelAFullPath = path.join(
+                jupyterPathKernelAPath,
+                jupyterPathKernelA.name,
+                'kernel.json'
+            );
+            const jupyterPathKernelBPath = path.join('Users', 'testuser', 'jupyterPathDirB', 'kernels');
+            const jupyterPathKernelBFullPath = path.join(
+                jupyterPathKernelBPath,
+                jupyterPathKernelB.name,
+                'kernel.json'
+            );
+            fileSystem
+                .setup((fs) => fs.search(typemoq.It.isAnyString(), jupyterPathKernelAPath, typemoq.It.isAny()))
+                .returns(() => Promise.resolve([path.join(jupyterPathKernelA.name, 'kernel.json')]));
+            fileSystem
+                .setup((fs) => fs.search(typemoq.It.isAnyString(), jupyterPathKernelBPath, typemoq.It.isAny()))
+                .returns(() => Promise.resolve([path.join(jupyterPathKernelB.name, 'kernel.json')]));
+
             // Set the file system to return our kernelspec json
             fileSystem
                 .setup((fs) => fs.readFile(typemoq.It.isAnyString()))
@@ -264,6 +309,10 @@ suite('Kernel Finder', () => {
                             return Promise.resolve(JSON.stringify(interpreter1Kernel));
                         case globalFullPath:
                             return Promise.resolve(JSON.stringify(globalKernel));
+                        case jupyterPathKernelAFullPath:
+                            return Promise.resolve(JSON.stringify(jupyterPathKernelA));
+                        case jupyterPathKernelBFullPath:
+                            return Promise.resolve(JSON.stringify(jupyterPathKernelB));
                         default:
                             return Promise.resolve('');
                     }
@@ -292,7 +341,9 @@ suite('Kernel Finder', () => {
             expect(specs[1]).to.deep.include(activeKernelB);
             expect(specs[2]).to.deep.include(interpreter0Kernel);
             expect(specs[3]).to.deep.include(interpreter1Kernel);
-            expect(specs[4]).to.deep.include(globalKernel);
+            expect(specs[4]).to.deep.include(jupyterPathKernelA);
+            expect(specs[5]).to.deep.include(jupyterPathKernelB);
+            expect(specs[6]).to.deep.include(globalKernel);
             fileSystem.reset();
         });
 
@@ -303,7 +354,9 @@ suite('Kernel Finder', () => {
             expect(specs[0]).to.deep.include(activeKernelB);
             expect(specs[1]).to.deep.include(interpreter0Kernel);
             expect(specs[2]).to.deep.include(interpreter1Kernel);
-            expect(specs[3]).to.deep.include(globalKernel);
+            expect(specs[3]).to.deep.include(jupyterPathKernelA);
+            expect(specs[4]).to.deep.include(jupyterPathKernelB);
+            expect(specs[5]).to.deep.include(globalKernel);
             fileSystem.reset();
         });
     });