Respond to ProduceReferenceAssembly default changing to true #31598

drewnoakes · 2023-04-04T13:03:13Z

Fixes dotnet/msbuild#8133
Relates to dotnet/msbuild#8571

We have changed the default for all .NET projects such that ProduceReferenceAssembly is true by default. This change requires updates to the SDK, which are included in this PR.

dotnet-issue-labeler · 2023-04-04T13:03:19Z

I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label.

drewnoakes · 2023-04-05T02:07:50Z

src/Tests/Microsoft.NET.Build.Tests/GivenThatWeWantToProduceReferenceAssembly.cs

+        [InlineData("net7.0", ".fsproj")]
+        [InlineData(ToolsetInfo.CurrentTargetFramework, ".csproj")]
+        [InlineData(ToolsetInfo.CurrentTargetFramework, ".fsproj")]
+        public void It_produces_ref_assembly_for_all_frameworks(string targetFramework, string extension)


The fixes here address the failures that led to dotnet/msbuild#8131

src/Tests/Microsoft.NET.Build.Tests/GivenThatWeWantToBuildANetCoreApp.cs

…0412.2 Microsoft.SourceBuild.Intermediate.msbuild , Microsoft.Build , Microsoft.Build.Localization From Version 17.7.0-preview-23212-01 -> To Version 17.7.0-preview-23212-02

.NET projects now produce a reference assembly by default, per a change in MSBuild's common targets. This commit updates a test on the SDK side in response to that change.

drewnoakes · 2023-04-13T03:52:42Z

This is failing due to test GivenThatWeWantToBuildANetCoreApp.It_builds_the_project_successfully_with_only_reference_assembly_set on line 909:

sdk/src/Tests/Microsoft.NET.Build.Tests/GivenThatWeWantToBuildANetCoreApp.cs

Lines 871 to 913 in 2d01755

    
           [Theory] 
        
           [InlineData(true)] 
        
           [InlineData(false)] 
        
           public void It_builds_the_project_successfully_with_only_reference_assembly_set(bool produceOnlyReferenceAssembly) 
        
           { 
        
               var testProject = new TestProject() 
        
               { 
        
                   Name = "MainProject", 
        
                   TargetFrameworks = ToolsetInfo.CurrentTargetFramework, 
        
                   IsSdkProject = true, 
        
                   IsExe = true 
        
               }; 
        
               testProject.AdditionalProperties["ProduceOnlyReferenceAssembly"] = produceOnlyReferenceAssembly.ToString(); 
        
               var testProjectInstance = _testAssetsManager.CreateTestProject(testProject, identifier: produceOnlyReferenceAssembly.ToString()); 
        
               var buildCommand = new BuildCommand(testProjectInstance); 
        
               buildCommand 
        
                   .Execute() 
        
                   .Should() 
        
                   .Pass(); 
        
               var outputPath = buildCommand.GetOutputDirectory(targetFramework: ToolsetInfo.CurrentTargetFramework).FullName; 
        
               if (produceOnlyReferenceAssembly == true) 
        
               { 
        
                   var refPath = Path.Combine(outputPath, "ref"); 
        
                   Directory.Exists(refPath) 
        
                       .Should() 
        
                       .BeFalse(); 
        
               } 
        
               else 
        
               { 
        
                   // Reference assembly should be produced in obj 
        
                   var refPath = Path.Combine( 
        
                       buildCommand.GetIntermediateDirectory(targetFramework: ToolsetInfo.CurrentTargetFramework).FullName, 
        
                       "ref", 
        
                       "MainProject.dll"); 
        
                   File.Exists(refPath) 
        
                       .Should() 
        
                       .BeTrue(); 
        
               } 
        
           }

The test fails when produceOnlyReferenceAssembly is false because the path obj\Debug\net7.0\ref\MainProject.dll does not exist. When I run this test locally it is created.

I'm going to need some help understanding the test failure here as it passes on my machine. I'm pretty sure my environment is set up correctly as the test I did change in this PR also passes on my machine (GivenThatWeWantToProduceReferenceAssembly.It_produces_ref_assembly_for_appropriate_frameworks) and it would fail if the corresponding MSBuild change wasn't being picked up.

Is there some way to get a .binlog from the CI machine for a specific unit test?

drewnoakes · 2023-04-13T06:06:49Z

Latest build is failing with:

