microsoft / DevSkim

DevSkim is a set of IDE plugins, language analyzers, and rules that provide security "linting" capabilities.
MIT License
902 stars 114 forks source link

Remove `.NET Install Tool for Extension Authors` dependency #565

Closed NewUserHa closed 1 year ago

NewUserHa commented 1 year ago

Is your feature request related to a problem? Please describe. .NET Install Tool for Extension Authors is installed. dependency

Describe the solution you'd like Since .NET Install Tool for Extension Authors declare This extension allows acquisition of the .NET runtime specifically for Visual Studio Code extension authors. ... The extension is not intended to be used directly by users to install .NET for development. in its readme, Can remove .NET Install Tool for Extension Authors dependency?

gfs commented 1 year ago

Sorry for the confusion, the .NET runtime dependency is in this case being used appropriately by us, the extension developers, to fetch the .NET dependency required for the functioning of the language server component of the extension.

The description of the install tool is to indicate it's not intended for end users to install independently of its inclusion as a dependency to another extension.

Here's a documentation page with more detail on this dependency. https://learn.microsoft.com/en-us/dotnet/core/additional-tools/vscode-dotnet-runtime

The .NET install tool for extension authors is a Visual Studio Code extension that allows acquisition of the .NET runtime specifically for VS Code extension authors. This tool is intended to be leveraged in extensions that are written in .NET and require .NET to boot pieces of the extension (for example, a language server). The extension is not intended to be used directly by users to install .NET for development.

NewUserHa commented 1 year ago

I thought it was just used for extension development.

But that dependency extension will without asking download another .NET 7.0 every time when vscode started. Is it able to let/ask the users to select to use the existing installed .NET SDK to avoid this behavior and this dependency?

gfs commented 1 year ago

But that dependency extension will without asking download another .NET 7.0 every time when vscode started.

I don't observe that it is redownloading the .NET runtime on every launch, only on first launch of the extension. On subsequent launches it will use the previously obtained runtime. Can you share more information about your configuration where you observe this behavior?

Is it able to let/ask the users to select to use the existing installed .NET SDK to avoid this behavior and this dependency?

We could theoretically allow users to configure the .NET path to use in the options, but I'm not sure I see much of a benefit to this - as by the time you can get to the settings.json to set it the extension will have already started and will have started installing the runtime via the .NET Install Tool dependency.

Simultaneously, it also makes the extension more brittle (when we for example switch to a new .NET version, as we will with .NET 8.0, the user would have to know to modify their settings to a different version of .NET). In either case, the dependency itself would still be installed in VS Code, as its defined in the package.json for the DevSkim extension.

NewUserHa commented 1 year ago

Can you share more information about your configuration where you observe this behavior?

In my env, .Net 6.0 is installed. And .Net 7.0 is un able to download in the env.

This issue is created actually by a big 'Error acquiring .NET!' red button in the status bar when every time vscode started, and it immediately brings the output 'Downloading the .NET Runtime.' of this extension to the front.

Simultaneously, it also makes the extension more brittle (when we for example switch to a new .NET version, as we will with .NET 8.0, the user would have to know to modify their settings to a different version of .NET).

Installing a new .NET SDK will consume the users more than 300MB-500MB of disk space without asking. What if use the AOT feature of .NET, or get rid of the version requirement of .Net SDK if users have already installed some?

So the question currently actually is installing something heavy directly other than poping up a window and asking users first. eg: the python extension will ask users if install a formatter(<<1MB); eg: the golang extension will ask users if install the gopls and staticcheck and etc.(<50MB)

But the extension just started to install a .NET SDK that can eat many disk space and may probably mess the already installed .NET

gfs commented 1 year ago

Can you share more information about your configuration where you observe this behavior?

In my env, .Net 6.0 is installed. And .Net 7.0 is un able to download in the env.

This issue is created actually by a big 'Error acquiring .NET!' red button in the status bar when every time vscode started, and it immediately brings the output 'Downloading the .NET Runtime.' of this extension to the front.

I see. You'd not mentioned before that it fails to download. Does it provide any additional information in the Output Window about why it fails? I have tested with only the .NET 6.0 SDK present and the extension does not interfere with it.

As for trying to redownload, if there is no version of .NET fetched it of course have to try to obtain a new one.

In any case, in your scenario you would still have to install the .NET 7.0 runtime, just manually, so there's no net saving of disk space. The DevSkim language server bundled with the extension is only built for .NET 7.

Simultaneously, it also makes the extension more brittle (when we for example switch to a new .NET version, as we will with .NET 8.0, the user would have to know to modify their settings to a different version of .NET).

Installing a new .NET SDK will consume the users more than 300MB-500MB of disk space without asking. What if use the AOT feature of .NET, or get rid of the version requirement of .Net SDK if users have already installed some?

As noted in your error message, we are fetching the Runtime not the SDK. It looks like on Windows AMD64 that it consumes about 90MB of disk space. As noted above, in your scenario we would still have to download the runtime because you do not have .NET 7.

Secondly, I tried building the LanguageServer with AOT, but there appear to be a number of dependencies not compatible with the AOT build/trimmer. Finally, even if we could resolve the trimming issues using the AOT feature wouldn't help much here in terms of disk space, when built without AOT the language server is about 10MB, with AOT its 40 MB, so at most you're saving ~ 60 MB of disk space - which would be less after fixing all the excess trimming (if it is even possible to do so).

This would also add significant complexity (like needing to build at least 6 different VSIX bundles for various architecture/platform combos) and make debugging significantly more difficult.

So the question currently actually is installing something heavy directly other than poping up a window and asking users first. eg: the python extension will ask users if install a formatter(<<1MB); eg: the golang extension will ask users if install the gopls and staticcheck and etc.(<50MB)

Are those components essential to the functioning of those extensions or do they enable additional features? The DevSkim extension cannot perform any actions without the language server.

But the extension just started to install a .NET SDK that can eat many disk space and may probably mess the already installed .NET

It won't mess with your existing .NET. The extension doesn't install .NET on your path. On windows, it installs it to C:\Users\USERNAME\AppData\Roaming\Code\User\globalStorage\ms-dotnettools.vscode-dotnet-runtime\.dotnet.

NewUserHa commented 1 year ago

I see. You'd not mentioned before that it fails to download. Does it provide any additional information in the Output Window about why it fails? I have tested with only the .NET 6.0 SDK present and the extension does not interfere with it.

no, the output is

Downloading the .NET Runtime.
Downloading .NET version(s) 7.0.8 ........................................................................................................................... Error!
Failed to download .NET 7.0.8:
.NET installation timed out.

 Error!
.NET Acquisition Failed: Installation failed: Error: .NET installation timed out.

In any case, in your scenario you would still have to install the .NET 7.0 runtime, just manually, so there's no net saving of disk space. The DevSkim language server bundled with the extension is only built for .NET 7.

ok, I thought this extension it would be Binary compatible and Source compatible since it is an extension in vscode.

Are those components essential to the functioning of those extensions or do they enable additional features? The DevSkim extension cannot perform any actions without the language server.

without those components, those two extensions only have syntax highlight work, and it's useless, so I should consider "yes, those components are essential to those extensions", but someone can also argue it with that and say no.

It won't mess with your existing .NET. The extension doesn't install .NET on your path. On windows, it installs it to C:\Users\USERNAME\AppData\Roaming\Code\User\globalStorage\ms-dotnettools.vscode-dotnet-runtime.dotnet.

Then it probably should be fine. I thought it would be runtime + SDK. The two extensions in the example are installed to their known path(one is to python, one is in %goroot%). The installing progress from ".NET Install Tool for Extension Authors" may be out of the common way of interacting with the users of VScode compared to the many other extensions from microsoft and other trusted publishers.

gfs commented 1 year ago

Thanks for the extra details. Based on that error message that look like an issue with the .NET Install tool itself.

ok, I thought this extension it would be Binary compatible and Source compatible since it is an extension in vscode.

The DevSkim VS Code Extension uses the Language Server Protocol (LSP). The Client is written in Typescript to integrate with VS Code but the Server is written in .NET to leverage the canonical C# DevSkim Engine. Previous versions of the VS Code Extension used a parallel implementation of DevSkim written in TypeScript as the language server, but over time this became too much maintenance overhead and resulted in a functionality delta between the Extension and the DevSkim CLI/Engine, so we opted to write a new Language Server that leveraged the canonical .NET implementation of DevSkim instead - (https://github.com/microsoft/DevSkim/tree/main/DevSkim-DotNet/Microsoft.DevSkim.LanguageServer). This also means we can reuse the same language server for both the VS Code extension and the Visual Studio 2022 extension.

This is a similar design to the Azure Bicep extension, which also uses the .NET Install tool to interface with a .NET Language Server using LSP. (https://github.com/Azure/bicep/blob/6fd52773948a97f11a776b9fbbf16fc03439af90/src/vscode-bicep/src/language/client.ts#L174-L180). It does appear that Bicep supports the user providing the path to their own install of .NET (though they also have a comment in the code mentioning it is a common source of user reported issues).

We can investigate adding that, but I can't provide a timeline at this time when that work would be complete. I don't think it helps much with your storage space concern (since you would still need the .NET 7.0 runtime available somewhere), but it at least provides a path if the .NET Install helper isn't working as it is in your case.

without those components, those two extensions only have syntax highlight work, and it's useless, so I should consider "yes, those components are essential to those extensions", but someone can also argue it with that and say no.

I see. I think the model we use is very similar to that employed by Bicep, which I've just tested - which also automatically downloads the .NET runtime as needed unless the user has beforehand configured a custom .net path.

Then it probably should be fine. I thought it would be runtime + SDK.

Okay, glad to hear that resolves one of your concerns.

The two extensions in the example are installed to their known path(one is to python, one is in %goroot%). The installing progress from ".NET Install Tool for Extension Authors" may be out of the common way of interacting with the users of VScode compared to the many other extensions from microsoft and other trusted publishers.

Yes, I understand the concern there. We do not intend to cause any secondary effects on any other aspect of your environment, and I believe the .NET Install Tool is designed to not have any secondary effects like that. As mentioned above the automatic behavior is consistent with other extensions that leverage the .NET install tool for authors like Azure Bicep, as the extension has essentially no functionality without the appropriate .NET runtime to execute the language server.

I've opened a follow-up issue to investigate adding the capability, as the Bicep extension has, for the user to "bring their own" .NET Runtime. https://github.com/microsoft/DevSkim/issues/566

NewUserHa commented 1 year ago

Ok. And thanks for the extra investigated info.