-
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
Enable publishing as Release
with PublishRelease
#23551
Comments
/cc @dotnet/nuget-team |
Today the default is selected based on the project's default, this is what you want to change/break, correct @richlander |
Release
assets by default
Yes and no. By default, the default is Or do I misunderstand? |
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) |
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 Stating the obvious, it would look like this:
Sound good? |
@richlander Would we also want a property for 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. |
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:
|
I implemented a fix via MSBuild but just used |
To my mind, the proposed properties are better, for two reasons:
|
Echoing @richlander here - we should have distinct properties for each case. |
Ok, thanks for clarifying. Will be likely MIA on this until Friday, but I'll wrap it up soon. |
@joeloff and I were having a discussion and this might break more than we thought. If a user provides the
|
Those pipelines would only break if they set |
This is true, though we want |
Ah. That's the misunderstanding. The intention is that Customers that want this experience would set this value in their project file or Sound good? |
Talking to various folks on the team and we wanted to point out that this is now activated (in the PR) for (We don't think there's a way to do it for that target-based invocation of |
It would be better if |
I agree it would be better, sounds good to me. |
Release
assets by defaultRelease
with PublishRelease
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
When This approach enables an non-breaking opt-in experience that matches the pattern of the other |
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. |
@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 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 |
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. |
@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 ? |
@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.
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 |
Makes sense. Can you create a new issue on that with this context? |
Thanks. Looks good. FYI @nagilson |
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 |
@richlander People usually set Properties in project file. Meaning after |
@Nirmal4G If |
I meant to move or copy the entire graph of properties that get affected by the 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 |
I personally don't want different behavior for |
@Nirmal4G Coincidentally, we had a similar discussion yesterday with another proposed change to MSBuild to allow us to detect |
Which one, MSBuild language feature, like adding |
@Nirmal4G |
Yeah, The |
Enable publishing as
Release
withPublishRelease
[breaking] dotnet publish/pack produce
Release
assets by defaultProposal: 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:
Debug
artifacts would be deployed into production. Developers would not have specify as much via the CLI to do the right thing.debug
mode).Context
Our basic guidance to developers since .NET Core 1.0 has been "use
build
for development andpublish
for prod". Given that, it would make a lot more sense ifpublish
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 ofbuild
andpublish
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, nobuild
andpublish
analogues. There is justpack
. 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, thenpack
it and upload it to their NuGet server/feed of choice. In that workflow,pack
defaulting toRelease
would be preferred. I haven't done the exercise, but it would interesting to see the split betweenRelease
andDebug
libraries on NuGet.org. There should be none (except for some very specific scenarios).There absolutely is a
Debug
scenario with NuGet packages and withpack
. I'm guessing that it is <10% case. The scenario would be that developers build their package asDebug
, 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 aDebug
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:
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.
The text was updated successfully, but these errors were encountered: