-
Notifications
You must be signed in to change notification settings - Fork 237
Add pathMappings option to debugger #2251
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -15,7 +15,6 @@ | |||||||||||
using Microsoft.PowerShell.EditorServices.Services.PowerShell.Execution; | ||||||||||||
using Microsoft.PowerShell.EditorServices.Services.PowerShell.Host; | ||||||||||||
using Microsoft.PowerShell.EditorServices.Services.PowerShell.Utility; | ||||||||||||
using Microsoft.PowerShell.EditorServices.Services.TextDocument; | ||||||||||||
using Microsoft.PowerShell.EditorServices.Utility; | ||||||||||||
|
||||||||||||
namespace Microsoft.PowerShell.EditorServices.Services | ||||||||||||
|
@@ -49,6 +48,7 @@ internal class DebugService | |||||||||||
private VariableContainerDetails scriptScopeVariables; | ||||||||||||
private VariableContainerDetails localScopeVariables; | ||||||||||||
private StackFrameDetails[] stackFrameDetails; | ||||||||||||
private PathMapping[] _pathMappings; | ||||||||||||
|
||||||||||||
private readonly SemaphoreSlim debugInfoHandle = AsyncUtils.CreateSimpleLockingSemaphore(); | ||||||||||||
#endregion | ||||||||||||
|
@@ -123,22 +123,22 @@ public DebugService( | |||||||||||
/// <summary> | ||||||||||||
/// Sets the list of line breakpoints for the current debugging session. | ||||||||||||
/// </summary> | ||||||||||||
/// <param name="scriptFile">The ScriptFile in which breakpoints will be set.</param> | ||||||||||||
/// <param name="scriptPath">The path in which breakpoints will be set.</param> | ||||||||||||
/// <param name="breakpoints">BreakpointDetails for each breakpoint that will be set.</param> | ||||||||||||
/// <param name="clearExisting">If true, causes all existing breakpoints to be cleared before setting new ones.</param> | ||||||||||||
/// <param name="skipRemoteMapping">If true, skips the remote file manager mapping of the script path.</param> | ||||||||||||
/// <returns>An awaitable Task that will provide details about the breakpoints that were set.</returns> | ||||||||||||
public async Task<IReadOnlyList<BreakpointDetails>> SetLineBreakpointsAsync( | ||||||||||||
ScriptFile scriptFile, | ||||||||||||
string scriptPath, | ||||||||||||
IReadOnlyList<BreakpointDetails> breakpoints, | ||||||||||||
bool clearExisting = true) | ||||||||||||
bool clearExisting = true, | ||||||||||||
bool skipRemoteMapping = false) | ||||||||||||
{ | ||||||||||||
DscBreakpointCapability dscBreakpoints = await _debugContext.GetDscBreakpointCapabilityAsync().ConfigureAwait(false); | ||||||||||||
|
||||||||||||
string scriptPath = scriptFile.FilePath; | ||||||||||||
|
||||||||||||
_psesHost.Runspace.ThrowCancelledIfUnusable(); | ||||||||||||
// Make sure we're using the remote script path | ||||||||||||
if (_psesHost.CurrentRunspace.IsOnRemoteMachine && _remoteFileManager is not null) | ||||||||||||
if (!skipRemoteMapping && _psesHost.CurrentRunspace.IsOnRemoteMachine && _remoteFileManager is not null) | ||||||||||||
{ | ||||||||||||
if (!_remoteFileManager.IsUnderRemoteTempPath(scriptPath)) | ||||||||||||
{ | ||||||||||||
|
@@ -162,7 +162,7 @@ public async Task<IReadOnlyList<BreakpointDetails>> SetLineBreakpointsAsync( | |||||||||||
{ | ||||||||||||
if (clearExisting) | ||||||||||||
{ | ||||||||||||
await _breakpointService.RemoveAllBreakpointsAsync(scriptFile.FilePath).ConfigureAwait(false); | ||||||||||||
await _breakpointService.RemoveAllBreakpointsAsync(scriptPath).ConfigureAwait(false); | ||||||||||||
} | ||||||||||||
|
||||||||||||
return await _breakpointService.SetBreakpointsAsync(breakpoints).ConfigureAwait(false); | ||||||||||||
|
@@ -603,6 +603,59 @@ public VariableScope[] GetVariableScopes(int stackFrameId) | |||||||||||
}; | ||||||||||||
} | ||||||||||||
|
||||||||||||
internal void SetPathMappings(PathMapping[] pathMappings) => _pathMappings = pathMappings; | ||||||||||||
|
||||||||||||
internal void UnsetPathMappings() => _pathMappings = null; | ||||||||||||
|
||||||||||||
internal bool TryGetMappedLocalPath(string remotePath, out string localPath) | ||||||||||||
{ | ||||||||||||
if (_pathMappings is not null) | ||||||||||||
{ | ||||||||||||
foreach (PathMapping mapping in _pathMappings) | ||||||||||||
{ | ||||||||||||
if (string.IsNullOrWhiteSpace(mapping.LocalRoot) || string.IsNullOrWhiteSpace(mapping.RemoteRoot)) | ||||||||||||
{ | ||||||||||||
// If either path mapping is null, we can't map the path. | ||||||||||||
continue; | ||||||||||||
} | ||||||||||||
|
||||||||||||
if (remotePath.StartsWith(mapping.RemoteRoot, StringComparison.OrdinalIgnoreCase)) | ||||||||||||
{ | ||||||||||||
localPath = mapping.LocalRoot + remotePath.Substring(mapping.RemoteRoot.Length); | ||||||||||||
return true; | ||||||||||||
} | ||||||||||||
} | ||||||||||||
} | ||||||||||||
|
||||||||||||
localPath = null; | ||||||||||||
return false; | ||||||||||||
} | ||||||||||||
|
||||||||||||
internal bool TryGetMappedRemotePath(string localPath, out string remotePath) | ||||||||||||
{ | ||||||||||||
if (_pathMappings is not null) | ||||||||||||
{ | ||||||||||||
foreach (PathMapping mapping in _pathMappings) | ||||||||||||
{ | ||||||||||||
if (string.IsNullOrWhiteSpace(mapping.LocalRoot) || string.IsNullOrWhiteSpace(mapping.RemoteRoot)) | ||||||||||||
{ | ||||||||||||
// If either path mapping is null, we can't map the path. | ||||||||||||
continue; | ||||||||||||
} | ||||||||||||
|
||||||||||||
if (localPath.StartsWith(mapping.LocalRoot, StringComparison.OrdinalIgnoreCase)) | ||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same path comparison issue as with TryGetMappedLocalPath. Using StartsWith without proper path boundary checking could lead to incorrect path mappings.
Suggested change
Copilot uses AI. Check for mistakes. Positive FeedbackNegative Feedback There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. See #2251 (comment) |
||||||||||||
{ | ||||||||||||
// If the local path starts with the local path mapping, we can replace it with the remote path. | ||||||||||||
remotePath = mapping.RemoteRoot + localPath.Substring(mapping.LocalRoot.Length); | ||||||||||||
return true; | ||||||||||||
} | ||||||||||||
} | ||||||||||||
} | ||||||||||||
|
||||||||||||
remotePath = null; | ||||||||||||
return false; | ||||||||||||
} | ||||||||||||
|
||||||||||||
#endregion | ||||||||||||
|
||||||||||||
#region Private Methods | ||||||||||||
|
@@ -873,14 +926,19 @@ private async Task FetchStackFramesAsync(string scriptNameOverride) | |||||||||||
StackFrameDetails stackFrameDetailsEntry = StackFrameDetails.Create(callStackFrame, autoVariables, commandVariables); | ||||||||||||
string stackFrameScriptPath = stackFrameDetailsEntry.ScriptPath; | ||||||||||||
|
||||||||||||
if (scriptNameOverride is not null | ||||||||||||
&& string.Equals(stackFrameScriptPath, StackFrameDetails.NoFileScriptPath)) | ||||||||||||
bool isNoScriptPath = string.Equals(stackFrameScriptPath, StackFrameDetails.NoFileScriptPath); | ||||||||||||
if (scriptNameOverride is not null && isNoScriptPath) | ||||||||||||
{ | ||||||||||||
stackFrameDetailsEntry.ScriptPath = scriptNameOverride; | ||||||||||||
} | ||||||||||||
else if (TryGetMappedLocalPath(stackFrameScriptPath, out string localMappedPath) | ||||||||||||
&& !isNoScriptPath) | ||||||||||||
{ | ||||||||||||
stackFrameDetailsEntry.ScriptPath = localMappedPath; | ||||||||||||
} | ||||||||||||
else if (_psesHost.CurrentRunspace.IsOnRemoteMachine | ||||||||||||
&& _remoteFileManager is not null | ||||||||||||
&& !string.Equals(stackFrameScriptPath, StackFrameDetails.NoFileScriptPath)) | ||||||||||||
&& !isNoScriptPath) | ||||||||||||
{ | ||||||||||||
stackFrameDetailsEntry.ScriptPath = | ||||||||||||
_remoteFileManager.GetMappedPath(stackFrameScriptPath, _psesHost.CurrentRunspace); | ||||||||||||
|
@@ -981,9 +1039,13 @@ await _executionService.ExecutePSCommandAsync<PSObject>( | |||||||||||
// Begin call stack and variables fetch. We don't need to block here. | ||||||||||||
StackFramesAndVariablesFetched = FetchStackFramesAndVariablesAsync(noScriptName ? localScriptPath : null); | ||||||||||||
|
||||||||||||
if (!noScriptName && TryGetMappedLocalPath(e.InvocationInfo.ScriptName, out string mappedLocalPath)) | ||||||||||||
{ | ||||||||||||
localScriptPath = mappedLocalPath; | ||||||||||||
} | ||||||||||||
// If this is a remote connection and the debugger stopped at a line | ||||||||||||
// in a script file, get the file contents | ||||||||||||
if (_psesHost.CurrentRunspace.IsOnRemoteMachine | ||||||||||||
else if (_psesHost.CurrentRunspace.IsOnRemoteMachine | ||||||||||||
&& _remoteFileManager is not null | ||||||||||||
&& !noScriptName) | ||||||||||||
{ | ||||||||||||
|
@@ -1034,8 +1096,12 @@ private void OnBreakpointUpdated(object sender, BreakpointUpdatedEventArgs e) | |||||||||||
{ | ||||||||||||
// TODO: This could be either a path or a script block! | ||||||||||||
string scriptPath = lineBreakpoint.Script; | ||||||||||||
if (_psesHost.CurrentRunspace.IsOnRemoteMachine | ||||||||||||
&& _remoteFileManager is not null) | ||||||||||||
if (TryGetMappedLocalPath(scriptPath, out string mappedLocalPath)) | ||||||||||||
{ | ||||||||||||
scriptPath = mappedLocalPath; | ||||||||||||
} | ||||||||||||
else if (_psesHost.CurrentRunspace.IsOnRemoteMachine | ||||||||||||
&& _remoteFileManager is not null) | ||||||||||||
{ | ||||||||||||
string mappedPath = _remoteFileManager.GetMappedPath(scriptPath, _psesHost.CurrentRunspace); | ||||||||||||
|
||||||||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Path comparison using StartsWith without ensuring proper path separators could lead to incorrect matches. For example, '/home/user' would incorrectly match '/home/username/file.ps1'. Consider using proper path comparison logic that respects directory boundaries.
Copilot uses AI. Check for mistakes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In this case it is use supplied values. if they wish to not use a directory separator at the end of the
localRoot
orremoteRoot
then that's their decision. The path mappings is designed to replace the root substring with the specified equivalent.