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

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.

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.

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.

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

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

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. https://github.com/dotnet/sdk/pull/26808 Elsewise, please do close this issue. Thanks for trying it out!

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.

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

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?

Is this going to be triaged for fixing in 7.0.100?

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.

@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.

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://github.com/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.

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

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.