Fixes for Linux release build #72658
Conversation
Linux release build currently fails due to a couple issues: - Path issues - The relative path metadata in the language server eventually makes it way into R2R tasks. The relative path has a backslash on Linux as currently written, so Path.Combine in the R2R tasks retains that backslash, and then we fail to create the subdir where the R2R output goes. - Hoist .vsextension exclusion to Imports.targets - These files end up in Content even when not using the VS SDK tooling, so exclude even when not using the VS SDK targets. - Exclude VS.ExternalApis.Roslyn.Package on Linux - It packs vs debugger files, which are not available. Additional fixes
dotnet-issue-labeler
bot
added
Area-Infrastructure
untriaged
dotnet-policy-service
bot
added
the
Community
|
@jaredpar No VMR specific changes here, but specific to the config/commands that the VMR build runs on Linux (roslyn in release w/ -pack) |
| <!-- | ||
| Microsoft.VisualStudio.Extensibility.JsonGenerators.Sdk.props creates content items for all files in .vsextension dir. | ||
| NuGet includes content items in package, by default but these items should only be included in the VSIX. | ||
| --> |
There was a problem hiding this comment.
Why did we need to make this change for non-VS targets? My instinct would be that instead of this change we should be including VisualStudio.targets in more places.
There was a problem hiding this comment.
The .vsextension files end up in the pack content because of this package ref: https://github.com/dotnet/roslyn/blob/main/src/VisualStudio/CSharp/Impl/Microsoft.VisualStudio.LanguageServices.CSharp.csproj#L46
<ItemGroup>
<Reference Include="System.Design" />
<Reference Include="System.Windows.Forms" />
<PackageReference Include="Microsoft.VisualStudio.Extensibility" />
<PackageReference Include="Microsoft.VisualStudio.Extensibility.JsonGenerators.Sdk" PrivateAssets="all" /> <-- THIS
</ItemGroup>
That ends up importing a props file which creates the content items. I could make this conditional on vssdk usage, but it seems like a better approach to keep this here, and just not conditionalize the .vsextension Pack exclusion on IsUsingToolVsSdk, which was what was implicitly happening due to https://github.com/dotnet/roslyn/blob/main/eng/targets/Imports.targets#L4
<Import Project="VisualStudio.targets" Condition="'$(UsingToolVSSDK)' == 'true'"/>
There was a problem hiding this comment.
Feel like that should be importing the VisualStudio.targets file. @dibarbet will know better though.
There was a problem hiding this comment.
I'm not super familiar with this stuff here tbh - I'll delegate to @tmat who added the jsongenerators reference for gladstone.
There was a problem hiding this comment.
I've been a bit out of the loop as well - why are we building VS layer projects on linux? Seems like the packages produced for Microsoft.VisualStudio.LanguageServices.CSharp.csproj and others wouldn't be useful without the vssdk?
There was a problem hiding this comment.
I think we generally build everything and it seems to work fine. It's the packing that generally doesn't work, at least in release modes.
src/NuGet/VS.ExternalAPIs.Roslyn.Package/VS.ExternalAPIs.Roslyn.Package.csproj
Outdated
Show resolved
Hide resolved
| TargetPath="%(NetFrameworkBuildHostAssets.ContentFolderName)\%(NetFrameworkBuildHostAssets.TargetPath)" | ||
| PackagePath="contentFiles\any\any\%(NetFrameworkBuildHostAssets.ContentFolderName)\%(NetFrameworkBuildHostAssets.TargetPath)" | ||
| TargetPath="$([System.IO.Path]::Combine('%(NetFrameworkBuildHostAssets.ContentFolderName)', '%(NetFrameworkBuildHostAssets.TargetPath)'))" | ||
| PackagePath="$([System.IO.Path]::Combine('contentFiles', 'any', 'any', '%(NetFrameworkBuildHostAssets.ContentFolderName)', '%(NetFrameworkBuildHostAssets.TargetPath)'))" |
There was a problem hiding this comment.
@baronfel type of stuff an msbuild analyzer / warning would help with.
There was a problem hiding this comment.
Interesting - I thought we were more or less slash agnostic in most of MSBuild. Unless you're wanting a stricter rule of 'no direct path creation allowed'?
There was a problem hiding this comment.
This isn't a globbing pattern, it's a relative path. It ends up as metadata and eventually fed into the R2R tasks, which path.combines it with another parameter. On Linux, at that point the task the Path.Combine sees the backslash and does what it should on Linux (not treat it as a directory separator char).
If it was a globbing pattern, then MSBuild would fix it up.
|
@jaredpar Overall the line between "using VS SDK" and not in this repo is pretty messy. It seems like some VS extensibility features are in use even if the VS SDK isn't available. That causes some messiness w.r.t. to building on Linux vs. Windows. |
|
Let's wait for @dibarbet sign off but I'm good when he signs off. |
|
ping @dibarbet |
Linux release build currently fails due to a couple issues: