dotnet publish should default to release mode

[benaadams]

Currently it defaults to debug (bad for performance of deployed sites, libraries put on nuget, and apps)

While this is obvious when using dotnet publish without parameters as it puts it in a Debug folder; it is not obvious when using an output setting as it is put in the output directory without an obvious Debug moniker

[livarcocc]

@KathleenDollard to consider this breaking change.

[forki]

It's not breaking. It's fixing. Livar <notifications@github.com> schrieb am Sa., 29. Juni 2019, 18:26:

…

@KathleenDollard <https://github.com/KathleenDollard> to consider this breaking change. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <https://github.com/dotnet/cli/issues/11668?email_source=notifications&email_token=AAAOANE7DEBURGDBUXHJ5ELP46EMXA5CNFSM4H4KD6AKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODY334WI#issuecomment-506969689>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAAOANDVRHGGH25CIE3JDHTP46EMXANCNFSM4H4KD6AA> .

[benaadams]

Was pointed out pre-1.0 that this was incorrect behaviour https://github.com/dotnet/cli/issues/477 however the issue was closed but not fixed

There are number of examples of people putting Debug builds into production and Debug libs onto NuGet

[nguerrera]

This is not as simple as it might seem.

dotnet publish doesn't explicitly set the configuration to Debug. It just does what msbuild /t:Publish does without a /p:Configuration=X argument. That is not necessarily debug: try it with a Directory.Build.props like this:

<Project>
  <PropertyGroup>
    <Configuration Condition=" '$(Configuration)' == '' ">Release</Configuration>
  </PropertyGroup>
</Project>

Further note that configurations can be arbitrarily customized. There might not even be one called Release in the project. Forcing /p:Configuration=Release by default is most definitely a breaking change. It might not even work. Or it might override to a less efficient configuration than a custom configuration that is not named Release and made to be the default in project code like above. And even when it works, it will certainly break cases where people are relying on getting Debug in the case where they have not passed an explicit configuration.

Microsoft.NET.Sdk chooses Debug by default here:

sdk/src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.Sdk.props

Line 23 in ef7932d

<Configuration Condition=" '$(Configuration)' == '' ">Debug</Configuration>

This applies to more than publish, and like so many other things in those props, it maps to what got spelled out in File -> New in classic projects. Breaking with that ancient tradition will likely have other unintended consequences.

[benaadams]

Perhaps a more general way of capturing the requirement is dotnet publish should default to optimized output if it is not overriden.

[nguerrera]

There are a lot of different ways to interpret that requirement.

My above comment demonstrates why changing dotnet publish to be equivalent to today’s dotnet publish -c Release is very problematic, technically.

It’s certainly possible that a more involved change could work. We’d need a concrete proposal to evaluate that.

[benaadams]

Microsoft.NET.Sdk chooses Debug by default here:

Can publish and build/run be differentiated and the default swapped for publish?

[nguerrera]

Possibly. We could set /p:RunningDotNetPublishWithDefaultConfig=true. And change above based on that. Or something. It would mean that dotnet publish and msbuild /t:Publish are different, though.

[benaadams]

It would mean that dotnet publish and msbuild /t:Publish are different, though.

Solving the compat worries in one fell swoop! 😉

Also I hear there's a major version (this and the next) which would be an ideal time for such a change 😄

[nguerrera]

Just to set expectations, I think we’re way too late for 3, but happy to explore this for 5.

[benaadams]

Can this be done for 5.0?

[benaadams]

but happy to explore this for 5.

@nguerrera @KathleenDollard any news?

[forki]

Thanks for still pushing them to do the right thing Ben

[benaadams]

Recent example of someone having performance issues from not publishing in release mode #12834

[nguerrera]

@benaadams Sorry, I don’t work on this project or .NET anymore. It’s been five months since I left so I don’t know if there’s been any development on this.

[richlander]

We're going to take another (and hopefully final) run at resolving this for .NET 6.0.

[forki]

Lol

[tmds]

<Project>
  <PropertyGroup>
    <Configuration Condition=" '$(Configuration)' == '' ">Release</Configuration>
  </PropertyGroup>
</Project>

I guess there is no way to set this conditionally based on the target because there is no property that describes the targets that are being built/requested?

[DanielHabenicht]

@richlander here we are, shortly before 6.0 release (no pressure 😄)

[forki]

[EskeRahn]

A little late to the party, but one could say that the issue here is semantics causing confusion.
With "publish" one sort of intuitively expect that it is actually about publishing. And not about producing code for debugging.

But taking a step backwards this also should match msbuild.

Perhaps the clean way would be to always have a default release path as well as the default debug path, and to define two new keywords for compiling to debug and to release. And have "publish" deprecated but mirroring the debug for backwards compatibility. Perhaps simply something like todebug and torelease?
And if people have multiple configuration profiles obviously allow us to pick a specific one by name

[richlander]

FYI: #23551

[dsplaisted]

We think that we can do this for projects that target .NET 8 or higher, by setting the PublishRelease property to true (if not already set) in the .NET SDK targets if the project targets .NET 8 or higher.

[dsplaisted]

This is done and can be closed, right @nagilson?

[nagilson]

That's correct. We've changed the default to Release for 8.0+ TFMS: #29155 ( as well as for pack.)

Was waiting for the release so there were no more 🥅 ;)