PublishRelease=true in project file doesn't actually change the configuration to "Release" when running "dotnet publish"

[DamianEdwards]

RE #23551

Trying out <PublishRelease>true</PublishRelease> in a console project and it seems if I also set <PublishAot>true</PublishAot> then a debug build is performed rather than a release build.

HelloWorld.Console.csproj

<Project Sdk="Microsoft.NET.Sdk">

  <PropertyGroup>
    <OutputType>Exe</OutputType>
    <TargetFramework>net7.0</TargetFramework>
    <ImplicitUsings>enable</ImplicitUsings>
    <Nullable>enable</Nullable>
    <PublishRelease>true</PublishRelease>
    <PublishAot>true</PublishAot>
  </PropertyGroup>
</Project>

Run dotnet publish HelloWorld.Console -r win-x64 --self-contained -v -o .artifacts\HelloWorld.Console

Output:

Publishing HelloWorld.Console: dotnet publish -r win10-x64 --self-contained
  Determining projects to restore...
  Restored ~\HelloWorld.Console\HelloWorld.Console.csproj (in 72 ms).
C:\Program Files\dotnet\sdk\7.0.100-rc.1.22368.2\Sdks\Microsoft.NET.Sdk\targets\Microsoft.NET.RuntimeIdentifierInference.targets(219
,5): message NETSDK1057: You are using a preview version of .NET. See: https://aka.ms/dotnet-support-policy [~\HelloWorld.Console\HelloWorld.Console.csproj]
  HelloWorld.Console -> ~\HelloWorld.Console\bin\Debug\net7.0\win10-x64\HelloWorld.Console
  .dll
  Generating compatible native code. To optimize for size or speed, visit https://aka.ms/OptimizeNativeAOT
     Creating library bin\Debug\net7.0\win10-x64\native\HelloWorld.Console.lib and object bin\Debug\net7.0\win10-x64\native\HelloWorld.Console.exp
  HelloWorld.Console -> ~\.artifacts\HelloWorld.Console\

dotnet --info

.NET SDK:
 Version:   7.0.100-rc.1.22368.2
 Commit:    a9c056cd39

Runtime Environment:
 OS Name:     Windows
 OS Version:  10.0.22621
 OS Platform: Windows
 RID:         win10-x64
 Base Path:   C:\Program Files\dotnet\sdk\7.0.100-rc.1.22368.2\

Host:
  Version:      7.0.0-rc.1.22366.5
  Architecture: x64
  Commit:       072eda8d6b

.NET SDKs installed:
  6.0.302 [C:\Program Files\dotnet\sdk]
  6.0.400-preview.22330.6 [C:\Program Files\dotnet\sdk]
  7.0.100-rc.1.22368.2 [C:\Program Files\dotnet\sdk]

.NET runtimes installed:
  Microsoft.AspNetCore.App 3.1.27 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 5.0.17 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 6.0.6 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 6.0.7 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 7.0.0-rc.1.22367.2 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.NETCore.App 3.1.27 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 5.0.17 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 6.0.6 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 6.0.7 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 7.0.0-preview.5.22301.12 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 7.0.0-rc.1.22366.5 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.WindowsDesktop.App 3.1.27 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
  Microsoft.WindowsDesktop.App 5.0.17 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
  Microsoft.WindowsDesktop.App 6.0.6 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
  Microsoft.WindowsDesktop.App 6.0.7 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
  Microsoft.WindowsDesktop.App 7.0.0-preview.5.22302.5 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
  Microsoft.WindowsDesktop.App 7.0.0-rc.1.22366.1 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]

Other architectures found:
  x86   [C:\Program Files (x86)\dotnet]

Environment variables:
  Not set

Learn more:
  https://aka.ms/dotnet/info

Download .NET:
  https://aka.ms/dotnet/download

@nagilson @richlander

[DamianEdwards]

OK actually it might not be related to <PublishAot> at all. The property <PublishRelease> doesn't appear to be changing the configuration.

[DamianEdwards]

It seems to work if I pass it in as a cmd line arg but not in the project file. Is this a property evaluation ordering issue perhaps?

[nagilson]

That is expected. It should work if you add it to Directory.Build.props instead of the .csproj. Try that and let me know how it goes. https://docs.microsoft.com/en-us/dotnet/core/project-sdk/msbuild-props#publishrelease

If there is a concern that this should work in the .csproj: well; I agree that would be better, and we can raise that issue. The reason it ended up being put into there instead of the .csproj from the implementation side is we had discussions questioning the feasibility of it evaluating early enough if it was put into the .csproj and it seemed to cause issues.

[DamianEdwards]

I really think this needs to be supported in the project files, as all the other Publish prefixed MSBuild properties do today and not doing so for this (or the other new properties) makes this confusing.

[baronfel]

I agree with @DamianEdwards - what's the reason we need the props file? Is there some MSBuild limitation we're hitting up against?

[nagilson]

I agree it's confusing, I'll investigate this. Thanks for taking the time to test it. @baronfel I do believe that was why but there is probably a work around.

[richlander]

You can already put Configuration in .csproj, right? I would assume this is similar.

[nagilson]

I think the main reason this is different is we want it to only take place if we are publishing but not if we are building. Otherwise PublishRelease will also make builds default to release. And there might not be a way to tell which one we are doing in time for it to be in the .csproj. If anything comes to mind, please do correct me or let me know. Kinda concerned about this though, still looking.

[DamianEdwards]

Seems there's a property that's set by the CLI when dotnet publish is run that indicates that a publish is being executed: https:/dotnet/sdk/blob/main/src/Cli/dotnet/commands/dotnet-publish/Program.cs#L39

Of course that's only true for invocations from the CLI, we'd need this to work for folks directly publishing from VS, or just invoking the publish target too.

[DamianEdwards]

Adding @dsplaisted in case he has input.

[nagilson]

@DamianEdwards I added that and that's how PublishRelease currently works. It turns out I was wrong and that's not the issue. That part is fine, though you're right it doesn't work invoking the publish target and we may need to fix that too in a separate issue. Previously I talked to Daniel about that, and we determined it likely wasn't possible.

The issue here is that the .csproj gets imported (very far) after the .NET.SDK.props is imported. Note that .NET.SDK.props is what sets Configuration. If PublishRelease is put in directory.build.props, that gets imported just before Net.SDK.Props, so it works as and it's literally the first thing that gets imported. Importing .csproj before .NET.SDK.props to flow PublishRelease in could break a lot of stuff. But overriding configuration when the .csproj is imported also doesn't work, because Optimize/Debug symbols are resolved much earlier than that. Either way we do it, if we enable this working in the csproj stuff is going to break.

Providing a potential workaround in a second but it's also not the best... Ah. That also breaks other stuff. Still looking for a solution.

[KalleOlaviNiemitalo]

I suppose the project author could avoid the Project/@SDK attribute and instead explicitly import Sdk.props and Sdk.targets as shown in How to: Use MSBuild project SDKs; then the csproj would be able to define PublishRelease before the imports. That doesn't seem any more convenient than Directory.Build.props, though.

[nagilson]

Thanks for pulling that up, it can work but I agree it's not optimal. Good news, though! @benvillalobos looked at this with me and it looks like even if Optimize and Debug Symbols are temporarily wrong it will be fine. (Unless someone specifically relies on Configuration in the .csproj and also adds PublishRelease, but there are likely workarounds and they would be breaking themselves so they'd have more knowledge about what's happening.) And that is what a custom Configuration is taking advantage of, so your intuition there was correct @richlander. I also think we can use this to get it working for VS Publish and such.

[benvillalobos]

Chiming in with a bit of context: We saw that the value of Configuration (very early on) determines the values of Optimize and DebugSymbols. Thankfully, Optimize and DebugSymbols aren't really used until Csc is called. From there we concluded that we can add a condition in the sdk's publish targets that set the two properties to what they should be under a Release configuration, and this would happen well before csc would be called.

[DamianEdwards]

Is this going to be triaged for fixing in 7.0.100?

[nagilson]

That is correct, thank you for checking in and pushing for the right changes @DamianEdwards for our customers. I have been working on this amongst other issues due for 7.0.100. This needs to be completed by Aug 12 for that, correct?

[richlander]

I cannot comment on the dates. Only @marcpopMSFT can.

[DamianEdwards]

Aug 12 is the code-complete date for rc.1 so if we want it in rc.1 then I'd assume that's the date the change is needed in main by.

[nagilson]

From a standpoint of publishing a solution, this becomes more complicated. For example, if a solution contains a top-level project defining PublishRelease=true but depends on a class library who set PublishRelease=false. Another issue is if two top-level projects in a solution disagree on PublishRelease. It is possible to support that by analyzing the dependency graph and building multiple configurations at the same time but that is not completable by 7.0.100.

We decided to do the following: Check PublishRelease at the top-level project and respect that value for the rest of the subprojects. If two top-level published projects in a solution disagree on PublishRelease we throw an error to be defensive.

[nagilson]

We got this working on dotnet publish foo.sln but even with parallelization any approach to get it in for 7.0.100 is too much of a performance regression (roughly +7% for small solution publishing to +25% time for publishing solutions with hundred+ projects.) So, we're gonna make it work with projects but not solutions and document that. I'll open another issue where we can discuss it. FTR, this is still going to work with projects for 7.0.100.

From a standpoint of publishing a solution, this becomes more complicated. For example, if a solution contains a top-level project defining PublishRelease=true but depends on a class library who set PublishRelease=false. Another issue is if two top-level projects in a solution disagree on PublishRelease. It is possible to support that by analyzing the dependency graph and building multiple configurations at the same time but that is not completable by 7.0.100.

We decided to do the following: Check PublishRelease at the top-level project and respect that value for the rest of the subprojects. If two top-level published projects in a solution disagree on PublishRelease we throw an error to be defensive. Sound good @DamianEdwards ?

[nagilson]

We decided to make it work for solutions via opt-in so the performance cost of checking this in a solution is not there by default. If a user tries to use PublishRelease or PackRelease with a solution they are informed of how to opt-in.

[DamianEdwards]

@nagilson thanks for the updates. What release will the fix go into? rc.2?

[nagilson]

@DamianEdwards Indeed, I'm expecting it to go in rc.2. The PR has been thoroughly tested and reviewed, just waiting for the final thumbs-up at this point. But we are heavily backlogged with a number of p1 issues (part of the rc.1 delay -- sorry!)

FYI The implicit RID and PublishSelfContained work is in a similar state, though needs some test updates and review. And of course, thanks for checking in.

[nagilson]

This was fixed last week in the release/7.0.1xx branch and should flow to the rc-2 branch when that's forked in the next week or two. Please let me know if any issues are encountered or you have any questions. #26808 Elsewise, please do close this issue. Thanks for trying it out!

[DamianEdwards]

Thanks, I'll try this out once we have rc.2 builds.

[DamianEdwards]

Confirmed this works as expected now in rc.2 builds, thanks!

[nagilson]

Awesome news, thanks for testing this 😄