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.

Sign up for GitHub

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

Add component debugger #726

Merged
merged 12 commits into from
Feb 17, 2021
Merged

Add component debugger #726

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
ce5ccd6
Add component debugger
chsienki Feb 9, 2021
b079869
PR Feedback
chsienki Feb 12, 2021
5ef7fee
Address PROTOTYPE: comments:
chsienki Feb 16, 2021
f2ddb61
Add package source
chsienki Feb 16, 2021
c86f8bd
Use async lazy to get compiler root from the UI thread
chsienki Feb 16, 2021
92e3b89
NuGoop
chsienki Feb 16, 2021
ffd65a0
Add cap provider for VB
chsienki Feb 16, 2021
f78a1b7
Make capability provider dataflow based
chsienki Feb 17, 2021
492c07c
Don't warn on downgrade
chsienki Feb 17, 2021
65f8b41
cries in nuget
chsienki Feb 17, 2021
2af0f11
nuget harder
chsienki Feb 17, 2021
4dcffa0
Nuget 4.0
chsienki Feb 17, 2021
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
22 changes: 9 additions & 13 deletions src/VisualStudio.Roslyn.SDK/ComponentDebugger/CapabilityProvider.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,34 +14,30 @@ namespace Roslyn.ComponentDebugger
[AppliesTo(ProjectCapabilities.CSharp + " | " + ProjectCapabilities.VB)]
Copy link
Member

@davkean davkean Feb 17, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

This component will load for every C# and VB project, we need a better approach for this. Is there something unique about these projects?

Copy link
Member

Choose a reason for hiding this comment

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

This also loads for Shared Projects.

Copy link
Member Author

Choose a reason for hiding this comment

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

It can apply to any project that has the <IsRoslyComponent>true</IsRoslynComponent>. So we load the provider for all projects and add the capability to any that match that.

Is there some better way to achieve that? I mentioned in the comments we could do it via the targets, but that does require a matching roslyn version.

Copy link
Member

Choose a reason for hiding this comment

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

Is there a targets file that it imports we can match on?

Copy link
Member Author

Choose a reason for hiding this comment

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

No, its just a regular old csharp project.

Copy link
Member

Choose a reason for hiding this comment

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

I mentioned in the comments we could do it via the targets, but that does require a matching roslyn version.

Can you expand on what you mean by this? You mean if the project itself has a package reference to a different version of the compiler?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah. We could drop the capability provider altogether if we just added the following to the targets file

<ItemGroup Condition="'$(IsRoslynComponent)' == 'true'">
    <Capability Include="RoslynComponent">
</ItemGroup>

But anyone using an older version of the compiler via NuGet wouldn't get the capability, and be able to use it. That might be worth the tradeoff to remove some of the complexity here tho.

public class CapabilityProvider : ConfiguredProjectCapabilitiesProviderBase
{
private readonly IProjectSnapshotService snapshotService;

[ImportingConstructor]
[System.Obsolete("This exported object must be obtained through the MEF export provider.", error: true)]
public CapabilityProvider(ConfiguredProject configuredProject)
public CapabilityProvider(ConfiguredProject configuredProject, IProjectSnapshotService snapshotService)
: base(nameof(CapabilityProvider), configuredProject)
{
this.snapshotService = snapshotService;
}

protected override async Task<ImmutableHashSet<string>> GetCapabilitiesAsync(CancellationToken cancellationToken)
{
// an alternative design could be to have 'IsRoslynComponent' just define the <Capability... directly in the managed.core targets
// but that would require a specific roslyn version to work, this allows it to be backwards compatible with older SDKs
var caps = Empty.CapabilitiesSet;
if (await IsRoslynComponentAsync(this.ConfiguredProject, cancellationToken).ConfigureAwait(false))

var snapshot = await snapshotService.GetLatestVersionAsync(ConfiguredProject, cancellationToken: cancellationToken).ConfigureAwait(false);
Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks @davkean and @tmeschter. I switched the cap provider over to this, which is the same way the 'get command line args' stuff works. Is that a valid way to handle getting it via dataflow?

Copy link
Member

@davkean davkean Feb 17, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

No. This needs to chain similar to what I pointed out here: https://github.com/dotnet/project-system/blob/feature/AspNet/src/Microsoft.VisualStudio.ProjectSystem.Managed.VS/ProjectSystem/VS/Web/ProjectTypeGuidsDataSource.cs.

These systems flow data from one data source -> another, you need to transform the data as it flows along the pipe. This code just grabs the latest version from the snapshot pipe and sets up no relationship between the pipes, so we never know to call this pipe when the data changes.

Copy link
Member Author

Choose a reason for hiding this comment

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

But isn't that handled by the base class? It sets up the pipe stuff and calls this method as it changes? I guess thats the bit I don't understand.

Copy link
Member

Choose a reason for hiding this comment

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

No. Here you basically saying "I am the start of a pipe, and I will tell you when there are changes". What you want to say is "My pipe is based on the snapshot data, so when it changes, call me".

Copy link
Member

Choose a reason for hiding this comment

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

This stuff is complicated and hard to get right - I can sit down and write it with you.

var isRoslynComponentProperty = snapshot.Value.ProjectInstance.GetPropertyValue(Constants.RoslynComponentPropertyName);
var isComponent = string.Compare(isRoslynComponentProperty.Trim(), "true", System.StringComparison.OrdinalIgnoreCase) == 0;
if (isComponent)
{
caps = caps.Add(Constants.RoslynComponentCapability);
}
return caps;
}

private static Task<bool> IsRoslynComponentAsync(ConfiguredProject configuredProject, CancellationToken token = default)
=> configuredProject.Services.ProjectLockService.ReadLockAsync(
async access =>
{
var project = await access.GetProjectAsync(configuredProject).ConfigureAwait(false);
var isRoslynComponentProperty = project.GetProperty(Constants.RoslynComponentPropertyName);
var isComponent = string.Compare(isRoslynComponentProperty?.EvaluatedValue.Trim(), "true", System.StringComparison.OrdinalIgnoreCase) == 0;
return isComponent;
},
token);
}
}
19 changes: 8 additions & 11 deletions src/VisualStudio.Roslyn.SDK/ComponentDebugger/LaunchSettingsProvider.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,18 +71,15 @@ private async Task<DebuggerOptionsViewModel> GetViewModelAsync()
var targetProject = await targetProjectUnconfigured.LoadConfiguredProjectAsync(configuredProject.ProjectConfiguration).ConfigureAwait(false);
if (targetProject is object)
{
// https://github.com/dotnet/roslyn-sdk/issues/731: the below is deadlocking on certain projects. for now just list them all
targetProjects.Add(targetProject);

// check if the args contain the project as an analyzer ref
//foreach (var arg in await targetProject.GetCompilationArgumentsAsync().ConfigureAwait(false))
//{
// if (arg.StartsWith("/analyzer", StringComparison.OrdinalIgnoreCase)
// && arg.EndsWith(target, StringComparison.OrdinalIgnoreCase))
// {
// targetProjects.Add(targetProject);
// }
//}
foreach (var arg in await targetProject.GetCompilationArgumentsAsync().ConfigureAwait(false))
{
if (arg.StartsWith("/analyzer", StringComparison.OrdinalIgnoreCase)
&& arg.EndsWith(target, StringComparison.OrdinalIgnoreCase))
{
targetProjects.Add(targetProject);
}
}
}
}
}
Expand Down