-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Add analyzer redirecting VSIX #42861
base: main
Are you sure you want to change the base?
Changes from 21 commits
62154cf
cdb4892
2ed93ed
53a5074
5928f32
5165f0f
9807811
4e01a87
cdac6cc
7a31ffb
586ced5
c4a206a
e65c1c0
61af4e8
245b2f1
f5d671e
92e2284
f0b86a4
d5a3924
5da7cda
6a8f110
8680087
f73918a
74518c1
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 |
---|---|---|
|
@@ -67,7 +67,6 @@ | |
<FileSignInfo Include="MessagePack.Annotations.dll" CertificateName="$(ExternalCertificateId)" /> | ||
<FileSignInfo Include="MessagePack.dll" CertificateName="$(ExternalCertificateId)" /> | ||
<FileSignInfo Include="Nerdbank.Streams.dll" CertificateName="$(ExternalCertificateId)" /> | ||
<FileSignInfo Include="StreamJsonRpc.dll" CertificateName="$(ExternalCertificateId)" /> | ||
<FileSignInfo Include="Newtonsoft.Json.dll" CertificateName="$(ExternalCertificateId)" /> | ||
<FileSignInfo Include="CommandLine.dll" CertificateName="$(ExternalCertificateId)" /> | ||
<FileSignInfo Include="FluentAssertions.dll" CertificateName="$(ExternalCertificateId)" /> | ||
|
@@ -81,6 +80,10 @@ | |
<FileSignInfo Include="Valleysoft.DockerCredsProvider.dll" CertificateName="$(ExternalCertificateId)" /> | ||
</ItemGroup> | ||
|
||
<ItemGroup> | ||
<FileSignInfo Include="StreamJsonRpc.dll" CertificateName="MicrosoftSHA2" /> | ||
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. What is this change about? 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. I was getting this signing error in official builds when building the VSIX:
FWIW, the same is specified in roslyn: https://github.com/dotnet/roslyn/blob/c737a04f3e9a895b5b5fff6d4c5467c17e9c49e8/eng/Signing.props#L62 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. Is this a new file you added? Why did your change cause it? @marcpopMSFT 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. I think StreamJsonRpc is some dependency brought in because we are building AnalyzerRedirecting as VSIX. |
||
</ItemGroup> | ||
|
||
<!-- Filter out any test packages from ItemsToSign --> | ||
<ItemGroup> | ||
<ItemsToSignPostBuild Remove="*tests*.nupkg" /> | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -97,43 +97,43 @@ | |
<Sha>63a09289745da5c256e7baf5f4194a750b1ec878</Sha> | ||
<SourceBuild RepoName="fsharp" ManagedOnly="true" /> | ||
</Dependency> | ||
<Dependency Name="Microsoft.Net.Compilers.Toolset" Version="4.13.0-3.25057.3"> | ||
<Dependency Name="Microsoft.Net.Compilers.Toolset" Version="4.12.0-3.24459.1"> | ||
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. Shouldn't these version changes come through Arcade code flow? 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. It shouldn't hurt to include dependency updates in your PR if your code depends on an external repo's changes that have not yet flowed to your current repo. It's ok as long as this file was modified using:
Ideally, a separate deps flow show come into the sdk repo with these same changes, and this PR can be rebased on top of it. 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 as #42861 (comment), this will go away before merging - we will need to merge the roslyn part first, then let it flow into SDK, then we will be able to remove these version/nuget changes. |
||
<Uri>https://github.com/dotnet/roslyn</Uri> | ||
<Sha>911cf5f462960bdd01df1ea3c0d0c217b3c3838b</Sha> | ||
<Sha>74f9a768bd10a7b9b0732bec47714ecd0f27e248</Sha> | ||
</Dependency> | ||
<!-- Intermediate is necessary for source build. --> | ||
<Dependency Name="Microsoft.SourceBuild.Intermediate.roslyn" Version="4.13.0-3.25057.3"> | ||
<Dependency Name="Microsoft.SourceBuild.Intermediate.roslyn" Version="4.12.0-3.24459.1"> | ||
<Uri>https://github.com/dotnet/roslyn</Uri> | ||
<Sha>911cf5f462960bdd01df1ea3c0d0c217b3c3838b</Sha> | ||
<Sha>74f9a768bd10a7b9b0732bec47714ecd0f27e248</Sha> | ||
<SourceBuild RepoName="roslyn" ManagedOnly="true" /> | ||
</Dependency> | ||
<Dependency Name="Microsoft.Net.Compilers.Toolset.Framework" Version="4.13.0-3.25057.3"> | ||
<Dependency Name="Microsoft.Net.Compilers.Toolset.Framework" Version="4.12.0-3.24459.1"> | ||
<Uri>https://github.com/dotnet/roslyn</Uri> | ||
<Sha>911cf5f462960bdd01df1ea3c0d0c217b3c3838b</Sha> | ||
<Sha>74f9a768bd10a7b9b0732bec47714ecd0f27e248</Sha> | ||
</Dependency> | ||
<Dependency Name="Microsoft.CodeAnalysis" Version="4.13.0-3.25057.3"> | ||
<Dependency Name="Microsoft.CodeAnalysis" Version="4.12.0-3.24459.1"> | ||
<Uri>https://github.com/dotnet/roslyn</Uri> | ||
<Sha>911cf5f462960bdd01df1ea3c0d0c217b3c3838b</Sha> | ||
<Sha>74f9a768bd10a7b9b0732bec47714ecd0f27e248</Sha> | ||
</Dependency> | ||
<Dependency Name="Microsoft.CodeAnalysis.CSharp" Version="4.13.0-3.25057.3"> | ||
<Dependency Name="Microsoft.CodeAnalysis.CSharp" Version="4.12.0-3.24459.1"> | ||
<Uri>https://github.com/dotnet/roslyn</Uri> | ||
<Sha>911cf5f462960bdd01df1ea3c0d0c217b3c3838b</Sha> | ||
<Sha>74f9a768bd10a7b9b0732bec47714ecd0f27e248</Sha> | ||
</Dependency> | ||
<Dependency Name="Microsoft.CodeAnalysis.CSharp.CodeStyle" Version="4.13.0-3.25057.3"> | ||
<Dependency Name="Microsoft.CodeAnalysis.CSharp.CodeStyle" Version="4.12.0-3.24459.1"> | ||
<Uri>https://github.com/dotnet/roslyn</Uri> | ||
<Sha>911cf5f462960bdd01df1ea3c0d0c217b3c3838b</Sha> | ||
<Sha>74f9a768bd10a7b9b0732bec47714ecd0f27e248</Sha> | ||
</Dependency> | ||
<Dependency Name="Microsoft.CodeAnalysis.CSharp.Features" Version="4.13.0-3.25057.3"> | ||
<Dependency Name="Microsoft.CodeAnalysis.CSharp.Features" Version="4.12.0-3.24459.1"> | ||
<Uri>https://github.com/dotnet/roslyn</Uri> | ||
<Sha>911cf5f462960bdd01df1ea3c0d0c217b3c3838b</Sha> | ||
<Sha>74f9a768bd10a7b9b0732bec47714ecd0f27e248</Sha> | ||
</Dependency> | ||
<Dependency Name="Microsoft.CodeAnalysis.CSharp.Workspaces" Version="4.13.0-3.25057.3"> | ||
<Dependency Name="Microsoft.CodeAnalysis.CSharp.Workspaces" Version="4.12.0-3.24459.1"> | ||
<Uri>https://github.com/dotnet/roslyn</Uri> | ||
<Sha>911cf5f462960bdd01df1ea3c0d0c217b3c3838b</Sha> | ||
<Sha>74f9a768bd10a7b9b0732bec47714ecd0f27e248</Sha> | ||
</Dependency> | ||
<Dependency Name="Microsoft.CodeAnalysis.Workspaces.MSBuild" Version="4.13.0-3.25057.3"> | ||
<Dependency Name="Microsoft.CodeAnalysis.Workspaces.MSBuild" Version="4.12.0-3.24459.1"> | ||
<Uri>https://github.com/dotnet/roslyn</Uri> | ||
<Sha>911cf5f462960bdd01df1ea3c0d0c217b3c3838b</Sha> | ||
<Sha>74f9a768bd10a7b9b0732bec47714ecd0f27e248</Sha> | ||
</Dependency> | ||
<Dependency Name="Microsoft.AspNetCore.DeveloperCertificates.XPlat" Version="10.0.0-alpha.2.25061.1"> | ||
<Uri>https://github.com/dotnet/aspnetcore</Uri> | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,17 @@ | ||
<package xmlns="http://schemas.microsoft.com/packaging/2011/08/nuspec.xsd"> | ||
<metadata> | ||
<id>VS.Redist.Common.Net.Core.SDK.RuntimeAnalyzers</id> | ||
<version>1.0.0</version> | ||
<title>VS.Redist.Common.Net.Core.SDK.RuntimeAnalyzers</title> | ||
<authors>Microsoft</authors> | ||
<owners>Microsoft</owners> | ||
<licenseUrl>https://www.microsoft.com/net/dotnet_library_license.htm</licenseUrl> | ||
<projectUrl>https://github.com/dotnet/sdk</projectUrl> | ||
<requireLicenseAcceptance>true</requireLicenseAcceptance> | ||
<description>Analyzers and generators from the runtime and SDK for VS insertion</description> | ||
<copyright>© Microsoft Corporation. All rights reserved.</copyright> | ||
</metadata> | ||
<files> | ||
<file src="$PAYLOAD_FILES$\**\*" /> | ||
</files> | ||
</package> |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
<Project> | ||
|
||
<PropertyGroup> | ||
<RuntimeAnalyzersLayoutDirectory>$(ArtifactsBinDir)$(Configuration)\RuntimeAnalyzers</RuntimeAnalyzersLayoutDirectory> | ||
</PropertyGroup> | ||
|
||
</Project> |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,24 @@ | ||
<Project> | ||
|
||
<PropertyGroup> | ||
<RuntimeAnalyzersSourceRoot>$(ArtifactsBinDir)redist\$(Configuration)\dotnet\</RuntimeAnalyzersSourceRoot> | ||
<NetCoreRuntimeAnalyzersSubPath>packs\Microsoft.NetCore.App.Ref\*\analyzers</NetCoreRuntimeAnalyzersSubPath> | ||
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. Will the SDK ever carry more than one pack? I guess if it did, we'd still be OK to include multiple versions since you match based on the major and minor. How would this interact with VS on installation -- is it possible we'd ever have two of these analyzer packs installed, or would the newer one replace the old one? 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. That should work fine - the analyzers are deployed with the version number in their path like
so multiple of them could live side by side. 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. Ok, so if that happens would ever have more than one of these DotNetRuntimeAnalyzers installer packages inserted into VS that are trying to install the same files? I'm guessing no - if possible we should ensure that's the case. |
||
<WindowsDesktopRuntimeAnalyzersSubPath>packs\Microsoft.WindowsDesktop.App.Ref\*\analyzers</WindowsDesktopRuntimeAnalyzersSubPath> | ||
<AspNetCoreRuntimeAnalyzersSubPath>packs\Microsoft.AspNetCore.App.Ref\*\analyzers</AspNetCoreRuntimeAnalyzersSubPath> | ||
<SDKAnalyzersSubPath>sdk\*\Sdks\Microsoft.NET.Sdk\analyzers</SDKAnalyzersSubPath> | ||
<WebSDKAnalyzersSubPath>sdk\*\Sdks\Microsoft.NET.Sdk.Web\analyzers</WebSDKAnalyzersSubPath> | ||
</PropertyGroup> | ||
|
||
<Target Name="GenerateRuntimeAnalyzers" AfterTargets="OverlaySdkOnLKG"> | ||
|
||
<ItemGroup> | ||
<RuntimeAnalyzersContent Include="$(ArtifactsBinDir)Microsoft.Net.Sdk.AnalyzerRedirecting\$(Configuration)\net472\**\*.*" DeploymentSubpath="AnalyzerRedirecting"/> | ||
<RuntimeAnalyzersContent Include="$(RuntimeAnalyzersSourceRoot)$(NetCoreRuntimeAnalyzersSubPath)\**\*.*" DeploymentSubpath="NetCoreAnalyzers"/> | ||
<RuntimeAnalyzersContent Include="$(RuntimeAnalyzersSourceRoot)$(WindowsDesktopRuntimeAnalyzersSubPath)\**\*.*" DeploymentSubpath="WindowsDesktopAnalyzers"/> | ||
<RuntimeAnalyzersContent Include="$(RuntimeAnalyzersSourceRoot)$(AspNetCoreRuntimeAnalyzersSubPath)\**\*.*" DeploymentSubpath="AspNetCoreAnalyzers"/> | ||
<RuntimeAnalyzersContent Include="$(RuntimeAnalyzersSourceRoot)$(SDKAnalyzersSubPath)\**\*.*" DeploymentSubpath="SDKAnalyzers"/> | ||
<RuntimeAnalyzersContent Include="$(RuntimeAnalyzersSourceRoot)$(WebSDKAnalyzersSubPath)\**\*.*" DeploymentSubpath="WebSDKAnalyzers"/> | ||
</ItemGroup> | ||
</Target> | ||
|
||
</Project> |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
using Microsoft.VisualStudio.Shell; | ||
|
||
namespace Microsoft.Net.Sdk.AnalyzerRedirecting; | ||
|
||
[Guid("ef89a321-14da-4de4-8f71-9bf1feea15aa")] | ||
public sealed class AnalyzerRedirectingPackage : AsyncPackage; |
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.
What does this feed provide?
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.
This will go away before merging. This PR depends on a roslyn API which is not merged yet (it's in PR dotnet/roslyn#74820). So I manually published the roslyn packages to the "general testing" feed to use them here.