Process ID: 4796
    Microsoft.NET.Build.Tests.GivenThatWeWantToProduceReferenceAssembly.It_produces_ref_assembly_for_appropriate_frameworks(targetFramework: "net7.0", extension: ".fsproj") [FAIL]
      System.Exception : Test dir C:\h\w\B2730982\t\dotnetSdkTests\42553ekg.5uy\It_produces_r---08D0FE86_2 already exists
      Stack Trace:
        /_/src/Tests/Microsoft.NET.TestFramework/TestAssetsManager.cs(223,0): at Microsoft.NET.TestFramework.TestAssetsManager.GetTestDestinationDirectoryPath(String testProjectName, String callingMethodAndFileName, String identifier, Boolean allowCopyIfPresent)
        /_/src/Tests/Microsoft.NET.TestFramework/TestAssetsManager.cs(83,0): at Microsoft.NET.TestFramework.TestAssetsManager.CreateTestProject(TestProject testProject, String callingMethod, String identifier, String targetExtension)
        /_/src/Tests/Microsoft.NET.Build.Tests/GivenThatWeWantToProduceReferenceAssembly.cs(41,0): at Microsoft.NET.Build.Tests.GivenThatWeWantToProduceReferenceAssembly.It_produces_ref_assembly_for_appropriate_frameworks(String targetFramework, String extension)
           at InvokeStub_GivenThatWeWantToProduceReferenceAssembly.It_produces_ref_assembly_for_appropriate_frameworks(Object, Object, IntPtr*)
           at System.Reflection.MethodInvoker.Invoke(Object obj, IntPtr* args, BindingFlags invokeAttr)

This failure seems intermittent. Could xUnit be running these in parallel? I can't see why this wouldn't have failed before my changes though.

drewnoakes · 2023-04-13T11:34:27Z

/azp run

azure-pipelines · 2023-04-13T11:34:38Z

Azure Pipelines successfully started running 1 pipeline(s).

drewnoakes · 2023-04-13T11:51:31Z

Ah, I see, the truncated-with-hash file name needs to include an identifier for the leg of a multi-leg theory that is unique. Fixed in 039025f.

rainersigwald · 2023-04-13T15:13:55Z

src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.TargetFrameworkInference.targets

@@ -239,11 +239,6 @@ Copyright (c) .NET Foundation. All rights reserved.
    <AppendTargetFrameworkToOutputPath Condition="'$(AppendTargetFrameworkToOutputPath)' == ''">true</AppendTargetFrameworkToOutputPath>
  </PropertyGroup>

-  <PropertyGroup>
-    <ProduceReferenceAssembly Condition="'$(ProduceReferenceAssembly)' == '' and '$(TargetFrameworkIdentifier)' == '.NETCoreApp' and $([MSBuild]::VersionGreaterThanOrEquals($(TargetFrameworkVersion), 5.0)) and ('$(ProduceOnlyReferenceAssembly)' != 'true') and '$(MSBuildProjectExtension)' != '.fsproj'" >true</ProduceReferenceAssembly>
-    <ProduceReferenceAssembly Condition="'$(ProduceReferenceAssembly)' == '' and '$(TargetFrameworkIdentifier)' == '.NETCoreApp' and $([MSBuild]::VersionGreaterThanOrEquals($(TargetFrameworkVersion), 7.0)) and ('$(ProduceOnlyReferenceAssembly)' != 'true') and '$(MSBuildProjectExtension)' == '.fsproj'" >true</ProduceReferenceAssembly>


Who from F# do we need to vet this?

@vzarytovskii is the go-to.

I don't think any action is needed from us? Now it will set it to true for all projects when using .net8?

Note that, combined with the MS Build change, ref assemblies will be produced for all F# projects when using VS 17.7, irrespective of the target framework.

Yeah, I think it should be fine, and leaves us window to make changes, if needed, before net8 release.

lewing · 2023-04-14T15:57:51Z

@dotnet/domestic-cat can we land this?

…31598)" This reverts commit d2e470b, reversing changes made to 377caa7. Follows the MSBuild revert in dotnet/msbuild#8571.

dotnet-issue-labeler bot added the untriaged Request triage from a team member label Apr 4, 2023

drewnoakes mentioned this pull request Apr 4, 2023

.NET projects produce a reference assembly by default dotnet/msbuild#8571

Merged

drewnoakes commented Apr 5, 2023

View reviewed changes

src/Tests/Microsoft.NET.Build.Tests/GivenThatWeWantToBuildANetCoreApp.cs Outdated Show resolved Hide resolved

