Enable publishing as Release with PublishRelease

[richlander]

Enable publishing as Release with PublishRelease

The following proposal has been replaced with this new (non-breaking) plan: #23551 (comment)

[breaking] dotnet publish/pack produce Release assets by default

Proposal: Default to Release builds for CLI commands that are strongly aligned with production deployment. Release builds are faster, and debug builds are not supported in production.

That means:

• Fewer Debug artifacts would be deployed into production. Developers would not have specify as much via the CLI to do the right thing.
• Some developers would be surprised during debugging (builds would be optimized and less debuggable) but there would be an easy workaround to resolve that (opt into debug mode).

Context

Our basic guidance to developers since .NET Core 1.0 has been "use build for development and publish for prod". Given that, it would make a lot more sense if publish defaulted to a release build. Debug builds run observably slow (sometimes you can see this with just your eyes; no stopwatch required). It is near certain that plenty of .NET apps are deployed as debug due to the current defaults. The CLI would be much better if it offered more differentiated options. There should probably be a more broad re-assessment of build and publish but I'm not digging into that here. I'm proposing a much simpler change (that doesn't preclude broader changes later; in fact, this change would encourage them).

pack is a bit more nuanced since there is no duality set of commands for it, no build and publish analogues. There is just pack. In fact, the same could be said of libraries in general. I assert that most developers go through their debug cycle with their NuGet library as just a library, then pack it and upload it to their NuGet server/feed of choice. In that workflow, pack defaulting to Release would be preferred. I haven't done the exercise, but it would interesting to see the split between Release and Debug libraries on NuGet.org. There should be none (except for some very specific scenarios).

There absolutely is a Debug scenario with NuGet packages and with pack. I'm guessing that it is <10% case. The scenario would be that developers build their package as Debug, deploy to a feed (which could just be a disk location), then run tests (in one form or another), and use the debugger in library code to determine library behavior. For that to work really well, they may have to use SourceLink. I haven't tested the non-SourceLink flow for a Debug NuGet package (where I have matching source) in quite some time. That's a detail to explore. The primary point is that debugging NuGet packages is a bit more involved than debugging ProjectReference libraries, so it probably happens a lot less often.

Zooming out, we should ensure that we have good gestures and good guidance for:

• Do this simple thing for the best local debug experience.
• Do this simple thing for the best prod experience.

I assert that our system is too biased to debugging and insufficiently focused on building optimized prod artifacts. At the end of the day, it's all about a great prod experience, so we should focus more on prod ergonomics. This concern isn't novel. I've heard developers complain about this exact issue for years, but no one (AFAIK) has ever written it up to resolve that.

[JonDouglas]

/cc @dotnet/nuget-team

[dasMulli]

Suggest explictily using uppercasing in text - Release / Debug - to avoid confusion and people running into breaking scenarios with case-sensitive filesstems such as in hosted linux-based CI or container scenarios. (#375, #9230, #1204)

[joeloff]

Today the default is selected based on the project's default, this is what you want to change/break, correct @richlander

[richlander]

Yes and no. By default, the default is Debug. If we made this change and someone had <Configuration>Debug</Configuration> in their project file, it should remain Debug. I'm not proposing to change that.

Or do I misunderstand?

[nagilson]

If I am understanding correctly, the considered breaking nature of this change is best described here: #10357 (comment) (TLDR: Some customers may expect and rely on publish to default to the debug configuration and we will be removing that)

[richlander]

Interesting. I like this solution: #10357 (comment).

.NET has always suffered from a lack of global/repo config (like what git has). This property seems like a poster child for that.

Perhaps, we should should create a new property PublishConfiguration. It would match all the other Publish* properties, like PublishTrimmed. This property could then be added to more global configuration, like Directory.Build.props.

Stating the obvious, it would look like this:

<PublishConfiguration>Release</PublishConfiguration>

Sound good?

[nagilson]

@richlander Would we also want a property for PackConfiguration? We have set a standard throughout commands such as clean which also contains the --configuration option.

I agree the property could benefit from having this exposed global/repo config and I like this idea. But would we want to change other commands to follow this pattern? This would also be a bit awkward, perhaps, from a customer perspective because we have this one argument option which is now a property but the rest remain arguments. It seems like a potentially much larger change in scope than initially suggested (changing the default behavior.)

Please let me know your thoughts.

[richlander]

I'm not proposing that we build global/repo config to solve this problem. I'm merely saying that this issue raises that need.

We could do something like this:

<Configuration Condition=" '$(PublishRelease)' == 'True' " and '$(Configuration)' = ''>Release</Configuration>
<Configuration Condition=" '$(PackRelease)' == 'True' " and '$(Configuration)' = ''>Release</Configuration>

It is very similar to what's already there:

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

Line 23 in 37b5956

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

[nagilson]

I implemented a fix via MSBuild but just used _ShouldDefaultToRelease instead of two separate variables; I think this is cleaner. (#25991) @baronfel Do we want that more granular control right now? I would appreciate confirmation, plus that we want to use the names PublishRelease and PackRelease if so, or if _ShouldDefaultToRelease is insufficient. There are a lot of hard-coded tests I will have to change, would like to confirm before I dive into that.

[richlander]

To my mind, the proposed properties are better, for two reasons:

• We already have the Publish* scheme, and this approach matches it.
• Publish* and Pack* obviously contrast with build. It's unclear when _ShouldDefaultToRelease would be effective. We don't want it to be effective for dotnet build.

[baronfel]

Echoing @richlander here - we should have distinct properties for each case.

[nagilson]

Ok, thanks for clarifying. Will be likely MIA on this until Friday, but I'll wrap it up soon.

[nagilson]

@joeloff and I were having a discussion and this might break more than we thought.

If a user provides the --no-build option and has a build step in their CI, it will fail because the build will target to Debug but the Publish will target to Release and the files will be missing. This is why the current test ItPublishesSuccessfullyWithNoBuildIfPreviouslyBuilt is failing in my implemented patch.

1. So, using --no-build in Publish now requires explicit specification of the Release configuration on build. How many pipelines will we be breaking by doing this versus how many people are saved by this change? @baronfel @richlander

2. OR we could point Publish to not default to Release if --no-build is used? How will we communicate these changes? It seems like separating the two is going to get (potentially) very confusing in this case.

[richlander]

Those pipelines would only break if they set PublishRelease = true. If they do so and their CI breaks due to that issue, that's good, right?

[nagilson]

Those pipelines would only break if they set PublishRelease = true. If they do so and their CI breaks due to that issue, that's good, right?

This is true, though we want PublishRelease to be true by default, correct? Or have I misunderstood, and we want that to be false by default, so Publish and Pack still default to Debug, so a user must opt-in to having publish and pack default to Release?

[richlander]

Ah. That's the misunderstanding. The intention is that *Release is an opt-in scenario. The values would be unset or false by default.

Customers that want this experience would set this value in their project file or Directory.Build.props. Certainly, if customers give us feedback that they want this value always set, that's something we can consider with .NET 8.

Sound good?

[nagilson]

Talking to various folks on the team and we wanted to point out that this is now activated (in the PR) for dotnet publish but not dotnet build /t:publish. I think that aligns with this proposal's vision still, correct? Do we want to explicitly communicate that somewhere?

(We don't think there's a way to do it for that target-based invocation of publish/pack because we cannot tell what the targeted command is early on enough with how all of our .props works atm.)

[richlander]

It would be better if publish and /t:publish did the same thing. However, I don't see that as a blocker. If we get feedback that this is a problem, we can reconsider. Fair?

[nagilson]

I agree it would be better, sounds good to me.

[richlander]

This conversation is a bit circuitous (since we were designing on the fly). Here's a quick recap of the plan.

We'll expose two new Publish* boolean properties:

• PublishRelease
• PackRelease

WhenPublishRelease is to true, dotnet publish will produce a Release build (w/o needing to specify -c Release). Same thing for PackRelease and dotnet pack.

This approach enables an non-breaking opt-in experience that matches the pattern of the other Publish* properties.

[nagilson]

This was implemented here #25991 and was merged in as described above. To prevent any confusion for other parties reading this, I'll close this for now. If this needs to remain open for the other issues, feel free to ping and I'll reopen it.

[eatdrinksleepcode]

@richlander @nagilson apologies for the direct ping, but I'm not sure you will see a comment on a closed issue without it.

My current project does not have a Release configuration. How can I take advantage of this behavior?

In the C# project system, defaults have historically been defined in the project itself, with the typical conventions supplied in templates, to cater to the flexibility allowed in defining configurations. As such I would have thought that a better way to provide this option to customers than a boolean PublishRelease property would be a DefaultPublishConfiguration property.

[richlander]

No apologies required.

Can you share what your project configurations are?

For most folks, I think a boolean property is best. We expected that some folks do customize project configurations but were unsure of how to approach that since we didn't have enough info on that. Any info you can share will be valuable.

@baronfel

[nagilson]

@eatdrinksleepcode No need to apologize. See https://docs.microsoft.com/en-us/dotnet/core/project-sdk/msbuild-props#publishrelease for documentation and usage details. Note that this is in the main branch of the SDK and not in production yet. If there is demand for it, we could do another servicing QB push for 6.0.4xx @marcpopMSFT ?

[eatdrinksleepcode]

@richlander @nagilson We use configurations to build for different deployment targets. For example, we have a separate configuration for building a version of our app that is designed to run on an AWS AMI. We recently added configurations for creating a FIPS-compliant version of our app by excluding non-compliant code. There are more examples.

For most folks, I think a boolean property is best.

I agree that most people stick with the default "Release" configuration (though my experience is that this is becoming less true since SDK-style projects have encouraged people to actually understand and own how their build works). But I don't think that <PublishConfiguration>Release</PublishConfiguration> is any harder or less usable for those people than <PublishRelease>true</PublishRelease>; but it has way more utility for people who do customize their build configurations.

[richlander]

Makes sense.

Can you create a new issue on that with this context?

[eatdrinksleepcode]

@richlander done

[richlander]

Thanks. Looks good.

FYI @nagilson

[richlander]

We no longer need evidence to fund this, but I wanted to raise today's example where this feature would be useful. I was updating our Docker samples today. I realized that all of the Configuration values were in lower-case (which I believe can do something different). Lots of smart and skilled folks have looked at these samples of the years and they have been wrong the whole time. Ughh! The fact that we cannot do this right for customer examples that are intended to be helpful guidance is a sign. I had a moment with that.

dotnet/dotnet-docker#3989

[Nirmal4G]

@richlander People usually set Properties in project file. Meaning after Sdk.props. That's why the first implementation didn't work. You can move it to Sdk.BeforeCommon<CrossTargeting>.targets and the rest will be history.

[nagilson]

@Nirmal4G If Configuration is modified in Sdk.BeforeCommon<CrossTargeting>.targets, I think that would be too late because it wouldn't affect values that rely on the Configuration in Microsoft.NET.Sdk.props (DebugSymbols, Optimize.) Even if we edit them later on, there might be external things that depend on those values being correct from the very beginning of the build. If that actually works though, that would be awesome.

[Nirmal4G]

I meant to move or copy the entire graph of properties that get affected by the Configuration. This would be breaking, yes but that's the cost of it. I can't see any other way to enable this. Either you have to duplicate the logic to preserve compat or move it in which case breaking.

It's precisely because of these scenarios, I asked MSBuild to implement a pattern like this

<Project Sdk="Microsot.NET.Sdk">
  <GlobalPropertyGroup>
     <Configuration>Release</Configuration>
  </GlobalPropertyGroup>
  <PropertyGroup Scope="Initial/Global">
     <Configuration>Release</Configuration>
  </PropertyGroup>
  <PropertyGroup BeforeSdk="SdkName">
     <Configuration>Release</Configuration>
  </PropertyGroup>
</Project>

If this is implemented, you could essentially move a host of properties into the first implicit props in the SDK. The major beneficiary would be the TargetFramework(s) properties.

[Nirmal4G]

I personally don't want different behavior for dotnet publish and msbuild -t:Publish and friends.

[nagilson]

@Nirmal4G Coincidentally, we had a similar discussion yesterday with another proposed change to MSBuild to allow us to detect IsPublishing so there's better feature parity between -t:Publish and dotnet publish. But so far we haven't come up with a way to do it that isn't breaking. Of course, the separate issue you mention is that we now run an evaluation inside of the SDK CLI to check PublishRelease so that doesn't solve the complete picture. I'm not sure if your proposed change would impact design time builds. I don't really have a good answer for you yet, but I too want feature parity, whether publishing in the SDK or not. It's something we are still working on, thanks for voicing it out here.

[Nirmal4G]

I'm not sure if your proposed change would impact design time builds.

Which one, MSBuild language feature, like adding GlobalPropertyGroup or Scoped PropertyGroup that I mentioned above or the copying of the properties to the targets? Neither does impact design time builds. For both the solutions, it doesn't even matter if the build is design time or not!

[nagilson]

@Nirmal4G GlobalPropertyGroup: I would quite like it too though it has its pros and cons. There is https://learn.microsoft.com/en-us/dotnet/api/microsoft.build.construction.projectrootelement.treataslocalproperty?view=msbuild-17-netcore which sort of serves a similar purpose. Thanks for clarifying!

[Nirmal4G]

Yeah, The TreatAsLocalProperty sets the scope to the defined file itself. If we bring the scope concept to MSBuild, we can do so much more, like setting read-only, local-only, global, etc... for both properties and items.