Skip to content

Commit

Permalink
Respond to PR feedback on RID Inference for Publish Only
Browse files Browse the repository at this point in the history
  • Loading branch information
@nagilson
nagilson committed Dec 13, 2022
1 parent e3c84ca commit c3a68ec
Show file tree
Hide file tree
Showing 4 changed files with 31 additions and 25 deletions.
16 changes: 9 additions & 7 deletions src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.RuntimeIdentifierInference.targets
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,12 +80,14 @@ Copyright (c) .NET Foundation. All rights reserved.
'$(RuntimeIdentifier)' == '' and
'$(_IsExecutable)' == 'true' and '$(IsTestProject)' != 'true' and
'$(IsRidAgnostic)' != 'true' and
('$(_IsPublishing)' == 'true' or '$(SelfContained)' == 'true') and
(
'$(SelfContained)' == 'true' or
'$(PublishReadyToRun)' == 'true' or
'$(PublishSingleFile)' == 'true' or
'$(PublishAot)' == 'true'
(
'$(SelfContained)' == 'true' or
('$(_IsPublishing)' == 'true' and
(
'$(PublishReadyToRun)' == 'true' or
'$(PublishSingleFile)' == 'true' or
'$(PublishAot)' == 'true'
)
)">true</UseCurrentRuntimeIdentifier>
</PropertyGroup>

Expand Down Expand Up @@ -184,7 +186,7 @@ Copyright (c) .NET Foundation. All rights reserved.
Condition="'$(TargetFrameworkIdentifier)' == '.NETCoreApp' and '$(HasRuntimeOutput)' == 'true'">

<!-- The following RID errors are asserts, and we don't expect them to ever occur. The error message is added as a safeguard.-->
<NETSdkError Condition="'$(SelfContained)' == 'true' and '$(RuntimeIdentifier)' == '''"
<NETSdkError Condition="'$(SelfContained)' == 'true' and '$(RuntimeIdentifier)' == ''"
ResourceName="ImplicitRuntimeIdentifierResolutionForPublishPropertyFailed"
FormatArguments="SelfContained"/>

Expand Down
13 changes: 7 additions & 6 deletions src/Tests/Microsoft.NET.Build.Tests/GivenThatWeWantToBuildASelfContainedApp.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -405,10 +405,10 @@ public void It_does_not_build_SelfContained_due_to_PublishSelfContained_being_tr
}

[Theory]
[InlineData("--p:PublishReadyToRun=true")]
[InlineData("-p:PublishSingleFile=true")]
[InlineData("-p:PublishSelfContained=true")]
[InlineData("-p:PublishAot=true")]
[InlineData("PublishReadyToRun")]
[InlineData("PublishSingleFile")]
[InlineData("PublishSelfContained")]
[InlineData("PublishAot")]
public void It_builds_without_implicit_rid_with_RuntimeIdentifier_specific_during_publish_only_properties(string property)
{
var tfm = ToolsetInfo.CurrentTargetFramework;
Expand All @@ -417,12 +417,13 @@ public void It_builds_without_implicit_rid_with_RuntimeIdentifier_specific_durin
IsExe = true,
TargetFrameworks = tfm,
};
testProject.AdditionalProperties[property] = "true";
testProject.RecordProperties("RuntimeIdentifier");
var testAsset = _testAssetsManager.CreateTestProject(testProject, identifier: $"ItBuildsWithoutImplicitRIDOnRIDRequiringPropertiesDuringPublish_{property}");
var testAsset = _testAssetsManager.CreateTestProject(testProject, identifier: property);

var buildCommand = new DotnetBuildCommand(testAsset);
buildCommand
.Execute(property)
.Execute()
.Should()
.Pass();

Expand Down
26 changes: 14 additions & 12 deletions src/Tests/Microsoft.NET.Build.Tests/GlobalPropertyFlowTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ public void TestGlobalPropertyFlowToLibrary(bool passSelfContained, bool passRun

bool buildingSelfContained = passSelfContained || passRuntimeIdentifier;

ValidateProperties(testAsset, _testProject, expectSelfContained: buildingSelfContained, expectRuntimeIdentifier: passRuntimeIdentifier);
ValidateProperties(testAsset, _testProject, expectSelfContained: buildingSelfContained, expectRuntimeIdentifier: buildingSelfContained);
ValidateProperties(testAsset, _referencedProject, expectSelfContained: false, expectRuntimeIdentifier: false);
}

Expand All @@ -103,8 +103,8 @@ public void TestGlobalPropertyFlowToExe(bool passSelfContained, bool passRuntime

bool buildingSelfContained = passSelfContained || passRuntimeIdentifier;

ValidateProperties(testAsset, _testProject, expectSelfContained: buildingSelfContained, expectRuntimeIdentifier: passRuntimeIdentifier);
ValidateProperties(testAsset, _referencedProject, expectSelfContained: buildingSelfContained, expectRuntimeIdentifier: passRuntimeIdentifier);
ValidateProperties(testAsset, _testProject, expectSelfContained: buildingSelfContained, expectRuntimeIdentifier: buildingSelfContained);
ValidateProperties(testAsset, _referencedProject, expectSelfContained: buildingSelfContained, expectRuntimeIdentifier: buildingSelfContained);
}


Expand Down Expand Up @@ -139,9 +139,9 @@ public void TestGlobalPropertyFlowToExeWithSelfContainedFalse(bool passSelfConta

bool buildingSelfContained = passSelfContained || passRuntimeIdentifier;

ValidateProperties(testAsset, _testProject, expectSelfContained: buildingSelfContained, expectRuntimeIdentifier: passRuntimeIdentifier);
ValidateProperties(testAsset, _testProject, expectSelfContained: buildingSelfContained, expectRuntimeIdentifier: buildingSelfContained);
// SelfContained will only flow to referenced project if it's explicitly passed in this case
ValidateProperties(testAsset, _referencedProject, expectSelfContained: passSelfContained, expectRuntimeIdentifier: passRuntimeIdentifier);
ValidateProperties(testAsset, _referencedProject, expectSelfContained: passSelfContained, expectRuntimeIdentifier: buildingSelfContained);
}
}

