Skip to content
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.

Sign up for GitHub

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

Jump to bottom

GH2953: Allow setting MSBuild target via MSBuildSettings using a string #3721

Closed
wants to merge 2 commits into from
Closed

GH2953: Allow setting MSBuild target via MSBuildSettings using a string #3721

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e93ca56
Added Target as a property in MSBuildSettings initializer
sbwaggoner Nov 29, 2021
c50e60a
Merge branch 'develop' into feature-addition-target-initializer-msbui…
sbwaggoner Nov 30, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
76 changes: 73 additions & 3 deletions src/Cake.Common.Tests/Unit/Tools/MSBuild/MSBuildRunnerTests.cs
View file
Open in desktop
Expand Up @@ -745,9 +745,55 @@ public void Should_Use_No_Logo_If_Specified()
public void Should_Append_Targets_To_Process_Arguments()
{
// Given
var fixture = new MSBuildRunnerFixture(false, PlatformFamily.Windows);
fixture.Settings.WithTarget("A");
fixture.Settings.WithTarget("B");
var fixture = new MSBuildRunnerFixture(false, PlatformFamily.Windows)
{
Settings = new MSBuildSettings
{
Target = "A;B"
}
};

// When
var result = fixture.Run();

// Then
Assert.Equal("/v:normal /target:A;B " +
"\"C:/Working/src/Solution.sln\"", result.Args);
}

[Fact]
public void Should_Add_Single_Target_With_Initializer()
{
// Given
var fixture = new MSBuildRunnerFixture(false, PlatformFamily.Windows)
{
Settings = new MSBuildSettings
{
Target = "A",
ToolVersion = MSBuildToolVersion.VS2019,
}
};

// When
var result = fixture.Run();

// Then
Assert.Equal("/v:normal /target:A " +
"\"C:/Working/src/Solution.sln\"", result.Args);
}

[Fact]
public void Should_Add_Multiple_Targets_With_Initializer()
{
// Given
var fixture = new MSBuildRunnerFixture(false, PlatformFamily.Windows)
{
Settings = new MSBuildSettings
{
Target = "A;B",
ToolVersion = MSBuildToolVersion.VS2019,
}
};

// When
var result = fixture.Run();
Expand All @@ -757,6 +803,30 @@ public void Should_Append_Targets_To_Process_Arguments()
"\"C:/Working/src/Solution.sln\"", result.Args);
}

[Fact]
public void Should_Add_Multiple_Targets_With_Initializer_And_AddTarget()
{
// Given
var fixture = new MSBuildRunnerFixture(false, PlatformFamily.Windows)
{
Settings = new MSBuildSettings
{
Target = "A;B",
ToolVersion = MSBuildToolVersion.VS2019,
}
};

fixture.Settings.WithTarget("C");
fixture.Settings.WithTarget("D");

// When
var result = fixture.Run();

// Then
Assert.Equal("/v:normal /target:A;B;C;D " +
"\"C:/Working/src/Solution.sln\"", result.Args);
}

[Fact]
public void Should_Append_Property_To_Process_Arguments()
{
Expand Down
20 changes: 20 additions & 0 deletions src/Cake.Common/Tools/MSBuild/MSBuildSettings.cs
View file
Open in desktop
Expand Up @@ -4,6 +4,7 @@

using System;
using System.Collections.Generic;
using System.Linq;
using Cake.Core.Diagnostics;
using Cake.Core.Tooling;

Expand Down Expand Up @@ -47,6 +48,16 @@ public sealed class MSBuildSettings : ToolSettings
/// <value>The MSBuild platform.</value>
public MSBuildPlatform MSBuildPlatform { get; set; }

/// <summary>
/// Gets or sets the MSBuild target.
/// </summary>
/// <value>The MSBuild target.</value>
public string Target
{
get => string.Join(";", Targets);
set => SetTargets(value);
}

/// <summary>
/// Gets or sets the tool version.
/// </summary>
Expand Down Expand Up @@ -269,6 +280,7 @@ public MSBuildSettings()
Configuration = string.Empty;
Verbosity = Verbosity.Normal;
MSBuildPlatform = MSBuildPlatform.Automatic;
Target = string.Empty;
}

private string GetPropertyValueOrDefault(string propertyName, string @default = null)
Expand All @@ -281,5 +293,13 @@ private string GetPropertyValueOrDefault(string propertyName, string @default =
var propertyValue = string.Join(";", propertyValues);
return propertyValue;
}

private void SetTargets(string value)
{
foreach (var target in value.Split(";").Where(p => !string.IsNullOrEmpty(p)))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would expect something like this

Suggested change
foreach (var target in value.Split(";").Where(p => !string.IsNullOrEmpty(p)))
Targets.Clear();
foreach (var target in value?.Split(';', _targetStringSplitOptions) ?? Array.Empty<string>())

so setting Target to a new value isn't keeping the old values or what do you think @augustoproiete ?

Where is something like this on top of class

#if NETCOREAPP3_1
        private static readonly StringSplitOptions _targetStringSplitOptions = StringSplitOptions.RemoveEmptyEntries;
#else
        private static readonly StringSplitOptions _targetStringSplitOptions = StringSplitOptions.RemoveEmptyEntries | StringSplitOptions.TrimEntries;
#endif

{
Targets.Add(target);
}
}
}
}
2 changes: 1 addition & 1 deletion src/Cake.Common/Tools/MSBuild/MSBuildSettingsExtensions.cs
View file
Open in desktop
Expand Up @@ -23,7 +23,7 @@ public static class MSBuildSettingsExtensions
/// <returns>The same <see cref="MSBuildSettings"/> instance so that multiple calls can be chained.</returns>
public static MSBuildSettings WithTarget(this MSBuildSettings settings, string target)
{
if (settings == null)
if (settings is null)
{
throw new ArgumentNullException(nameof(settings));
}
Expand Down