Skip to content

Commit

Permalink
Fix for issue microsoft#11396
Browse files Browse the repository at this point in the history
* During notebook cell debugging, apply path normalization on the path of the dumped tmp file only if the kernel runs on a Windows machine.
  • Loading branch information
Ádám Zlatniczki committed Oct 2, 2022
1 parent 06ca869 commit f665497
Show file tree
Hide file tree
Showing 5 changed files with 51 additions and 5 deletions.
2 changes: 2 additions & 0 deletions news/2 Fixes/11396.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Fixed a bug that introduced a cross-OS interworking issue, making the notebook cell debugger skip breakpoints when VSCode is run on Windows but the kernel is run on Unix
(thanks [Adam Zlatniczki](https://github.com/adam-zlatniczki))
6 changes: 4 additions & 2 deletions src/interactive-window/debugger/jupyter/kernelDebugAdapter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ export class KernelDebugAdapter extends KernelDebugAdapterBase {
const response = await this.session.customRequest('dumpCell', { code });

// We know jupyter will strip out leading white spaces, hence take that into account.
const norm = path.normalize((response as IDumpCellResponse).sourcePath);
const norm = KernelDebugAdapterBase.extractDumpFilePathOnKernelSide(response as IDumpCellResponse);
this.fileToCell.set(norm, Uri.parse(metadata.interactive.uristring));

// If this cell doesn't have a cell marker, then
Expand Down Expand Up @@ -131,8 +131,10 @@ export class KernelDebugAdapter extends KernelDebugAdapterBase {
if (!cell) {
return;
}

source.name = path.basename(cell.interactiveWindow.path);
source.path = cell.interactiveWindow.toString();
source.path = cell.interactiveWindow.path;

if (typeof lines?.endLine === 'number') {
lines.endLine = lines.endLine + (cell.lineOffset || 0);
}
Expand Down
5 changes: 2 additions & 3 deletions src/notebooks/debugger/kernelDebugAdapter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,10 @@

'use strict';

import * as path from '../../platform/vscode-path/path';
import { IDumpCellResponse } from './debuggingTypes';
import { traceError } from '../../platform/logging';
import { KernelDebugAdapterBase } from './kernelDebugAdapterBase';
import { DebugProtocol } from 'vscode-debugprotocol';
import { IDumpCellResponse } from './debuggingTypes';

/**
* Concrete implementation of the KernelDebugAdapterBase class that will dump cells
Expand All @@ -22,7 +21,7 @@ export class KernelDebugAdapter extends KernelDebugAdapterBase {
const response = await this.session.customRequest('dumpCell', {
code: cell.document.getText().replace(/\r\n/g, '\n')
});
const norm = path.normalize((response as IDumpCellResponse).sourcePath);
const norm = KernelDebugAdapterBase.extractDumpFilePathOnKernelSide(response as IDumpCellResponse);
this.fileToCell.set(norm, cell.document.uri);
this.cellToFile.set(cell.document.uri.toString(), norm);
} catch (err) {
Expand Down
15 changes: 15 additions & 0 deletions src/notebooks/debugger/kernelDebugAdapterBase.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import {
Uri,
workspace
} from 'vscode';
import { IDumpCellResponse } from './debuggingTypes';
import { DebugProtocol } from 'vscode-debugprotocol';
import { executeSilently } from '../../kernels/helpers';
import { IKernel, IKernelConnectionSession } from '../../kernels/types';
Expand Down Expand Up @@ -264,6 +265,20 @@ export abstract class KernelDebugAdapterBase implements DebugAdapter, IKernelDeb
});
}

static extractDumpFilePathOnKernelSide(dumpCellResponse: IDumpCellResponse) {
// Based on the response, check if the kernel runs on a Unix or Windows machine.
// If it runs on Windows, then path windows-style normalization might be necessary
// (see https://github.com/ipython/ipykernel/issues/995).
const dumpFilePathOnKernelSide = dumpCellResponse.sourcePath;
let norm = '';
if (dumpFilePathOnKernelSide.match(/^[A-Za-z]:/)) {
norm = path.win32.normalize(dumpFilePathOnKernelSide);
} else {
norm = dumpFilePathOnKernelSide;
}
return norm;
}

private lookupCellByLongName(sourcePath: string) {
if (!this.platformService.isWindows) {
return undefined;
Expand Down
28 changes: 28 additions & 0 deletions src/test/debugger/jupyter/kernelDebugAdapter.unit.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.

import { expect } from 'chai';
import { KernelDebugAdapterBase } from '../../../notebooks/debugger/kernelDebugAdapterBase';
import { IDumpCellResponse } from '../../../notebooks/debugger/debuggingTypes';

suite('Debugging - KernelDebugAdapterBase', () => {
suite('extractDumpFilePathOnKernelSide', async () => {
test('Kernel runs on Windows backend', () => {
const mockResponseFromKernel = { sourcePath: 'c:\\tmp\\1.py' } as IDumpCellResponse;
const path = KernelDebugAdapterBase.extractDumpFilePathOnKernelSide(mockResponseFromKernel);
expect(path).to.equal('c:\\tmp\\1.py');
});

test('Kernel runs on Windows backend with ipykernel issue', () => {
const mockResponseFromKernel = { sourcePath: 'c:\\tmp/1.py' } as IDumpCellResponse;
const path = KernelDebugAdapterBase.extractDumpFilePathOnKernelSide(mockResponseFromKernel);
expect(path).to.equal('c:\\tmp\\1.py');
});

test('Kernel runs on Unix backend', () => {
const mockResponseFromKernel = { sourcePath: '/tmp/1.py' } as IDumpCellResponse;
const path = KernelDebugAdapterBase.extractDumpFilePathOnKernelSide(mockResponseFromKernel);
expect(path).to.equal('/tmp/1.py');
});
});
});

0 comments on commit f665497

Please sign in to comment.