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.

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

Update handling of SelfContained and RuntimeIdentifier properties when specified on the command line #21986

Merged
merged 12 commits into from
Aug 22, 2022

Conversation

dsplaisted
Copy link
Member

Fixes #21677

Also changes how the SelfContained and RuntimeIdentifier properties flow between projects when they are specified on the command line. More details are in the comments and the breaking change documentation included in this PR.

This also requires the changes in dotnet/msbuild#6924

Comment on lines 82 to 84
<AcceptsRuntimeIdentifier Condition="'$(_IsExecutable)' == 'true' Or
'$(RuntimeIdentifier)' != '' Or
'$(RuntimeIdentifiers)' != ''">true</AcceptsRuntimeIdentifier>
Copy link
Member

Choose a reason for hiding this comment

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

Does this imply that if the project has any RID, we'll flow the RID from the entrypoint? Is that what we want? What if e.g. the entry-point is win11-x64 and the reference is win7-x86? That seems like an OK combination to me . . .

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, we will flow any RID. Most of this is existing logic, ie if RuntimeIdentifier or RuntimeIdentifiers was set in the referenced project, we would have flowed the RID since .NET Core 1.0 I think. The change here is to also flow it if the referenced project is an Exe project, but not if it's a test project.

@@ -53,6 +55,14 @@ protected override void ExecuteCore()
bool referencedProjectIsExecutable = MSBuildUtilities.ConvertStringToBool(projectAdditionalProperties["_IsExecutable"]);
bool referencedProjectIsSelfContained = MSBuildUtilities.ConvertStringToBool(projectAdditionalProperties["SelfContained"]);

if (SelfContainedIsGlobalProperty && project.GetBooleanMetadata("AcceptsRuntimeIdentifier") == true)
Copy link
Member

Choose a reason for hiding this comment

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

Since you need this in a task, use dotnet/msbuild#4939 and avoid the try-overwrite gunk.

@dsplaisted dsplaisted changed the base branch from release/6.0.1xx to release/6.0.2xx November 18, 2021 01:30
@beatcracker
Copy link

Folks, FYI: looks like #10566 is related to this.

@dsplaisted dsplaisted changed the base branch from release/6.0.2xx to main August 12, 2022 00:49
@dsplaisted dsplaisted marked this pull request as ready for review August 12, 2022 14:35
@dsplaisted dsplaisted requested review from joeloff and a team August 18, 2022 19:32

## New Behavior

Both `SelfContained` and `RuntimeIdentifier` will flow a referenced project if any of the following are true for the referenced project:
Copy link
Member

Choose a reason for hiding this comment

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

Should that read will flow to a referenced...?

@dsplaisted dsplaisted changed the base branch from main to release/7.0.1xx-rc1 August 19, 2022 22:59
@dsplaisted dsplaisted merged commit 5d20158 into dotnet:release/7.0.1xx-rc1 Aug 22, 2022
Forgind pushed a commit to dotnet/msbuild that referenced this pull request Sep 28, 2022
Fixes #7995

Context
#6924 changed how IsRidAgnostic works. Normally, the .NET SDK should set the IsRidAgnostic property (added in dotnet/sdk#21986). But when using an older version of the .NET SDK, there is fallback logic in the GetTargetFrameworksWithPlatformForSingleTargetFramework target to replicate the previous logic.

However, this fallback logic was incorrect, as it was setting item metadata, but reading from the property to determine whether to set it to the default value. So if the property wasn't set, the IsRidAgnostic metadata would always be false.

Testing
Manual testing
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants