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.

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

Include user specified MSBuild parameters when evaluating MSBuild properties #352

Merged
merged 2 commits into from
Dec 16, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
11 changes: 11 additions & 0 deletions .autover/changes/79b678d7-6533-4e17-ae69-100730e43027.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
{
"Projects": [
{
"Name": "Amazon.Lambda.Tools",
"Type": "Patch",
"ChangelogMessages": [
"Include user specified MSBuild parameters when evaluating MSBuild properties"
]
}
]
}
24 changes: 16 additions & 8 deletions src/Amazon.Common.DotNetCli.Tools/Utilities.cs
Original file line number Diff line number Diff line change
Expand Up @@ -211,9 +211,10 @@ public static string DeterminePublishLocation(string workingDirectory, string pr
/// Looks up specified properties from a project.
/// </summary>
/// <param name="projectLocation">The location of the project file.</param>
/// <param name="msBuildParameters">Additional MSBuild parameters passed by the user from the commandline</param>
/// <param name="propertyNames">The names of the properties to look up.</param>
/// <returns>A dictionary of property names and their values.</returns>
public static Dictionary<string, string> LookupProjectProperties(string projectLocation, params string[] propertyNames)
public static Dictionary<string, string> LookupProjectProperties(string projectLocation, string msBuildParameters, params string[] propertyNames)
{
var projectFile = FindProjectFileInDirectory(projectLocation);
var properties = new Dictionary<string, string>();
Expand All @@ -225,6 +226,11 @@ public static Dictionary<string, string> LookupProjectProperties(string projectL
$"--getProperty:{string.Join(',', propertyNames)}"
};

if (!string.IsNullOrEmpty(msBuildParameters))
{
arguments.Add(msBuildParameters);
}

var process = new Process
{
StartInfo = new ProcessStartInfo
Expand Down Expand Up @@ -302,16 +308,17 @@ private static Dictionary<string, string> LookupProjectPropertiesFromXml(string
{
}
return properties;
}
}

/// <summary>
/// Looks up the target framework from a project file.
/// </summary>
/// <param name="projectLocation">The location of the project file.</param>
/// <param name="msBuildParameters">Additonal MSBuild paramteres passed by the user from the commandline</param>
/// <returns>The target framework of the project.</returns>
public static string LookupTargetFrameworkFromProjectFile(string projectLocation)
public static string LookupTargetFrameworkFromProjectFile(string projectLocation, string msBuildParameters)
{
var properties = LookupProjectProperties(projectLocation, "TargetFramework", "TargetFrameworks");
var properties = LookupProjectProperties(projectLocation, msBuildParameters, "TargetFramework", "TargetFrameworks");
if (properties.TryGetValue("TargetFramework", out var targetFramework) && !string.IsNullOrEmpty(targetFramework))
{
return targetFramework;
Expand All @@ -331,10 +338,11 @@ public static string LookupTargetFrameworkFromProjectFile(string projectLocation
/// Retrieve the `OutputType` property of a given project
/// </summary>
/// <param name="projectLocation">Path of the project</param>
/// <param name="msBuildParameters">Additonal MSBuild paramteres passed by the user from the commandline</param>
/// <returns>The value of the `OutputType` property</returns>
public static string LookupOutputTypeFromProjectFile(string projectLocation)
public static string LookupOutputTypeFromProjectFile(string projectLocation, string msBuildParameters)
{
var properties = LookupProjectProperties(projectLocation, "OutputType");
var properties = LookupProjectProperties(projectLocation, msBuildParameters, "OutputType");
return properties.TryGetValue("OutputType", out var outputType) ? outputType.Trim() : null;
}

Expand All @@ -353,7 +361,7 @@ public static bool LookPublishAotFlag(string projectLocation, string msBuildPara
}
}

var properties = LookupProjectProperties(projectLocation, "PublishAot");
var properties = LookupProjectProperties(projectLocation, msBuildParameters, "PublishAot");
if (properties.TryGetValue("PublishAot", out var publishAot))
{
return bool.TryParse(publishAot, out var result) && result;
Expand All @@ -369,7 +377,7 @@ public static bool HasExplicitSelfContainedFlag(string projectLocation, string m
return true;
}

var properties = LookupProjectProperties(projectLocation, "SelfContained");
var properties = LookupProjectProperties(projectLocation, msBuildParameters, "SelfContained");
if (properties.TryGetValue("SelfContained", out var selfContained))
{
return bool.TryParse(selfContained, out var isSelfContained) && isSelfContained;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ protected override async Task<bool> PerformActionAsync()

if (string.IsNullOrEmpty(targetFramework))
{
targetFramework = Utilities.LookupTargetFrameworkFromProjectFile(projectLocation);
targetFramework = Utilities.LookupTargetFrameworkFromProjectFile(projectLocation, null);
if (string.IsNullOrEmpty(targetFramework))
{
targetFramework = this.GetStringValueOrDefault(this.DeployEnvironmentOptions.TargetFramework, CommonDefinedCommandOptions.ARGUMENT_FRAMEWORK, true);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ protected override Task<bool> PerformActionAsync()

if (string.IsNullOrEmpty(targetFramework))
{
targetFramework = Utilities.LookupTargetFrameworkFromProjectFile(projectLocation);
targetFramework = Utilities.LookupTargetFrameworkFromProjectFile(projectLocation, null);
if (string.IsNullOrEmpty(targetFramework))
{
targetFramework = this.GetStringValueOrDefault(this.DeployEnvironmentOptions.TargetFramework, CommonDefinedCommandOptions.ARGUMENT_FRAMEWORK, true);
Expand Down
4 changes: 2 additions & 2 deletions src/Amazon.Lambda.Tools/Commands/DeployFunctionCommand.cs
Original file line number Diff line number Diff line change
Expand Up @@ -227,10 +227,11 @@ protected override async Task<bool> PerformActionAsync()
// Release will be the default configuration if nothing set.
string configuration = this.GetStringValueOrDefault(this.Configuration, CommonDefinedCommandOptions.ARGUMENT_CONFIGURATION, false);

string msbuildParameters = this.GetStringValueOrDefault(this.MSBuildParameters, CommonDefinedCommandOptions.ARGUMENT_MSBUILD_PARAMETERS, false);
var targetFramework = this.GetStringValueOrDefault(this.TargetFramework, CommonDefinedCommandOptions.ARGUMENT_FRAMEWORK, false);
if (string.IsNullOrEmpty(targetFramework))
{
targetFramework = Utilities.LookupTargetFrameworkFromProjectFile(projectLocation);
targetFramework = Utilities.LookupTargetFrameworkFromProjectFile(projectLocation, msbuildParameters);

// If we still don't know what the target framework is ask the user what targetframework to use.
// This is common when a project is using multi targeting.
Expand All @@ -239,7 +240,6 @@ protected override async Task<bool> PerformActionAsync()
targetFramework = this.GetStringValueOrDefault(this.TargetFramework, CommonDefinedCommandOptions.ARGUMENT_FRAMEWORK, true);
}
}
string msbuildParameters = this.GetStringValueOrDefault(this.MSBuildParameters, CommonDefinedCommandOptions.ARGUMENT_MSBUILD_PARAMETERS, false);

bool isNativeAot = Utilities.LookPublishAotFlag(projectLocation, this.MSBuildParameters);

Expand Down
4 changes: 2 additions & 2 deletions src/Amazon.Lambda.Tools/Commands/PackageCommand.cs
Original file line number Diff line number Diff line change
Expand Up @@ -206,10 +206,11 @@ protected override async Task<bool> PerformActionAsync()
// Release will be the default configuration if nothing set.
var configuration = this.GetStringValueOrDefault(this.Configuration, CommonDefinedCommandOptions.ARGUMENT_CONFIGURATION, false);

var msbuildParameters = this.GetStringValueOrDefault(this.MSBuildParameters, CommonDefinedCommandOptions.ARGUMENT_MSBUILD_PARAMETERS, false);
var targetFramework = this.GetStringValueOrDefault(this.TargetFramework, CommonDefinedCommandOptions.ARGUMENT_FRAMEWORK, false);
if (string.IsNullOrEmpty(targetFramework))
{
targetFramework = Utilities.LookupTargetFrameworkFromProjectFile(projectLocation);
targetFramework = Utilities.LookupTargetFrameworkFromProjectFile(projectLocation, msbuildParameters);

// If we still don't know what the target framework is ask the user what targetframework to use.
// This is common when a project is using multi targeting.
Expand All @@ -221,7 +222,6 @@ protected override async Task<bool> PerformActionAsync()

bool isNativeAot = Utilities.LookPublishAotFlag(projectLocation, this.MSBuildParameters);

var msbuildParameters = this.GetStringValueOrDefault(this.MSBuildParameters, CommonDefinedCommandOptions.ARGUMENT_MSBUILD_PARAMETERS, false);
var architecture = this.GetStringValueOrDefault(this.Architecture, LambdaDefinedCommandOptions.ARGUMENT_FUNCTION_ARCHITECTURE, false);
var disableVersionCheck = this.GetBoolValueOrDefault(this.DisableVersionCheck, LambdaDefinedCommandOptions.ARGUMENT_DISABLE_VERSION_CHECK, false).GetValueOrDefault();

Expand Down
2 changes: 1 addition & 1 deletion src/Amazon.Lambda.Tools/LambdaPackager.cs
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ public static bool CreateApplicationBundle(LambdaToolsDefaults defaults, IToolLo
bool? useContainerForBuild, string containerImageForBuild, string codeMountDirectory,
out string publishLocation, ref string zipArchivePath)
{
LambdaUtilities.ValidateTargetFramework(projectLocation, targetFramework, isNativeAot);
LambdaUtilities.ValidateTargetFramework(projectLocation, msbuildParameters, targetFramework, isNativeAot);

LambdaUtilities.ValidateNativeAotArchitecture(architecture, isNativeAot);

Expand Down
8 changes: 4 additions & 4 deletions src/Amazon.Lambda.Tools/LambdaUtilities.cs
Original file line number Diff line number Diff line change
Expand Up @@ -64,13 +64,13 @@ public static class LambdaUtilities
{Amazon.Lambda.Runtime.Dotnetcore10.Value, TargetFrameworkMonikers.netcoreapp10}
};

public static string DetermineTargetFrameworkFromLambdaRuntime(string lambdaRuntime, string projectLocation)
public static string DetermineTargetFrameworkFromLambdaRuntime(string lambdaRuntime, string projectLocation, string msbuildParameters)
{
string framework;
if (_lambdaRuntimeToDotnetFramework.TryGetValue(lambdaRuntime, out framework))
return framework;

framework = Utilities.LookupTargetFrameworkFromProjectFile(projectLocation);
framework = Utilities.LookupTargetFrameworkFromProjectFile(projectLocation, msbuildParameters);
return framework;
}

Expand All @@ -83,9 +83,9 @@ public static string DetermineLambdaRuntimeFromTargetFramework(string targetFram
return kvp.Key;
}

public static void ValidateTargetFramework(string projectLocation, string targetFramework, bool isNativeAot)
public static void ValidateTargetFramework(string projectLocation, string msbuildParameters, string targetFramework, bool isNativeAot)
{
var outputType = Utilities.LookupOutputTypeFromProjectFile(projectLocation);
var outputType = Utilities.LookupOutputTypeFromProjectFile(projectLocation, msbuildParameters);
var ouputTypeIsExe = outputType != null && outputType.ToLower().Equals("exe");

if (isNativeAot && !ouputTypeIsExe)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -251,7 +251,7 @@ private async Task<UpdateResourceResults> PackageDotnetProjectAsync(IUpdateResou
var outputPackage = GenerateOutputZipFilename(field);
command.OutputPackageFileName = outputPackage;
command.TargetFramework =
LambdaUtilities.DetermineTargetFrameworkFromLambdaRuntime(field.Resource.LambdaRuntime, location);
LambdaUtilities.DetermineTargetFrameworkFromLambdaRuntime(field.Resource.LambdaRuntime, location, null);

command.Architecture = field.Resource.LambdaArchitecture;
command.LayerVersionArns = field.Resource.LambdaLayers;
Expand Down
18 changes: 16 additions & 2 deletions test/Amazon.Common.DotNetCli.Tools.Test/UtilitiesTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ public void CheckFramework(string projectPath, string expectedFramework)
{
var assembly = this.GetType().GetTypeInfo().Assembly;
var fullPath = Path.GetFullPath(Path.GetDirectoryName(assembly.Location) + projectPath);
var determinedFramework = Utilities.LookupTargetFrameworkFromProjectFile(projectPath);
var determinedFramework = Utilities.LookupTargetFrameworkFromProjectFile(projectPath, null);
Assert.Equal(expectedFramework, determinedFramework);
}

Expand Down Expand Up @@ -80,7 +80,7 @@ public void TestLookForPublishAotFlag(string projectLocation, string msBuildPara
[InlineData("../../../../../testapps/TestNativeAotSingleProject", "Exe")]
public void TestLookupOutputTypeFromProjectFile(string projectLocation, string expected)
{
var result = Utilities.LookupOutputTypeFromProjectFile(projectLocation);
var result = Utilities.LookupOutputTypeFromProjectFile(projectLocation, null);

Assert.Equal(expected, result);
}
Expand All @@ -100,5 +100,19 @@ public void TestHasExplicitSelfContainedFlag(string projectLocation, string msBu

Assert.Equal(expected, result);
}

[Theory]
[InlineData("TargetFramework", "", "net6.0")]
[InlineData("TargetFramework", "/p:NonExistence=net20.0", "net6.0")]
[InlineData("TargetFramework", "/p:TargetFramework=net20.0", "net20.0")]
[InlineData("TargetFramework", "/p:TargetFramework=net20.0 /p:OutputType=FutureDevice", "net20.0")]
[InlineData("OutputType", "/p:TargetFramework=net20.0 /p:OutputType=FutureDevice", "FutureDevice")]
public void TestPropertyEvaluationWithMSBuildParameters(string property, string msbuildparameters, string expectedValue)
Copy link
Contributor

Choose a reason for hiding this comment

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

Worth adding a test case for when more that one property is added by the user?

Copy link
Member Author

Choose a reason for hiding this comment

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

I added a couple more test scenarios.

{
var projectLocation = "../../../../../testapps/TestFunction";

var value = Utilities.LookupProjectProperties(projectLocation, msbuildparameters, property)[property];
Assert.Equal(expectedValue, value);
}
}
}
4 changes: 2 additions & 2 deletions test/Amazon.Lambda.Tools.Test/UtilitiesTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -216,12 +216,12 @@ public void TestValidateTargetFramework(string projectLocation, string targetFra
{
if (shouldThrow)
{
Assert.Throws<LambdaToolsException>(() => LambdaUtilities.ValidateTargetFramework(projectLocation, targetFramework, isNativeAot));
Assert.Throws<LambdaToolsException>(() => LambdaUtilities.ValidateTargetFramework(projectLocation, null, targetFramework, isNativeAot));
}
else
{
// If this throws an exception, the test will fail, hench no assert is necessary
LambdaUtilities.ValidateTargetFramework(projectLocation, targetFramework, isNativeAot);
LambdaUtilities.ValidateTargetFramework(projectLocation, null, targetFramework, isNativeAot);
}
}

Expand Down
Loading