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

Jump to bottom

Use PackageDownload in torn builds #41951

Merged
merged 9 commits into from
Jul 12, 2024
Merged

Use PackageDownload in torn builds #41951

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
7570bfc
Use PackageDownload in torn builds
jjonescz Jul 2, 2024
080bd3d
Fix texts
jjonescz Jul 9, 2024
9e8feb3
Lowercase NuGet package name
jjonescz Jul 9, 2024
13beb4d
Re-add the warning
jjonescz Jul 9, 2024
23cf26a
Move PackageDownload outside the target
jjonescz Jul 9, 2024
f8fce72
Note UsingTasks duplication in MSBuild
jjonescz Jul 10, 2024
3824f41
Extend build tests
jjonescz Jul 10, 2024
70bd6ed
Fix warning condition
jjonescz Jul 10, 2024
b1840b4
Improve wording
jjonescz Jul 11, 2024
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
13 changes: 0 additions & 13 deletions eng/Publishing.props
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,19 +88,6 @@
</ItemGroup>
</Target>

<!-- Similarly to FSharp above, Roslyn needs to publish its Framework.Toolset package alongside SDK
so it can be picked up in BuildWithNetFrameworkHostedCompiler scenarios. -->
<PropertyGroup>
<PublishDependsOnTargets>$(PublishDependsOnTargets);_ResolvePublishRoslynNuGetPackages</PublishDependsOnTargets>
</PropertyGroup>

<Target Name="_ResolvePublishRoslynNuGetPackages" Condition="'$(EnableDefaultArtifacts)' == 'true'">
<ItemGroup>
<RoslynPackagesToPush Include="$(NuGetPackageRoot)\Microsoft.Net.Compilers.Toolset.Framework\$(MicrosoftNetCompilersToolsetFrameworkPackageVersion)\*.nupkg" />
<ItemsToPushToBlobFeed Include="@(RoslynPackagesToPush)" IsShipping="true" />
</ItemGroup>
</Target>

<!-- We use a separate target to publish this to blob storage so that we can push this to a relative path inside the blob storage. -->
<Target Name="PublishToolsetAssets" DependsOnTargets="ReadToolsetVersion" BeforeTargets="Publish" Condition="'$(EnableDefaultArtifacts)' == 'true'">
<ItemGroup>
Expand Down
7 changes: 7 additions & 0 deletions sdk.sln
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -509,6 +509,8 @@ Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Microsoft.WebTools.AspireSe
EndProject
Project("{D954291E-2A0B-460D-934E-DC6B0785DB48}") = "Microsoft.WebTools.AspireService", "src\BuiltInTools\AspireService\Microsoft.WebTools.AspireService.shproj", "{94C8526E-DCC2-442F-9868-3DD0BA2688BE}"
EndProject
Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Microsoft.Net.Sdk.Compilers.Toolset", "src\Microsoft.Net.Sdk.Compilers.Toolset\Microsoft.Net.Sdk.Compilers.Toolset.csproj", "{FA579C03-2EB4-4D47-88EE-BFF339E96FAF}"
EndProject
Global
GlobalSection(SolutionConfigurationPlatforms) = preSolution
Debug|Any CPU = Debug|Any CPU
Expand Down Expand Up @@ -971,6 +973,10 @@ Global
{19014C60-F87C-4CC7-AC0F-C41B6126EBCE}.Debug|Any CPU.Build.0 = Debug|Any CPU
{19014C60-F87C-4CC7-AC0F-C41B6126EBCE}.Release|Any CPU.ActiveCfg = Release|Any CPU
{19014C60-F87C-4CC7-AC0F-C41B6126EBCE}.Release|Any CPU.Build.0 = Release|Any CPU
{FA579C03-2EB4-4D47-88EE-BFF339E96FAF}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
{FA579C03-2EB4-4D47-88EE-BFF339E96FAF}.Debug|Any CPU.Build.0 = Debug|Any CPU
{FA579C03-2EB4-4D47-88EE-BFF339E96FAF}.Release|Any CPU.ActiveCfg = Release|Any CPU
{FA579C03-2EB4-4D47-88EE-BFF339E96FAF}.Release|Any CPU.Build.0 = Release|Any CPU
EndGlobalSection
GlobalSection(SolutionProperties) = preSolution
HideSolutionNode = FALSE
Expand Down Expand Up @@ -1150,6 +1156,7 @@ Global
{149E3D40-8115-4965-9305-5A1ADBF04899} = {580D1AE7-AA8F-4912-8B76-105594E00B3B}
{19014C60-F87C-4CC7-AC0F-C41B6126EBCE} = {71A9F549-0EB6-41F9-BC16-4A6C5007FC91}
{94C8526E-DCC2-442F-9868-3DD0BA2688BE} = {71A9F549-0EB6-41F9-BC16-4A6C5007FC91}
{FA579C03-2EB4-4D47-88EE-BFF339E96FAF} = {22AB674F-ED91-4FBC-BFEE-8A1E82F9F05E}
EndGlobalSection
GlobalSection(ExtensibilityGlobals) = postSolution
SolutionGuid = {FB8F26CE-4DE6-433F-B32A-79183020BBD6}
Expand Down
16 changes: 16 additions & 0 deletions src/Microsoft.Net.Sdk.Compilers.Toolset/Microsoft.Net.Sdk.Compilers.Toolset.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
<Project Sdk="Microsoft.NET.Sdk">

