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

RuntimeIdentifier in project is not respected when building/publishing as self-contained #25062

Closed
elinor-fung opened this issue Apr 26, 2022 · 7 comments
Assignees
Labels
Area-CLI good first issue Issues that would be a good fit for someone new to the repository. Narrow in scope, well-defined.
Milestone

Comments

@elinor-fung
Copy link
Member

Describe the bug

RuntimeIdentifier in project file is ignored when building/publishing using dotnet build --self-contained or dotnet publish --self-contained. The runtime identifier of the SDK is always used.

To Reproduce

  1. Create a new app
  2. Update the .csproj to specify a RuntimeIdentifier that is not the same as the SDK you are using. For example, when using a Windows x64 SDK, specify:
    <RuntimeIdentifier>win-x86</RuntimeIdentifier>
  3. Build or publish as self-contained:
    dotnet build --self-contained
    dotnet publish --self-contained

Expected: The application is built/published using the RID specified in the project - win-x86.

Actual: The application is built/published using the RID of the SDK - win-x64 - not the one explicitly set in the project.

Further technical details

For the repro steps above, MSBuild ends up invoked with:
-property:RuntimeIdentifier=win-x64 -property:_CommandLineDefinedRuntimeIdentifier=true

The SDK is explicitly adding the RuntimeIdentifier property on the command line if self-contained is specified on the command line, but the runtime identifier is not. As a result, the SDK's RID is used, overriding any settings in the project.

if (!UserSpecifiedRidOption(parseResult) && isSelfContained)
{
var ridProperties = RuntimeArgFunc(GetCurrentRuntimeId());
selfContainedProperties = selfContainedProperties.Concat(ridProperties);
}

Environment info:

.NET SDK (reflecting any global.json):
 Version:   7.0.100-preview.3.22179.4
 Commit:    c48f2c30ee

Runtime Environment:
 OS Name:     Windows
 OS Version:  10.0.22000
 OS Platform: Windows
 RID:         win10-x64

This is a regression from 6.0.

We hit this trying to update the runtime repo to a newer SDK: dotnet/runtime#67771

@baronfel
Copy link
Member

Verified on 6.0.200 vs 7.0.100-preview.5 builds. We should fix this for a 6.0.xxx release. Workaround is to explicitly specify a RID on the commandline, either via -r alone or via --os and --arch flags.

@baronfel baronfel added good first issue Issues that would be a good fit for someone new to the repository. Narrow in scope, well-defined. and removed untriaged Request triage from a team member labels Apr 26, 2022
@baronfel baronfel added this to the 6.0.4xx milestone Apr 26, 2022
@Tunduk
Copy link
Contributor

Tunduk commented Jun 20, 2022

@baronfel Hi!
I've checked msbuild behavior. RID required if -property:SelfContained=True. I think we should remove logic with RID auto-detection when --self-contained passed.
Then we will have two cases.

  1. A RID specified in a csproj file.
  2. If a RID not specified in a csproj file then user should pass a RID via CLI with --self-contained option

It's the simplest solution but it's a breaking change.

Also, we can parse csproj and try to find a RID but it's even sound complicated =)

What do you think?

@baronfel
Copy link
Member

We're actively moving away from your suggested plan with designs like #26028 - the SDK has a concept of an 'implicit RID', meaning the RID that corresponds to the platform that the user is running on. You can see this in the existing --use-current-runtime CLI option.

I think that @nagilson will end up fixing this issue when he does #26028, because the core problem of this issue is that the CLI is doing RID assignment. If we push RID assignment down into the targets entirely and remove the SDK's explicit assignment (as is the current plan for #26028), then this issue should be resolved implicitly.

@Tunduk
Copy link
Contributor

Tunduk commented Jun 20, 2022

Totally agree with you. It would be better solution.
Can we move this issue to 7.0.1xx milestone? Or even close it?

@marcpopMSFT
Copy link
Member

The linked issue was fixed in 7.0.100 so closing this out.

@Tunduk
Copy link
Contributor

Tunduk commented Oct 26, 2022

The linked issue was fixed in 7.0.100 so closing this out.

@marcpopMSFT sorry, I don't understand. Could you link an issue or PR where this issue is resolved? Just for history.

@marcpopMSFT
Copy link
Member

I believe that #26028 was largely resolved by this PR per my understanding:
#26143

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-CLI good first issue Issues that would be a good fit for someone new to the repository. Narrow in scope, well-defined.
Projects
None yet
Development

No branches or pull requests

5 participants