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

Use MSBuild to find target framework #348

Merged
merged 7 commits into from
Nov 26, 2024
Merged
Show file tree
Hide file tree
Changes from 5 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/8a5204e4-83ec-45ac-8c75-27512ae9cdf8.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
{
"Projects": [
{
"Name": "Amazon.Lambda.Tools",
"Type": "Patch",
"ChangelogMessages": [
"Use MSBuild to find target framework"
Copy link
Contributor

Choose a reason for hiding this comment

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

Add more explanation here for the significance of this for customers. Not sure everyone would know what this implies.

]
}
]
}
7 changes: 7 additions & 0 deletions aws-extensions-for-dotnet-cli.sln
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,8 @@ Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "TestNativeAotNet8WebApp", "
EndProject
Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "TestIntegerFunction", "testapps\TestIntegerFunction\TestIntegerFunction.csproj", "{D7F1DFA4-066B-469C-B04C-DF032CF152C1}"
EndProject
Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "TestFunctionBuildProps", "testapps\TestFunctionBuildProps\TestFunctionBuildProps\TestFunctionBuildProps.csproj", "{AFA71B8E-F0AA-4704-8C4E-C11130F82B13}"
EndProject
Global
GlobalSection(SolutionConfigurationPlatforms) = preSolution
Debug|Any CPU = Debug|Any CPU
Expand Down Expand Up @@ -169,6 +171,10 @@ Global
{D7F1DFA4-066B-469C-B04C-DF032CF152C1}.Debug|Any CPU.Build.0 = Debug|Any CPU
{D7F1DFA4-066B-469C-B04C-DF032CF152C1}.Release|Any CPU.ActiveCfg = Release|Any CPU
{D7F1DFA4-066B-469C-B04C-DF032CF152C1}.Release|Any CPU.Build.0 = Release|Any CPU
{AFA71B8E-F0AA-4704-8C4E-C11130F82B13}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
{AFA71B8E-F0AA-4704-8C4E-C11130F82B13}.Debug|Any CPU.Build.0 = Debug|Any CPU
{AFA71B8E-F0AA-4704-8C4E-C11130F82B13}.Release|Any CPU.ActiveCfg = Release|Any CPU
{AFA71B8E-F0AA-4704-8C4E-C11130F82B13}.Release|Any CPU.Build.0 = Release|Any CPU
EndGlobalSection
GlobalSection(SolutionProperties) = preSolution
HideSolutionNode = FALSE
Expand Down Expand Up @@ -200,6 +206,7 @@ Global
{AD31D053-97C5-4262-B187-EC42BFD51A9F} = {BB3CF729-8213-4DDD-85AE-A5E7754F3944}
{69FFA03C-D29F-40E0-9E7F-572D5E10AF77} = {BB3CF729-8213-4DDD-85AE-A5E7754F3944}
{D7F1DFA4-066B-469C-B04C-DF032CF152C1} = {BB3CF729-8213-4DDD-85AE-A5E7754F3944}
{AFA71B8E-F0AA-4704-8C4E-C11130F82B13} = {BB3CF729-8213-4DDD-85AE-A5E7754F3944}
EndGlobalSection
GlobalSection(ExtensibilityGlobals) = postSolution
SolutionGuid = {DBFC70D6-49A2-40A1-AB08-5D9504AB7112}
Expand Down
175 changes: 111 additions & 64 deletions src/Amazon.Common.DotNetCli.Tools/Utilities.cs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
using System.Text.RegularExpressions;
using System.Collections;
using System.Xml;
using System.Text.Json;

namespace Amazon.Common.DotNetCli.Tools
{
Expand Down Expand Up @@ -205,72 +206,129 @@ public static string DeterminePublishLocation(string workingDirectory, string pr
return path;
}

public static string LookupTargetFrameworkFromProjectFile(string projectLocation)
{
var projectFile = FindProjectFileInDirectory(projectLocation);

var xdoc = XDocument.Load(projectFile);

var element = xdoc.XPathSelectElement("//PropertyGroup/TargetFramework");
return element?.Value;
}

/// <summary>
/// Retrieve the `OutputType` property of a given project
// <summary>
/// Looks up specified properties from a project file.
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: drop "file" since MSBUILD is not looking at the project file (.csproj) alone

/// </summary>
/// <param name="projectLocation">Path of the project</param>
/// <returns>The value of the `OutputType` property</returns>
public static string LookupOutputTypeFromProjectFile(string projectLocation)
/// <param name="projectLocation">The location of the project file.</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)
{
var process = new Process();
var output = string.Empty;
var msbuildProcessFailed = false;
try
var projectFile = FindProjectFileInDirectory(projectLocation);
var properties = new Dictionary<string, string>();
var arguments = new List<string>
{
process.StartInfo = new ProcessStartInfo()
"msbuild",
projectFile,
"-nologo",
$"--getProperty:{string.Join(',', propertyNames)}"
};

var process = new Process
{
StartInfo = new ProcessStartInfo
{
FileName = "dotnet",
Arguments = $"msbuild {projectLocation} -getProperty:OutputType",
Arguments = string.Join(" ", arguments),
RedirectStandardOutput = true,
RedirectStandardError = true,
UseShellExecute = false,
WindowStyle = ProcessWindowStyle.Hidden
};

CreateNoWindow = true
}
};
try
{
process.Start();
output = process.StandardOutput.ReadToEnd();
var hasExited = process.WaitForExit(5000);
string output = process.StandardOutput.ReadToEnd();
string error = process.StandardError.ReadToEnd();
process.WaitForExit(5000);

// If it hasn't completed in the specified timeout, stop the process and give up
if (!hasExited)
if (process.ExitCode == 0)
{
process.Kill();
msbuildProcessFailed = true;
}
using JsonDocument doc = JsonDocument.Parse(output);
Copy link
Member

Choose a reason for hiding this comment

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

I might be missing something but if propertyNames is a single value isn't the output of the command just the value and not the JSON document?

Copy link
Contributor Author

@gcbeattyAWS gcbeattyAWS Nov 20, 2024

Choose a reason for hiding this comment

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

PS C:\dev\repos\aws-extensions-for-dotnet-cli> dotnet msbuild .\testapps\TestFunction\TestFunction.csproj  --getProperty:TargetFramework,TargetFrameworks
{
  "Properties": {
    "TargetFramework": "net6.0",
    "TargetFrameworks": ""
  }
}

is what the commmand returns, so it is a json document

Copy link
Contributor

Choose a reason for hiding this comment

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

We need to check if the count of propertyNames is 1 or greater since the result varies. If you run:

dotnet msbuild --nologo --getProperty:TargetFramework

The result will be

net8.0

If you run

dotnet msbuild --nologo --getProperty:TargetFramework,TargetFrameworks

The result will be:

{
  "Properties": {
    "TargetFramework": "net8.0",
    "TargetFrameworks": ""
  }
}

You need to handle both cases

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ah yes you are right. i will update that!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated to handle case for when only one property is provided. Thanks for catching this!

JsonElement root = doc.RootElement;
JsonElement propertiesElement = root.GetProperty("Properties");

// If it has completed but unsuccessfully, give up
if (process.ExitCode != 0)
foreach (var property in propertyNames)
{
if (propertiesElement.TryGetProperty(property, out JsonElement propertyValue))
{
properties[property] = propertyValue.GetString();
}
}
}
else
{
msbuildProcessFailed = true;
Console.WriteLine($"MSBuild process exited with code {process.ExitCode}. Error: {error}");
Copy link
Member

Choose a reason for hiding this comment

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

No Console.WriteLine calls are used. This same code is used inside Visual Studio and there is no console there. If you want to log something you need to pass in an instance if IToolLogger to handle logging.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not too sure we should be logging anything visible to the user here. We are exposing an implementation detail that would be more confusing if anything.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i just removed the logging all together

// Fallback to XML parsing
properties = LookupProjectPropertiesFromXml(projectFile, propertyNames);
}
}
catch (Exception)
catch (Exception ex)
{
// swallow any exceptions related to `dotnet msbuild`
msbuildProcessFailed = true;
Console.WriteLine($"Error executing MSBuild or parsing output: {ex.Message}");
Copy link
Member

Choose a reason for hiding this comment

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

No Console.WriteLine calls are used. This same code is used inside Visual Studio and there is no console there. If you want to log something you need to pass in an instance if IToolLogger to handle logging.

Copy link
Contributor

Choose a reason for hiding this comment

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

Same comment about logging

// Fallback to XML parsing
properties = LookupProjectPropertiesFromXml(projectFile, propertyNames);
}

if (msbuildProcessFailed)
return properties;
}

private static Dictionary<string, string> LookupProjectPropertiesFromXml(string projectFile, string[] propertyNames)
{
var properties = new Dictionary<string, string>();
try
{
var projectFile = FindProjectFileInDirectory(projectLocation);
var xdoc = XDocument.Load(projectFile);
var element = xdoc.XPathSelectElement("//PropertyGroup/OutputType");
output = element?.Value;
foreach (var propertyName in propertyNames)
{
var element = xdoc.XPathSelectElement($"//PropertyGroup/{propertyName}");
if (element != null && !string.IsNullOrWhiteSpace(element.Value))
{
properties[propertyName] = element.Value;
}
}
}
catch (Exception ex)
{
Console.WriteLine($"Error parsing project file XML: {ex.Message}");
Copy link
Member

Choose a reason for hiding this comment

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

No Console.WriteLine calls are used. This same code is used inside Visual Studio and there is no console there. If you want to log something you need to pass in an instance if IToolLogger to handle logging.

Copy link
Contributor

Choose a reason for hiding this comment

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

Same comment about logging

}
return properties;
}

return
string.IsNullOrEmpty(output) ?
null :
output.Trim();
/// <summary>
/// Looks up the target framework from a project file.
/// </summary>
/// <param name="projectLocation">The location of the project file.</param>
/// <returns>The target framework of the project.</returns>
public static string LookupTargetFrameworkFromProjectFile(string projectLocation)
{
var properties = LookupProjectProperties(projectLocation, "TargetFramework", "TargetFrameworks");
if (properties.TryGetValue("TargetFramework", out var targetFramework) && !string.IsNullOrEmpty(targetFramework))
{
return targetFramework;
}
if (properties.TryGetValue("TargetFrameworks", out var targetFrameworks) && !string.IsNullOrEmpty(targetFrameworks))
normj marked this conversation as resolved.
Show resolved Hide resolved
{
var frameworks = targetFrameworks.Split(';');
if (frameworks.Length > 1 ){
return null;
}
return frameworks[0];
}
return null;
}

/// <summary>
/// Retrieve the `OutputType` property of a given project
/// </summary>
/// <param name="projectLocation">Path of the project</param>
/// <returns>The value of the `OutputType` property</returns>
public static string LookupOutputTypeFromProjectFile(string projectLocation)
{
var properties = LookupProjectProperties(projectLocation, "OutputType");
return properties.TryGetValue("OutputType", out var outputType) ? outputType.Trim() : null;
}

public static bool LookPublishAotFlag(string projectLocation, string msBuildParameters)
Expand All @@ -288,43 +346,32 @@ public static bool LookPublishAotFlag(string projectLocation, string msBuildPara
}
}

// If the property wasn't provided in msBuildParameters, fall back to searching project file
var projectFile = FindProjectFileInDirectory(projectLocation);

var xdoc = XDocument.Load(projectFile);

var element = xdoc.XPathSelectElement("//PropertyGroup/PublishAot");

if (bool.TryParse(element?.Value, out bool result))
var properties = LookupProjectProperties(projectLocation, "PublishAot");
if (properties.TryGetValue("PublishAot", out var publishAot))
{
return result;
return bool.TryParse(publishAot, out var result) && result;
}

return false;
}


public static bool HasExplicitSelfContainedFlag(string projectLocation, string msBuildParameters)
{
if (msBuildParameters != null && msBuildParameters.IndexOf("--self-contained", StringComparison.InvariantCultureIgnoreCase) != -1)
{
return true;
}

// If the property wasn't provided in msBuildParameters, fall back to searching project file
var projectFile = FindProjectFileInDirectory(projectLocation);

var xdoc = XDocument.Load(projectFile);

var element = xdoc.XPathSelectElement("//PropertyGroup/SelfContained");

if (bool.TryParse(element?.Value, out _))
var properties = LookupProjectProperties(projectLocation, "SelfContained");
if (properties.TryGetValue("SelfContained", out var selfContained))
{
return true;
return bool.TryParse(selfContained, out _);
normj marked this conversation as resolved.
Show resolved Hide resolved
}

return false;
}

public static string FindProjectFileInDirectory(string directory)
private static string FindProjectFileInDirectory(string directory)
{
if (File.Exists(directory))
return directory;
Expand Down
1 change: 1 addition & 0 deletions test/Amazon.Common.DotNetCli.Tools.Test/UtilitiesTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ public class UtilitiesTests
[InlineData("../../../../../testapps/TestFunction", "net6.0")]
[InlineData("../../../../../testapps/ServerlessWithYamlFunction", "net6.0")]
[InlineData("../../../../../testapps/TestBeanstalkWebApp", "netcoreapp3.1")]
[InlineData("../../../../../testapps/TestFunctionBuildProps/TestFunctionBuildProps", "net6.0")]
public void CheckFramework(string projectPath, string expectedFramework)
{
var assembly = this.GetType().GetTypeInfo().Assembly;
Expand Down
5 changes: 5 additions & 0 deletions testapps/TestFunctionBuildProps/Directory.Build.props
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
<Project>
<PropertyGroup>
<TargetFramework>net6.0</TargetFramework>
</PropertyGroup>
</Project>
14 changes: 14 additions & 0 deletions testapps/TestFunctionBuildProps/TestFunctionBuildProps/Function.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
using Amazon.Lambda.Core;

// Assembly attribute to enable the Lambda function's JSON input to be converted into a .NET class.
[assembly: LambdaSerializer(typeof(Amazon.Lambda.Serialization.SystemTextJson.DefaultLambdaJsonSerializer))]

namespace TestFunctionBuildProps;

public class Function
{
public string FunctionHandler(string input, ILambdaContext context)
{
return input.ToUpper();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<ImplicitUsings>enable</ImplicitUsings>
<Nullable>enable</Nullable>
<GenerateRuntimeConfigurationFiles>true</GenerateRuntimeConfigurationFiles>
<AWSProjectType>Lambda</AWSProjectType>
<!-- This property makes the build directory similar to a publish directory and helps the AWS .NET Lambda Mock Test Tool find project dependencies. -->
<CopyLocalLockFileAssemblies>true</CopyLocalLockFileAssemblies>
<!-- Generate ready to run images during publishing to improve cold start time. -->
<PublishReadyToRun>true</PublishReadyToRun>
</PropertyGroup>
<ItemGroup>
<PackageReference Include="Amazon.Lambda.Core" Version="2.4.0" />
<PackageReference Include="Amazon.Lambda.Serialization.SystemTextJson" Version="2.4.4" />
</ItemGroup>
</Project>
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
{
"Information": [
"This file provides default values for the deployment wizard inside Visual Studio and the AWS Lambda commands added to the .NET Core CLI.",
"To learn more about the Lambda commands with the .NET Core CLI execute the following command at the command line in the project root directory.",
"dotnet lambda help",
"All the command line options for the Lambda command can be specified in this file."
],
"profile": "default",
"region": "us-west-2",
"configuration": "Release",
"function-architecture": "x86_64",
"function-runtime": "dotnet8",
"function-memory-size": 512,
"function-timeout": 30,
"function-handler": "TestFunctionBuildProps::TestFunctionBuildProps.Function::FunctionHandler"
}