<PropertyGroup>
<TargetFramework>$(SdkTargetFramework)</TargetFramework>
<Description>Transport package for Microsoft.Net.Compilers.Toolset.Framework assemblies. For internal use only.</Description>
<IsPackable>true</IsPackable>
<IncludeBuildOutput>false</IncludeBuildOutput>
<NoPackageAnalysis>true</NoPackageAnalysis>
</PropertyGroup>

<ItemGroup>
<PackageReference Include="Microsoft.Net.Compilers.Toolset.Framework" ExcludeAssets="All" GeneratePathProperty="true" Condition="'$(DotNetBuildSourceOnly)' != 'true'" />
<Content Include="$(PkgMicrosoft_Net_Compilers_Toolset_Framework)\tasks\net472\**\*" PackagePath="%(RecursiveDir)" />
Copy link
Member

Choose a reason for hiding this comment

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

Two follow up items:

  1. Can you also make a change to roslyn which adds some documentation to this package outlining how it is now consumed in the .NET SDK? Essentially a marker that will alert anyone that is trying to change the code in roslyn to what it means in the .NET SDK?
  2. Can you file an issue in roslyn that tracks changing the bootstrap framework CI leg to more closely follow what the .NET SDK is now doing? Essentially set /p:RoslynTargetsPath=...\net472. That will help ensure our test coverage catches any authoring mistakes we make in roslyn.

Copy link
Member Author

Choose a reason for hiding this comment

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

  1. Note that toolset package layout is hard-coded in SDK roslyn#74301
  2. Change bootstrap framework CI leg to be consistent with SDK roslyn#74303

</ItemGroup>

</Project>
4 changes: 0 additions & 4 deletions src/Tasks/Common/Resources/Strings.resx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -905,10 +905,6 @@ You may need to build the project on another operating system or architecture, o
<value>NETSDK1204: Ahead-of-time compilation is not supported on the current platform '{0}'.</value>
<comment>{StrBegin="NETSDK1204: "}</comment>
</data>
<data name="CannotDirectlyReferenceMicrosoftNetCompilersToolsetFramework" xml:space="preserve">
<value>NETSDK1205: The Microsoft.Net.Compilers.Toolset.Framework package should not be set directly. Set the property 'BuildWithNetFrameworkHostedCompiler' to 'true' instead if you need it.</value>
<comment>{StrBegin="NETSDK1205: "}{Locked="Microsoft.Net.Compilers.Toolset.Framework"}{Locked="BuildWithNetFrameworkHostedCompiler"}</comment>
</data>
Copy link
Member Author

Choose a reason for hiding this comment

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

@marcpopMSFT had a comment in a Teams thread about this - instead of removing this warning, should that be changed to warn customers if they reference the new Microsoft.Net.Sdk.Compilers.Toolset package?

We could do that, but nothing happens when customers reference the new package as it has a non-standard layout. So, a warning seems unnecessary to me.

A related question is what happens when a customer references the Microsoft.Net.Compilers.Toolset.Framework package (which we used to warn about). From local testing, it looks like the compiler from the explicit PackageReference takes precedence over the built-in disjoint detection and the PackageDownload.

Copy link
Member

Choose a reason for hiding this comment

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

We can always remove a warning but adding one is a compat break, so I think I lean toward changing it rather than removing it.

Copy link
Member

Choose a reason for hiding this comment

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

If referencing the old package takes precedents over the packagedownload, do we want to keep the original warning in place? I'm less worried now about the sdk package if referencing it doesn't do anything. CC @baronfel as this is one of those situations where a customer could do bad things to themselves but how much do we want to prevent/warn.

<data name="NonPortableRuntimeIdentifierDetected" xml:space="preserve">
<value>NETSDK1206: Found version-specific or distribution-specific runtime identifier(s): {0}. Affected libraries: {1}. In .NET 8.0 and higher, assets for version-specific and distribution-specific runtime identifiers will not be found by default. See https://aka.ms/dotnet/rid-usage for details.</value>
<comment>{StrBegin="NETSDK1206: "}</comment>
Expand Down
5 changes: 0 additions & 5 deletions src/Tasks/Common/Resources/xlf/Strings.cs.xlf
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 0 additions & 5 deletions src/Tasks/Common/Resources/xlf/Strings.de.xlf
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 0 additions & 5 deletions src/Tasks/Common/Resources/xlf/Strings.es.xlf
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 0 additions & 5 deletions src/Tasks/Common/Resources/xlf/Strings.fr.xlf
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 0 additions & 5 deletions src/Tasks/Common/Resources/xlf/Strings.it.xlf
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 0 additions & 5 deletions src/Tasks/Common/Resources/xlf/Strings.ja.xlf
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 0 additions & 5 deletions src/Tasks/Common/Resources/xlf/Strings.ko.xlf
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 0 additions & 5 deletions src/Tasks/Common/Resources/xlf/Strings.pl.xlf
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 0 additions & 5 deletions src/Tasks/Common/Resources/xlf/Strings.pt-BR.xlf
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 0 additions & 5 deletions src/Tasks/Common/Resources/xlf/Strings.ru.xlf
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading
Loading