Use MSBuild to find target framework #348
+188
−63
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| 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" | ||
| ] | ||
| } | ||
| ] | ||
| } | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -18,6 +18,7 @@ | |
| using System.Text.RegularExpressions; | ||
| using System.Collections; | ||
| using System.Xml; | ||
| using System.Text.Json; | ||
|
|
||
| namespace Amazon.Common.DotNetCli.Tools | ||
| { | ||
|
|
@@ -205,72 +206,125 @@ public static string DeterminePublishLocation(string workingDirectory, string pr | |
| return path; | ||
| } | ||
|
|
||
|
|
||
| // <summary> | ||
| /// Looks up specified properties from a project file. | ||
|
There was a problem hiding this comment. nit: drop "file" since MSBUILD is not looking at the project file (.csproj) alone |
||
| /// </summary> | ||
| /// <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 projectFile = FindProjectFileInDirectory(projectLocation); | ||
| var properties = new Dictionary<string, string>(); | ||
| var arguments = new List<string> | ||
| { | ||
| "msbuild", | ||
| projectFile, | ||
| "-nologo", | ||
| $"--getProperty:{string.Join(',', propertyNames)}" | ||
| }; | ||
|
|
||
| var process = new Process | ||
| { | ||
| StartInfo = new ProcessStartInfo | ||
| { | ||
| FileName = "dotnet", | ||
| Arguments = string.Join(" ", arguments), | ||
| RedirectStandardOutput = true, | ||
| RedirectStandardError = true, | ||
| UseShellExecute = false, | ||
| CreateNoWindow = true | ||
| } | ||
| }; | ||
| try | ||
| { | ||
| process.Start(); | ||
| string output = process.StandardOutput.ReadToEnd(); | ||
| string error = process.StandardError.ReadToEnd(); | ||
| process.WaitForExit(); | ||
|
There was a problem hiding this comment. Should we keep the 5 second timeout or at least some timeout? I'm thinking of @philasmar recent PR that at least added There was a problem hiding this comment. sounds good i will update to 5 seconds |
||
|
|
||
| if (process.ExitCode == 0) | ||
| { | ||
| using JsonDocument doc = JsonDocument.Parse(output); | ||
|
There was a problem hiding this comment. 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? There was a problem hiding this comment. is what the commmand returns, so it is a json document There was a problem hiding this comment. We need to check if the count of propertyNames is 1 or greater since the result varies. If you run: The result will be If you run The result will be: You need to handle both cases There was a problem hiding this comment. ah yes you are right. i will update that! There was a problem hiding this comment. updated to handle case for when only one property is provided. Thanks for catching this! |
||
| JsonElement root = doc.RootElement; | ||
| JsonElement propertiesElement = root.GetProperty("Properties"); | ||
|
|
||
| foreach (var property in propertyNames) | ||
| { | ||
| if (propertiesElement.TryGetProperty(property, out JsonElement propertyValue)) | ||
| { | ||
| properties[property] = propertyValue.GetString(); | ||
| } | ||
| } | ||
| } | ||
| else | ||
| { | ||
| Console.WriteLine($"MSBuild process exited with code {process.ExitCode}. Error: {error}"); | ||
|
There was a problem hiding this comment. No There was a problem hiding this comment. 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. There was a problem hiding this comment. i just removed the logging all together |
||
| // Fallback to XML parsing | ||
| properties = LookupProjectPropertiesFromXml(projectFile, propertyNames); | ||
| } | ||
| } | ||
| catch (Exception ex) | ||
| { | ||
| Console.WriteLine($"Error executing MSBuild or parsing output: {ex.Message}"); | ||
|
There was a problem hiding this comment. No There was a problem hiding this comment. Same comment about logging |
||
| // Fallback to XML parsing | ||
| properties = LookupProjectPropertiesFromXml(projectFile, propertyNames); | ||
| } | ||
|
|
||
| return properties; | ||
| } | ||
|
|
||
| private static Dictionary<string, string> LookupProjectPropertiesFromXml(string projectFile, string[] propertyNames) | ||
| { | ||
| var properties = new Dictionary<string, string>(); | ||
| try | ||
| { | ||
| var xdoc = XDocument.Load(projectFile); | ||
| 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}"); | ||
|
There was a problem hiding this comment. No There was a problem hiding this comment. Same comment about logging |
||
| } | ||
| return properties; | ||
| } | ||
|
|
||
| /// <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
|
||
| { | ||
| return targetFrameworks.Split(';')[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) | ||
|
|
@@ -288,43 +342,32 @@ public static bool LookPublishAotFlag(string projectLocation, string msBuildPara | |
| } | ||
| } | ||
|
|
||
| var properties = LookupProjectProperties(projectLocation, "PublishAot"); | ||
| if (properties.TryGetValue("PublishAot", out var publishAot)) | ||
| { | ||
| 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; | ||
| } | ||
|
|
||
| var properties = LookupProjectProperties(projectLocation, "SelfContained"); | ||
| if (properties.TryGetValue("SelfContained", out var selfContained)) | ||
| { | ||
| return bool.TryParse(selfContained, out _); | ||
normj marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| } | ||
|
|
||
| return false; | ||
| } | ||
|
|
||
| private static string FindProjectFileInDirectory(string directory) | ||
| { | ||
| if (File.Exists(directory)) | ||
| return directory; | ||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| <Project> | ||
| <PropertyGroup> | ||
| <TargetFramework>net6.0</TargetFramework> | ||
| </PropertyGroup> | ||
| </Project> |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| 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(); | ||
| } | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| 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> |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| 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" | ||
| } |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
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.