Expand All @@ -159,9 +159,11 @@ public void TestGlobalPropertyFlowToLibraryWithRuntimeIdentifier(bool passSelfCo

bool buildingSelfContained = passSelfContained || passRuntimeIdentifier;

ValidateProperties(testAsset, _testProject, expectSelfContained: buildingSelfContained, expectRuntimeIdentifier: passRuntimeIdentifier);
ValidateProperties(testAsset, _referencedProject, expectSelfContained: passSelfContained, expectRuntimeIdentifier: passRuntimeIdentifier,
expectedRuntimeIdentifier: passRuntimeIdentifier ? "" : _referencedProject.RuntimeIdentifier);
ValidateProperties(testAsset, _testProject, expectSelfContained: buildingSelfContained, expectRuntimeIdentifier: buildingSelfContained);
ValidateProperties(testAsset, _referencedProject, expectSelfContained: passSelfContained, expectRuntimeIdentifier: buildingSelfContained,
// Right now passing "--self-contained" also causes the RuntimeIdentifier to be passed as a global property.
// That should change with https://github.com/dotnet/sdk/pull/26143, which will likely require updating this and other tests in this class
expectedRuntimeIdentifier: buildingSelfContained ? "" : _referencedProject.RuntimeIdentifier);
}

[RequiresMSBuildVersionTheory("17.4.0.41702")]
Expand All @@ -184,13 +186,13 @@ public void TestGlobalPropertyFlowToMultitargetedProject(bool passSelfContained,

bool buildingSelfContained = passSelfContained || passRuntimeIdentifier;

ValidateProperties(testAsset, _testProject, expectSelfContained: buildingSelfContained, expectRuntimeIdentifier: passRuntimeIdentifier,
ValidateProperties(testAsset, _testProject, expectSelfContained: buildingSelfContained, expectRuntimeIdentifier: buildingSelfContained,
targetFramework: "net6.0");
ValidateProperties(testAsset, _testProject, expectSelfContained: buildingSelfContained, expectRuntimeIdentifier: passRuntimeIdentifier,
ValidateProperties(testAsset, _testProject, expectSelfContained: buildingSelfContained, expectRuntimeIdentifier: buildingSelfContained,
targetFramework: "net7.0");
ValidateProperties(testAsset, _referencedProject, expectSelfContained: false, expectRuntimeIdentifier: false,
targetFramework: "net6.0");
ValidateProperties(testAsset, _referencedProject, expectSelfContained: buildingSelfContained, expectRuntimeIdentifier: passRuntimeIdentifier,
ValidateProperties(testAsset, _referencedProject, expectSelfContained: buildingSelfContained, expectRuntimeIdentifier: buildingSelfContained,
targetFramework: "net7.0");
}

Expand Down Expand Up @@ -251,7 +253,7 @@ private static void ValidateProperties(TestAsset testAsset, TestProject testProj
targetFramework = targetFramework ?? testProject.TargetFrameworks;


if (string.IsNullOrEmpty(expectedRuntimeIdentifier) && expectRuntimeIdentifier)
if (string.IsNullOrEmpty(expectedRuntimeIdentifier) && (expectSelfContained || expectRuntimeIdentifier))
{
// RuntimeIdentifier might be inferred, so look at the output path to figure out what the actual value used was
string dir = (Path.Combine(testAsset.TestRoot, testProject.Name, "bin", "Debug", targetFramework));
Expand Down
1 change: 1 addition & 0 deletions src/Tests/Microsoft.NET.TestFramework/TestAssetsManager.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ public TestAsset CopyTestAsset(
/// <param name="callingMethod">Defaults to the name of the caller function (presumably the test).
/// Used to prevent file collisions on tests which share the same test project.</param>
/// <param name="identifier">Use this for theories.
/// Pass in the unique theory parameters that can indentify that theory from others.
/// The Identifier is used to distinguish between theory child tests. Generally it should be created using a combination of all of the theory parameter values.
/// This is distinct from the test project name and is used to prevent file collisions between theory tests that use the same test project.</param>
/// <param name="targetExtension">The extension type of the desired test project, e.g. .csproj, or .fsproj.</param>
Expand Down

0 comments on commit c3a68ec

Please sign in to comment.