drewnoakes added the Area-Compilers label Apr 5, 2023

drewnoakes force-pushed the ref-assemblies-by-default branch from 32fbc35 to 4549367 Compare April 5, 2023 13:16

dotnet-maestro bot and others added 2 commits April 12, 2023 23:22

Update dependencies from https://github.com/dotnet/msbuild build 2023…

458cd31

…0412.2 Microsoft.SourceBuild.Intermediate.msbuild , Microsoft.Build , Microsoft.Build.Localization From Version 17.7.0-preview-23212-01 -> To Version 17.7.0-preview-23212-02

Respond to ProduceReferenceAssembly default changing to true

9ded50a

.NET projects now produce a reference assembly by default, per a change in MSBuild's common targets. This commit updates a test on the SDK side in response to that change.

drewnoakes force-pushed the ref-assemblies-by-default branch from 4549367 to 9ded50a Compare April 13, 2023 00:16

drewnoakes changed the base branch from main to release/7.0.4xx April 13, 2023 00:17

drewnoakes mentioned this pull request Apr 13, 2023

[release/7.0.4xx] Update dependencies from dotnet/msbuild #31768

Merged

drewnoakes added 2 commits April 13, 2023 13:53

Set required MSBuild version for test

90c089a

Use built in fluent validation method

c234603

drewnoakes force-pushed the ref-assemblies-by-default branch from 876e5e9 to c234603 Compare April 13, 2023 05:06

Make identifier unique across theory legs

039025f

rainersigwald approved these changes Apr 13, 2023

View reviewed changes

drewnoakes marked this pull request as ready for review April 13, 2023 22:46

JanKrivanek mentioned this pull request Apr 14, 2023

[main] Update dependencies from dotnet/msbuild #31784

Merged

marcpopMSFT merged commit d2e470b into dotnet:release/7.0.4xx Apr 14, 2023

drewnoakes deleted the ref-assemblies-by-default branch April 14, 2023 22:05

rainersigwald mentioned this pull request Apr 21, 2023

[release/7.0.4xx] Update dependencies from dotnet/msbuild #31927

Merged

rainersigwald added a commit that referenced this pull request Apr 21, 2023

Revert "Respond to ProduceReferenceAssembly default changing to true (#…

a84060a

…31598)" This reverts commit d2e470b, reversing changes made to 377caa7. Follows the MSBuild revert in dotnet/msbuild#8571.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Respond to ProduceReferenceAssembly default changing to true #31598

Respond to ProduceReferenceAssembly default changing to true #31598

drewnoakes commented Apr 4, 2023 •

edited

Loading

dotnet-issue-labeler bot commented Apr 4, 2023

drewnoakes Apr 5, 2023

drewnoakes commented Apr 13, 2023

drewnoakes commented Apr 13, 2023 •

edited

Loading

drewnoakes commented Apr 13, 2023

azure-pipelines bot commented Apr 13, 2023

drewnoakes commented Apr 13, 2023

rainersigwald Apr 13, 2023

baronfel Apr 13, 2023

vzarytovskii Apr 13, 2023

drewnoakes Apr 13, 2023

vzarytovskii Apr 13, 2023

lewing commented Apr 14, 2023

Respond to ProduceReferenceAssembly default changing to true #31598

Respond to ProduceReferenceAssembly default changing to true #31598

Conversation

drewnoakes commented Apr 4, 2023 • edited Loading

dotnet-issue-labeler bot commented Apr 4, 2023

drewnoakes Apr 5, 2023

Choose a reason for hiding this comment

drewnoakes commented Apr 13, 2023

drewnoakes commented Apr 13, 2023 • edited Loading

drewnoakes commented Apr 13, 2023

azure-pipelines bot commented Apr 13, 2023

drewnoakes commented Apr 13, 2023

rainersigwald Apr 13, 2023

Choose a reason for hiding this comment

baronfel Apr 13, 2023

Choose a reason for hiding this comment

vzarytovskii Apr 13, 2023

Choose a reason for hiding this comment

drewnoakes Apr 13, 2023

Choose a reason for hiding this comment

vzarytovskii Apr 13, 2023

Choose a reason for hiding this comment

lewing commented Apr 14, 2023

drewnoakes commented Apr 4, 2023 •

edited

Loading

drewnoakes commented Apr 13, 2023 •

edited

Loading