-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
dotnet publish should default to release mode #10357
Comments
@KathleenDollard to consider this breaking change. |
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>
.
|
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 |
This is not as simple as it might seem.
<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:
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. |
Perhaps a more general way of capturing the requirement is |
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. |
Can |
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. |
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 😄 |
Just to set expectations, I think we’re way too late for 3, but happy to explore this for 5. |
Can this be done for 5.0? |
@nguerrera @KathleenDollard any news? |
Thanks for still pushing them to do the right thing Ben |
Recent example of someone having performance issues from not publishing in release mode #12834 |
@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. |
We're going to take another (and hopefully final) run at resolving this for .NET 6.0. |
Lol |
<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? |
@richlander here we are, shortly before 6.0 release (no pressure 😄) |
A little late to the party, but one could say that the issue here is semantics causing confusion. 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? |
FYI: #23551 |
We think that we can do this for projects that target .NET 8 or higher, by setting the |
This is done and can be closed, right @nagilson? |
That's correct. We've changed the default to Was waiting for the release so there were no more 🥅 ;) |
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 obviousDebug
monikerThe text was updated successfully, but these errors were encountered: