Make `dotnet publish` use `Release` by default in 8.0+ Projects #29155

nagilson · 2022-11-22T00:37:58Z

Changes:

dotnet publish (AND dotnet pack) use a configuration of Release by default now if the TFM is above 8. dotnet pack will use Release REGARDLESS*** of the TFM.
The environment variables to opt in or out for solutions have been removed. The error before you'd get using them was tighter than the new error, and while you won't be warned you need to turn on the ENV var anymore, you just don't need to.
There's a new Environment variable that disables the features all-together called DOTNET_CLI_DISABLE_PUBLISH_AND_PACK_RELEASE
<Configuration> in a project file never worked to begin with. This no longer considers if you set <Configuration> in your project file. Frankly, if that was supported, it would have dramatic consequences.
If you multi-target with 8.0, I initially wrote it so you're going to get the Release behavior for each TFM in dotnet pack. No way to go around that besides making this an 8.0 SDK thing and not a TFM thing. FWIW dotnet publish with TargetFrameworks won't work so it picks whatever you used with -f.

…emaining p*release tests.

src/Assets/TestProjects/TestAppWithSlnUsingPublishReleaseConflictingValues/App.sln

src/Cli/dotnet/commands/dotnet-pack/PackCommand.cs

src/Common/EnvironmentVariableNames.cs

src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.Publish.targets

src/Tasks/Common/Resources/Strings.resx

src/Tests/Microsoft.DotNet.PackageInstall.Tests/Microsoft.DotNet.PackageInstall.Tests.csproj

src/Tests/Microsoft.NET.Publish.Tests/GivenThatWeWantToPublishAHelloWorldProject.cs

src/Cli/dotnet/ReleasePropertyProjectLocator.cs

…how that got there

…eting has been broken since at least net 5.0 so not doing that.

src/Cli/dotnet/ReleasePropertyProjectLocator.cs

nagilson · 2022-12-13T22:25:56Z

@dotnet/dotnet-cli Gentle ping reminder that this is available for review. Merge conflicts are just NETSDK Errors being added to the .resx in the time I was gone.

dsplaisted

Good progress.

I think we are missing tests for publishing and packing solutions where the PublishRelease or PackRelease properties aren't specified but there's no conflict between projects. I think currently that will fail.

To get it to work I think we may need to move the default values for the properties back into the .targets files.

src/Assets/TestProjects/TestAppWithSlnUsingPublishReleaseConflictingValues/App/App.csproj

src/Cli/dotnet/ReleasePropertyProjectLocator.cs

src/Tests/Microsoft.NET.Pack.Tests/GivenThatWeWantToPackAHelloWorldProject.cs

src/Tests/Microsoft.NET.Publish.Tests/GivenThatWeWantToPublishAHelloWorldProject.cs

Co-authored-by: Daniel Plaisted <dsplaisted@gmail.com>

…licts

…ease-default

… of TFM to avoid multi-targeting multi-target sln

nagilson · 2023-01-25T17:05:03Z

Waiting for Daniel to approve of the name but @AntonLapounov in this instance it means it will only evaluate one project in the solution for Configuration correctness instead of all of them... saving efficiency/speed, but it cannot as precisely distinguish what is an error and what is not this way. By that, I mean if you add any 8.0 project to an existing solution containing not 8.0 projects, that does not define PublishRelease, give -c, or disable the new feature, it will fail. This will be off by default but can be turned on if the performance hit of evaluating every project is not desired.

I will be updating/letting gewarren know to include this as soon as the name is agreed upon.

src/Cli/dotnet/ReleasePropertyProjectLocator.cs

src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.Publish.targets

src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.Sdk.DefaultItems.targets

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

Co-authored-by: Daniel Plaisted <dsplaisted@gmail.com>

…and don't deny custom Configurations anymore.

…/nagilson/sdk into nagilson-publish-release-default

nagilson · 2023-01-25T23:08:02Z

@dsplaisted I have responded to the feedback here. Thanks!

dsplaisted

I haven't made it through all the tests, but this is looking good so far.

src/Cli/dotnet/ReleasePropertyProjectLocator.cs

src/Tasks/Common/Resources/Strings.resx

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

src/Tests/Microsoft.NET.Pack.Tests/GivenThatWeWantToPackAHelloWorldProject.cs

src/Tests/Microsoft.NET.Publish.Tests/GivenThatWeWantToPublishAHelloWorldProject.cs

Co-authored-by: Daniel Plaisted <dsplaisted@gmail.com>

…ror message. Co-authored-by: Daniel Plaisted <dsplaisted@gmail.com>

….) not actually sure why we are doing this one Co-authored-by: Daniel Plaisted <dsplaisted@gmail.com>

Co-authored-by: Daniel Plaisted <dsplaisted@gmail.com>

....Publish.Tests/GivenThatWeWantToTestAMultitargetedSolutionWithPublishReleaseOrPackRelease.cs

src/Tests/Microsoft.NET.TestFramework/ProjectConstruction/TestSolution.cs

....Publish.Tests/GivenThatWeWantToTestAMultitargetedSolutionWithPublishReleaseOrPackRelease.cs

Co-authored-by: Daniel Plaisted <dsplaisted@gmail.com>

…/nagilson/sdk into nagilson-publish-release-default

…ferent than win

dsplaisted · 2023-01-30T17:14:54Z

🎉

First attempt at implementing default release behavior

fef73a3

dotnet-issue-labeler bot added the Area-Infrastructure label Nov 22, 2022

nagilson added 5 commits November 22, 2022 14:45

Basic case is working.

b897115

Add test for failing if conflicting (need asset) AND fix logic

1657da1

Improve test quality

0d5d088

Update the name of some publish release and pack release related strings

0ed140f

Move the pack error to a place where it'll actually trigger and fix r…

d67c269

…emaining p*release tests.

nagilson commented Nov 29, 2022

View reviewed changes

nagilson added 3 commits November 29, 2022 11:32

Remove the publishsinglefileError that I somehow added -- don't know …

f2d81f0

…how that got there

Make tests that demonstrate the new default behavior

de55192

Add test for pack release multi-targeting. Publishing with multi-targ…

0b7520e

…eting has been broken since at least net 5.0 so not doing that.

nagilson marked this pull request as ready for review November 29, 2022 21:46

nagilson requested review from AntonLapounov and a team as code owners November 29, 2022 21:46

nagilson commented Nov 29, 2022

View reviewed changes

src/Cli/dotnet/ReleasePropertyProjectLocator.cs Outdated Show resolved Hide resolved

dsplaisted reviewed Dec 15, 2022

View reviewed changes

nagilson and others added 13 commits December 15, 2022 09:55

Use simple identifier for the test

954ce09

Co-authored-by: Daniel Plaisted <dsplaisted@gmail.com>

Use specialized asserts instead of asset.true

d0dcfa1

Co-authored-by: Daniel Plaisted <dsplaisted@gmail.com>

Simplify a pack release test identifier

e7d07d2

Co-authored-by: Daniel Plaisted <dsplaisted@gmail.com>

use path.combine instead of string concat in pack release test

8be1cbd

Co-authored-by: Daniel Plaisted <dsplaisted@gmail.com>

Respond to some basic pr feedback about tests and tmp files

9e6e28e

Respond to all of the 'nits' before the more substantive issues

390f985

Consider multi-targeted projects with inherit implicit p*release conf…

890169f

…licts

[WIP] Add test infra code to create test solutions

be3728c

Clean-up test code regarding TestSolution.cs

7d897c7

Merge remote-tracking branch 'primary/main' into nagilson-publish-rel…

65e819c

…ease-default

Merge with main

cc99dc5

Fail even if P*Release not defined, and enable PackRelease regardless…

e2616d8

… of TFM to avoid multi-targeting multi-target sln

[WIP] check the target framework in a better way

b303257

dsplaisted reviewed Jan 25, 2023

View reviewed changes

nagilson and others added 3 commits January 25, 2023 14:53

Improve comment clarity by removing TFM and making it TargetFramework

2523195

Co-authored-by: Daniel Plaisted <dsplaisted@gmail.com>

Respond to PR feedback -> improve comments regarding PublishRelease, …

9803137

…and don't deny custom Configurations anymore.

Merge branch 'nagilson-publish-release-default' of https://github.com…

168a0dd

…/nagilson/sdk into nagilson-publish-release-default

dsplaisted reviewed Jan 26, 2023

View reviewed changes

nagilson and others added 9 commits January 26, 2023 10:35

Make a comment point to what MSBuild code it is relying oon

47a78ee

Co-authored-by: Daniel Plaisted <dsplaisted@gmail.com>

Use Assert.Equal instead of Assert.True ==

693546c

Co-authored-by: Daniel Plaisted <dsplaisted@gmail.com>

Use Path.Combine instead of str + str

af4eb20

Co-authored-by: Daniel Plaisted <dsplaisted@gmail.com>

Remove the "v" that was supposed to be a down arrow in the comment

ebf7fea

Co-authored-by: Daniel Plaisted <dsplaisted@gmail.com>

Update parameter name to reflect the pass by reference default of C#

fda2de8

Co-authored-by: Daniel Plaisted <dsplaisted@gmail.com>

Be clear that Configuration > PublishRelease or PackRelease in the er…

d057474

…ror message. Co-authored-by: Daniel Plaisted <dsplaisted@gmail.com>

Check that FileInfo exists instead of using Assert.True(File.Exists..…

66e4967

….) not actually sure why we are doing this one Co-authored-by: Daniel Plaisted <dsplaisted@gmail.com>

Do FileInfo asserts because they give the path if they fail

a353113

Co-authored-by: Daniel Plaisted <dsplaisted@gmail.com>

Fix a small typo in the test name

51e4385

Co-authored-by: Daniel Plaisted <dsplaisted@gmail.com>

dsplaisted approved these changes Jan 26, 2023

View reviewed changes

nagilson and others added 6 commits January 26, 2023 15:54

Use Tuples with named elements instead of a Tuple<K, V>

951e941

Co-authored-by: Daniel Plaisted <dsplaisted@gmail.com>

Use named tuples over Tuple<>

0c7d76f

Co-authored-by: Daniel Plaisted <dsplaisted@gmail.com>

Respond to PR feedback to improve code quality

26e5f57

Merge branch 'nagilson-publish-release-default' of https://github.com…

71902de

…/nagilson/sdk into nagilson-publish-release-default

Update XLF

905b44b

Respond to PR feedback to improve test code quality for publishrelease

3d70802

nagilson enabled auto-merge January 27, 2023 18:20

Remove the / in a test that would fail because paths in linux are dif…

d6e4e38

…ferent than win

nagilson merged commit 7f685ff into dotnet:main Jan 27, 2023

marcpopMSFT mentioned this pull request Feb 2, 2023

[NETSDKE2E]With .NET 8 SDK installed, the default publish path is "/bin/Release/..." not "/bin/Debug/..." when running "dotnet publish" in CLI #30245

Closed

nagilson mentioned this pull request Mar 2, 2023

dotnet publish should default to release mode #10357

Closed

eerhardt mentioned this pull request Mar 30, 2023

Adds TodosApi app (native AOT "goldilocks" stage 2) aspnet/Benchmarks#1828

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make `dotnet publish` use `Release` by default in 8.0+ Projects #29155

Make `dotnet publish` use `Release` by default in 8.0+ Projects #29155

nagilson commented Nov 22, 2022 •

edited

nagilson commented Dec 13, 2022

dsplaisted left a comment

nagilson commented Jan 25, 2023 •

edited

nagilson commented Jan 25, 2023

dsplaisted left a comment

dsplaisted commented Jan 30, 2023

Make dotnet publish use Release by default in 8.0+ Projects #29155

Make dotnet publish use Release by default in 8.0+ Projects #29155

Conversation

nagilson commented Nov 22, 2022 • edited

Changes:

nagilson commented Dec 13, 2022

dsplaisted left a comment

Choose a reason for hiding this comment

nagilson commented Jan 25, 2023 • edited

nagilson commented Jan 25, 2023

dsplaisted left a comment

Choose a reason for hiding this comment

dsplaisted commented Jan 30, 2023

Make `dotnet publish` use `Release` by default in 8.0+ Projects #29155

Make `dotnet publish` use `Release` by default in 8.0+ Projects #29155

nagilson commented Nov 22, 2022 •

edited

nagilson commented Jan 25, 2023 •

edited