Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
74 commits
Select commit Hold shift + click to select a range
fef73a3
First attempt at implementing default release behavior
nagilson Nov 22, 2022
b897115
Basic case is working.
nagilson Nov 22, 2022
1657da1
Add test for failing if conflicting (need asset) AND fix logic
nagilson Nov 24, 2022
0d5d088
Improve test quality
nagilson Nov 28, 2022
0ed140f
Update the name of some publish release and pack release related strings
nagilson Nov 29, 2022
d67c269
Move the pack error to a place where it'll actually trigger and fix r…
nagilson Nov 29, 2022
f2d81f0
Remove the publishsinglefileError that I somehow added -- don't know …
nagilson Nov 29, 2022
de55192
Make tests that demonstrate the new default behavior
nagilson Nov 29, 2022
0b7520e
Add test for pack release multi-targeting. Publishing with multi-targ…
nagilson Nov 29, 2022
954ce09
Use simple identifier for the test
nagilson Dec 15, 2022
d0dcfa1
Use specialized asserts instead of asset.true
nagilson Dec 15, 2022
e7d07d2
Simplify a pack release test identifier
nagilson Dec 15, 2022
8be1cbd
use path.combine instead of string concat in pack release test
nagilson Dec 15, 2022
9e6e28e
Respond to some basic pr feedback about tests and tmp files
nagilson Dec 15, 2022
390f985
Respond to all of the 'nits' before the more substantive issues
nagilson Dec 15, 2022
890169f
Consider multi-targeted projects with inherit implicit p*release conf…
nagilson Dec 19, 2022
be3728c
[WIP] Add test infra code to create test solutions
nagilson Dec 20, 2022
7d897c7
Clean-up test code regarding TestSolution.cs
nagilson Dec 20, 2022
65e819c
Merge remote-tracking branch 'primary/main' into nagilson-publish-rel…
nagilson Dec 20, 2022
cc99dc5
Merge with main
nagilson Dec 20, 2022
e2616d8
Fail even if P*Release not defined, and enable PackRelease regardless…
nagilson Jan 11, 2023
b303257
[WIP] check the target framework in a better way
nagilson Jan 11, 2023
cb7f0b1
Move PublishRelease default to a new area so it can be picked up in p…
nagilson Jan 12, 2023
983e49e
Update solution tests to use correct identifier syntax
nagilson Jan 12, 2023
09e53a5
Merge branch 'nagilson-publish-release-default' of https://github.com…
nagilson Jan 12, 2023
12557b8
Do PublishRelease error only if IsPublishing so we don't fail VS publish
nagilson Jan 12, 2023
d140c91
Make it so the P*Release errors dont occur if env-var opt out enabled.
nagilson Jan 12, 2023
aac4107
Cleanup code and add the names of a TON of edge cases in tests with m…
nagilson Jan 12, 2023
025aec4
Make the code build before finishing tests
nagilson Jan 12, 2023
57844da
Merge remote-tracking branch 'primary/main' into nagilson-publish-rel…
nagilson Jan 12, 2023
2f0fcc2
update xlf post merge of PR and ILLink error conflict
nagilson Jan 12, 2023
e36427a
Update tests and fix bug where the solution arbitrary project not def…
nagilson Jan 13, 2023
98fa0e6
Consider that the filepath of project is not a fullpath
nagilson Jan 13, 2023
3a3d4a0
[WIP] Fix some of the newly added PublishRelease tests
nagilson Jan 14, 2023
2f7c1d3
Merge branch 'main' into nagilson-publish-release-default
nagilson Jan 17, 2023
1bb42a1
.WithTargetframeworks uses Targetframeworks instead of TargetFramewor…
nagilson Jan 17, 2023
466b909
Fix a bug I added trying to simplify code where publish command parse…
nagilson Jan 17, 2023
ec10854
CONSIDER: 1 Project Turns Into 9 Evaluations With Pack Multitarget ->…
nagilson Jan 17, 2023
ccda58c
Make publish tests with Debug hardcoded use Release. Code cleanup.
nagilson Jan 18, 2023
0638edc
Fix some more tests expecting publish debug
nagilson Jan 18, 2023
44b2397
Clean-up publishrelease default code again before review, anticipatin…
nagilson Jan 18, 2023
1fc9949
Make error message not say use Directory.Build.props because it doesn…
nagilson Jan 18, 2023
b3a342e
Remove quotes from error message
nagilson Jan 18, 2023
eb32cf3
UCR test to use release config instead of debug
nagilson Jan 18, 2023
bfd1f90
PublishRelease only affects publish test change
nagilson Jan 18, 2023
80b58d5
Fix solution handling code to not undefine properties if not set
nagilson Jan 18, 2023
0447d59
Fix some crappy code in .net tool pack test per @dsplaisted where mul…
nagilson Jan 19, 2023
f3fc8d6
Revert back to the old error message
nagilson Jan 19, 2023
c301b25
[!] Change the design to be slower but break less customers.
nagilson Jan 19, 2023
66b69bb
Update one remaining test to reflect new change that fails less often
nagilson Jan 19, 2023
be4ef83
Respond to PR feedback and add the slow and fast way as a flag
nagilson Jan 24, 2023
90ed485
Merge remote-tracking branch 'primary/main' into nagilson-publish-rel…
nagilson Jan 24, 2023
baa0905
Remove test with the old environment variable idea.
nagilson Jan 24, 2023
d94f360
Remove the test solution files and update tests
nagilson Jan 24, 2023
59abc25
Clean up remaining tests with removed solution file gunk code that ca…
nagilson Jan 24, 2023
2523195
Improve comment clarity by removing TFM and making it TargetFramework
nagilson Jan 25, 2023
9803137
Respond to PR feedback -> improve comments regarding PublishRelease, …
nagilson Jan 25, 2023
168a0dd
Merge branch 'nagilson-publish-release-default' of https://github.com…
nagilson Jan 25, 2023
47a78ee
Make a comment point to what MSBuild code it is relying oon
nagilson Jan 26, 2023
693546c
Use Assert.Equal instead of Assert.True ==
nagilson Jan 26, 2023
af4eb20
Use Path.Combine instead of str + str
nagilson Jan 26, 2023
ebf7fea
Remove the "v" that was supposed to be a down arrow in the comment
nagilson Jan 26, 2023
fda2de8
Update parameter name to reflect the pass by reference default of C#
nagilson Jan 26, 2023
d057474
Be clear that Configuration > PublishRelease or PackRelease in the er…
nagilson Jan 26, 2023
66e4967
Check that FileInfo exists instead of using Assert.True(File.Exists..…
nagilson Jan 26, 2023
a353113
Do FileInfo asserts because they give the path if they fail
nagilson Jan 26, 2023
51e4385
Fix a small typo in the test name
nagilson Jan 26, 2023
951e941
Use Tuples with named elements instead of a Tuple<K, V>
nagilson Jan 26, 2023
0c7d76f
Use named tuples over Tuple<>
nagilson Jan 26, 2023
26e5f57
Respond to PR feedback to improve code quality
nagilson Jan 27, 2023
71902de
Merge branch 'nagilson-publish-release-default' of https://github.com…
nagilson Jan 27, 2023
905b44b
Update XLF
nagilson Jan 27, 2023
3d70802
Respond to PR feedback to improve test code quality for publishrelease
nagilson Jan 27, 2023
d6e4e38
Remove the / in a test that would fail because paths in linux are dif…
nagilson Jan 27, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
CONSIDER: 1 Project Turns Into 9 Evaluations With Pack Multitarget ->…
… Move PackRelease Default to Sdk.Props to be correct in this case
  • Loading branch information
nagilson committed Jan 17, 2023
commit ec10854af887937b4dfcbb3208cf40bb4e912c5e
8 changes: 3 additions & 5 deletions src/Cli/dotnet/ReleasePropertyProjectLocator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,8 @@ public struct DependentCommandOptions
public string? FrameworkOption;
public string? ConfigurationOption;

public DependentCommandOptions(IEnumerable<string> slnOrProjectArgs, string? frameworkOption = null, string? configOption = null)
=> (SlnOrProjectArgs, FrameworkOption, ConfigurationOption) = (slnOrProjectArgs, frameworkOption, configOption);
public DependentCommandOptions(IEnumerable<string> slnOrProjectArgs, string? configOption = null, string? frameworkOption = null)
=> (SlnOrProjectArgs, ConfigurationOption, FrameworkOption) = (slnOrProjectArgs, configOption, frameworkOption);
}


Expand Down Expand Up @@ -230,11 +230,9 @@ private Dictionary<string, string> InjectTargetFrameworkIntoGlobalProperties(Dic
{
if (_options.FrameworkOption != null)
{
string givenFrameworkOption = _parseResult.GetValue(_options.FrameworkOption);

// Note: dotnet -f FRAMEWORK_1 --property:TargetFramework=FRAMEWORK_2 will use FRAMEWORK_1.
// So we can replace the value in the globals non-dubiously if it exists.
oldGlobalProperties[MSBuildPropertyNames.TARGET_FRAMEWORK] = givenFrameworkOption;
oldGlobalProperties[MSBuildPropertyNames.TARGET_FRAMEWORK] = _options.FrameworkOption;
}
return oldGlobalProperties;
}
Expand Down
3 changes: 3 additions & 0 deletions src/Cli/dotnet/commands/dotnet-pack/PackCommand.cs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
using System.CommandLine.Parsing;
using Microsoft.DotNet.Tools.Publish;
using System.Linq;
using System.Diagnostics;

namespace Microsoft.DotNet.Tools.Pack
{
Expand Down Expand Up @@ -44,6 +45,8 @@ public static PackCommand FromParseResult(ParseResult parseResult, string msbuil

msbuildArgs.AddRange(parseResult.OptionValuesToBeForwarded(PackCommandParser.GetCommand()));

Debugger.Launch();

ReleasePropertyProjectLocator projectLocator = new ReleasePropertyProjectLocator(parseResult, MSBuildPropertyNames.PACK_RELEASE,
new ReleasePropertyProjectLocator.DependentCommandOptions(
parseResult.GetValue(PackCommandParser.SlnOrProjectArgument),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ Copyright (c) .NET Foundation. All rights reserved.

<PropertyGroup>
<_TargetFrameworkVersionWithoutV>$(TargetFrameworkVersion.TrimStart('vV'))</_TargetFrameworkVersionWithoutV>

<MicrosoftNETBuildExtensionsTasksAssembly Condition="'$(MicrosoftNETBuildExtensionsTasksAssembly)' == '' AND '$(MSBuildRuntimeType)' == 'Core'">$(MSBuildThisFileDirectory)\tools\net8.0\Microsoft.NET.Build.Extensions.Tasks.dll</MicrosoftNETBuildExtensionsTasksAssembly>
<MicrosoftNETBuildExtensionsTasksAssembly Condition="'$(MicrosoftNETBuildExtensionsTasksAssembly)' == ''">$(MSBuildThisFileDirectory)\tools\net472\Microsoft.NET.Build.Extensions.Tasks.dll</MicrosoftNETBuildExtensionsTasksAssembly>

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,17 @@ Copyright (c) .NET Foundation. All rights reserved.
<_TargetFrameworkVersionWithoutV>$(TargetFrameworkVersion.TrimStart('vV'))</_TargetFrameworkVersionWithoutV>
</PropertyGroup>

<!-- Evaluate the two below properties so they are available early enough to be defined in any ProjectInstance evaluation. -->
<!-- Default the value of PackRelease for every project. Note that this is pre-evaluated by the CLI in an evaluation before evaluation (see ReleasePropertyProjectLocator.cs).-->
<PropertyGroup Condition="'$(PackRelease)' == ''">
<PackRelease>true</PackRelease>
</PropertyGroup>

<!-- Default the value of PublishRelease for 8.0+ TFMs. Note that this is pre-evaluated by the CLI in an evaluation before evaluation (see ReleasePropertyProjectLocator.cs).-->
<PropertyGroup Condition="'$(PublishRelease)' == '' and '$(_TargetFrameworkVersionWithoutV)' >= '8.0'">
<PublishRelease>true</PublishRelease>
</PropertyGroup>

<!--
Import targets from RazorSDK if referenced
Sdks\Microsoft.NET.Sdk\targets\Microsoft.NET.Sdk.BeforeCommon.targets
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -153,16 +153,11 @@ Copyright (c) .NET Foundation. All rights reserved.
<NETSdkWarning ResourceName="UnnecessaryWindowsDesktopSDK" />
</Target>

<!-- Default the value of PackRelease for every project. Note that this is pre-evaluated by the CLI in an evaluation before evaluation (see ReleasePropertyProjectLocator.cs).-->
<PropertyGroup Condition="'$(PackRelease)' == ''">
<PackRelease>true</PackRelease>
</PropertyGroup>

<!-- Default the value of PublishRelease for 8.0+ TFMs. Note that this is pre-evaluated by the CLI in an evaluation before evaluation (see ReleasePropertyProjectLocator.cs).-->
<PropertyGroup Condition="'$(PublishRelease)' == '' and '$(_TargetFrameworkVersionWithoutV)' >= '8.0'">
<PublishRelease>true</PublishRelease>
</PropertyGroup>

<!-- This verifies that the configuration between projects in a solution set by PackRelease agree.
PackRelease only affects the CLI since VS has its own Configuration UI and it's not possible to detect like this in MSBuild.
_IsPacking will only be set if packing in the CLI, so that's fine here. The pack targets are all outside of the SDK, which is why this is here.-->
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,14 @@ Copyright (c) .NET Foundation. All rights reserved.
<Platform Condition=" '$(Platform)' == '' ">AnyCPU</Platform>
</PropertyGroup>

<!-- Default the value of PackRelease for every project. Note that this is pre-evaluated by the CLI in an evaluation before evaluation (see ReleasePropertyProjectLocator.cs).
Multitargeting pack splits a single 2 TFM project into 9 separate evaluations, with different target imports. Microsoft.NET.SDK.DefaultItems.Targets ...
is where PublishRelease is defaulted because it depends on _TargetFrameworkVersionWithoutV. Most targets do not run in some instances of dotnet pack.
So, we must set PackRelease here. Even though this occurs before project import, the project import can override PackRelease later, before the pre-evaluation, so this is fine.-->
<PropertyGroup Condition="'$(PackRelease)' == ''">
<PackRelease>true</PackRelease>
</PropertyGroup>

<!-- User-facing configuration-agnostic defaults -->
<PropertyGroup>
<OutputType Condition=" '$(OutputType)' == '' ">Library</OutputType>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -111,8 +111,10 @@ public void It_packs_with_release_if_PackRelease_property_set(bool optedOut)
Assert.True(File.Exists(expectedAssetPath));
}

[Fact]
public void It_packs_with_release_if_PackRelease_property_set_in_csproj()
[Theory]
[InlineData("true")]
[InlineData("false")]
public void It_packs_with_release_if_PackRelease_property_set_in_csproj(string valueOfPackRelease)
{
var helloWorldAsset = _testAssetsManager
.CopyTestAsset("HelloWorld")
Expand All @@ -121,7 +123,7 @@ public void It_packs_with_release_if_PackRelease_property_set_in_csproj()
{
var ns = project.Root.Name.Namespace;
var propertyGroup = project.Root.Elements(ns + "PropertyGroup").First();
propertyGroup.Add(new XElement(ns + "PackRelease", "true"));
propertyGroup.Add(new XElement(ns + "PackRelease", valueOfPackRelease));
});

var packCommand = new DotnetPackCommand(Log, helloWorldAsset.TestRoot);
Expand All @@ -131,30 +133,34 @@ public void It_packs_with_release_if_PackRelease_property_set_in_csproj()
.Should()
.Pass();

var expectedAssetPath = Path.Combine(helloWorldAsset.Path, "bin", "Release", "HelloWorld.1.0.0.nupkg");
var expectedAssetPath = Path.Combine(helloWorldAsset.Path, "bin", valueOfPackRelease == "true" ? "Release" : "Debug", "HelloWorld.1.0.0.nupkg");
Assert.True(File.Exists(expectedAssetPath));
}

[InlineData("")]
[InlineData("false")]
[Theory]
public void It_packs_successfully_with_debug_with_Mutlitargeting_where_net_8_and_net_7_project_defines_PackRelease_or_not(string packReleaseValue)
public void It_packs_successfully_with_Mutlitargeting_where_net_8_and_net_7_project_defines_PackRelease_or_not(string packReleaseValue)
{
var helloWorldAsset = _testAssetsManager
.CopyTestAsset("HelloWorld")
.CopyTestAsset("HelloWorld", identifier: packReleaseValue)
.WithSource()
.WithTargetFrameworks("net8.0;net7.0")
.WithProjectChanges(project =>
{
var ns = project.Root.Name.Namespace;
var propertyGroup = project.Root.Elements(ns + "PropertyGroup").First();
propertyGroup.Add(new XElement(packReleaseValue != "" ? ns + "PackRelease" + packReleaseValue : ""));
propertyGroup.Add(packReleaseValue != "" ?
new XElement(ns + "PackRelease", packReleaseValue)
:
new XElement(ns + "Foo", packReleaseValue)
);
});

var packCommand = new DotnetPackCommand(Log, helloWorldAsset.TestRoot);

packCommand
.Execute()
.Execute("-bl:C:\\users\\noahgilson\\whydoesthispackdebug.binlog")
.Should()
.Pass();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,16 +50,17 @@ internal Tuple<TestSolution, List<TestProject>> Setup(ITestOutputHelper log, Lis
IsExe = true
};
testProject.RecordProperties("Configuration", "Optimize", PReleaseProperty);
var mainProject = _testAssetsManager.CreateTestProject(testProject, callingMethod: testPath, identifier: PReleaseProperty);
testProject.AdditionalProperties[PReleaseProperty] = firstProjPReleaseValue;
var mainProject = _testAssetsManager.CreateTestProject(testProject, callingMethod: testPath, identifier: string.Join("", firstProjTfms) + PReleaseProperty);

var referencedProject = new TestProject("ReferencedProject")
{
TargetFrameworks = String.Join(";", secondProjTfms),
IsExe = false
};
referencedProject.RecordProperties("Configuration", "Optimize", PReleaseProperty);
testProject.AdditionalProperties[PReleaseProperty] = secondProjPReleaseValue;
var secondProject = _testAssetsManager.CreateTestProject(referencedProject, callingMethod: testPath, identifier: PReleaseProperty);
referencedProject.AdditionalProperties[PReleaseProperty] = secondProjPReleaseValue;
var secondProject = _testAssetsManager.CreateTestProject(referencedProject, callingMethod: testPath, identifier: string.Join("", secondProjTfms) + PReleaseProperty);

List<TestAsset> projects = new List<TestAsset> { mainProject, secondProject };

Expand All @@ -74,7 +75,6 @@ internal Tuple<TestSolution, List<TestProject>> Setup(ITestOutputHelper log, Lis

[InlineData(PublishRelease, "net8.0", true)]
[InlineData(PublishRelease, "-p:TargetFramework=net8.0", false)]
[InlineData(PackRelease, "-p:TargetFramework=net8.0", false)]
[Theory]
public void ItUsesReleaseWithATargetFrameworkOptionNet8ForNet6AndNet7MultitargetingProjectWithPReleaseUndefined(string releaseProperty, string args, bool passDashF)
{
Expand All @@ -87,7 +87,7 @@ public void ItUsesReleaseWithATargetFrameworkOptionNet8ForNet6AndNet7Multitarget

var dotnetCommand = new DotnetCommand(Log, releaseProperty == PublishRelease ? publish : pack);
dotnetCommand
.Execute(passDashF ? "-f" : "", args, sln.SolutionPath, "-bl:C:\\users\\noahgilson\\releaseframework.binlog")
.Execute(passDashF ? "-f" : "", args, sln.SolutionPath)
.Should()
.Pass();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -455,19 +455,12 @@ public void A_PublishRelease_property_does_not_override_other_command_configurat

System.IO.File.WriteAllText(helloWorldAsset.Path + "/Directory.Build.props", "<Project><PropertyGroup><PublishRelease>true</PublishRelease></PropertyGroup></Project>");

// Another command, which should not be affected by PublishRelease
new BuildCommand(helloWorldAsset)
.Execute()
.Should()
.Pass();

// Another command, which should not be affected by PublishRelease
var packCommand = new DotnetPackCommand(Log, helloWorldAsset.TestRoot);

packCommand
.Execute()
.Should()
.Pass();

var expectedAssetPath = System.IO.Path.Combine(helloWorldAsset.Path, "bin", "Release", "HelloWorld.1.0.0.nupkg");
Assert.False(File.Exists(expectedAssetPath));
}
Expand Down