Add Amazon.Lambda.RuntimeSupport dll to the output nuget package #1897

ghost · 2024-12-05T18:23:59Z

Issue #, if available:

Description of changes:

Add all runtime versions of Amazon.Lambda.RuntimeSupport dll to the output nuget package. each dll is in its own folder in the content folder
Add test to verify dlsl are inside the content folder.

Testing

I copied the outputted dlls into the lampda poc project and verified that they work

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

ghost · 2024-12-05T18:24:27Z

Tools/LambdaTestTool-v2/src/Amazon.Lambda.TestTool/Amazon.Lambda.TestTool.csproj

+  </ItemGroup>
+
+  <ItemGroup>
+	  <None Include="$(OutputPath)Amazon.Lambda.RuntimeSupport.dll" Pack="true" PackagePath="content" Visible="false" />


wasnt sure if this needed to be content or lib folder. was seeing this warning when doing pack command

The assembly 'content\Amazon.Lambda.RuntimeSupport.dll' is not inside the 'lib' folder and hence it won't be added as a reference when the package is installed into a project. Move it into the 'lib' folder if it needs to be referenced.

but in our case in we are referencing the dll as part of a command line argument in the aspire proof of concept , so not sure if it really matters

I'm assuming this is only adding the current target framework (net8.0) version of RuntimeSupport. I believe we need to add all targets of Runtimesupport. @normj is my assumption correct?

i remember he mentioned when he updated the test tool v2 something about to not worry about the other versions. but ill let him confirm

We will need to distribute each target framework for Lambda RuntimeSupport. That is how things work in Lambda. I think what you are thinking about is I meant with test tool v2 we only need to ship one version of the test tool NuGet package unlike the current v1 version were we ship separate versions of the package for each target framework. In the end we will have a single V2 test tool NuGet package with one .NET tool which is the Lambda emulator and then separate content folders for each version of RuntimeSupport with RuntimeSupport's dependencies.

Ahh ok let me make that change. thanks for the detailed explanation

ive updated it now, where each runtime support target version has its own folder with a dll in the content folder of the test tool

ive also updated the screenshot in the description

I've updated the latest logic to now use dotnet publish and copy everything in the publish folder to the content folder. and i have updated the screenshot in the description

Tools/LambdaTestTool-v2/tests/Amazon.Lambda.TestTool.UnitTests/PackagingTests.cs

Tools/LambdaTestTool-v2/src/Amazon.Lambda.TestTool/Amazon.Lambda.TestTool.csproj

philasmar

I'm assuming this is only adding the current target framework (net8.0) version of RuntimeSupport. I believe we need to add all targets of Runtimesupport. @normj is my assumption correct?

ghost · 2025-01-14T20:12:41Z

Tools/LambdaTestTool-v2/src/Amazon.Lambda.TestTool/Amazon.Lambda.TestTool.csproj

+  </ItemGroup>
+
+  <ItemGroup>
+    <RuntimeSupportFramework Include="net5.0;net6.0;net8.0;netstandard2.0" />


we havent updated runtime support in this branch to add .net9.0 which is why i havent added it here

Tools/LambdaTestTool-v2/src/Amazon.Lambda.TestTool/Amazon.Lambda.TestTool.csproj

normj

Instead of adding the project reference and manually copying the dll I wonder if we should have a customer target that runs before the Pack target and run some script that does a dotnet publish for each supported target. And then copy the output publish folder to a content folder in the test package for each target framework. That would cover all of the dependencies and would be the same as who the Runtime assembly and its dependencies are copied into the image.

Plus having the project dependency even though we don't use it is annoying because it means we have to keep building RuntimeSupport and its dependencies as part of our dev loop.

ghost · 2025-01-15T17:25:54Z

Instead of adding the project reference and manually copying the dll I wonder if we should have a customer target that runs before the Pack target and run some script that does a dotnet publish for each supported target. And then copy the output publish folder to a content folder in the test package for each target framework. That would cover all of the dependencies and would be the same as who the Runtime assembly and its dependencies are copied into the image.

Plus having the project dependency even though we don't use it is annoying because it means we have to keep building RuntimeSupport and its dependencies as part of our dev loop.

i have updated the latest logic to do this

philasmar · 2025-01-17T15:58:10Z

Tools/LambdaTestTool-v2/src/Amazon.Lambda.TestTool/Amazon.Lambda.TestTool.csproj

+    <Message Importance="high" Text="Publishing Amazon.Lambda.RuntimeSupport project..." />
+
+    <ItemGroup>
+      <TargetFrameworks Include="netstandard2.0;net5.0;net6.0;net8.0" />


You should drop net5 and add net9

the reason i didnt add net9 in this PR was that i saw in the main branch, adding net9 required a bunch of changes. so i was going to do those separately

ive updated this in my local branch but waiting on https://github.com/aws/aws-lambda-dotnet/pull/1937/files to be merged first

philasmar · 2025-01-17T15:58:48Z

Tools/LambdaTestTool-v2/src/Amazon.Lambda.TestTool/Amazon.Lambda.TestTool.csproj

@@ -23,9 +22,25 @@
    <PackageReference Include="BlazorMonaco" Version="3.2.0" />
  </ItemGroup>

+  <Target Name="CopyRuntimeSupportFiles" BeforeTargets="_GetPackageFiles">
+    <Message Importance="high" Text="Publishing Amazon.Lambda.RuntimeSupport project..." />


nit: message is a bit misleading or non descriptive

yea i iwll remove the message

philasmar · 2025-01-17T15:59:24Z

Tools/LambdaTestTool-v2/tests/Amazon.Lambda.TestTool.UnitTests/PackagingTests.cs

+{
+    private readonly ITestOutputHelper _output;
+    private static readonly string[] ExpectedFrameworks = new[]
+    {


update frameworks here too

Would be great if instead of having this hardcoded list if you read the TargetFrameworks from the Amazon.Lambda.RuntimeSupport project file. That way we don't have to update this every time we add a new supported version.

You can skip .NET Standard 2.0.

normj · 2025-01-24T01:13:49Z

Tools/LambdaTestTool-v2/src/Amazon.Lambda.TestTool/Amazon.Lambda.TestTool.csproj

@@ -1,14 +1,13 @@
-<Project Sdk="Microsoft.NET.Sdk.Web">
-
+<Project Sdk="Microsoft.NET.Sdk.Web">


Because of our unorthodox packaging of the assemblies I'm getting a lot of NU5100 warnings. Can you add <NoWarn>NU5100</NoWarn> to the PropertyGroup.

normj · 2025-01-24T01:19:08Z

Tools/LambdaTestTool-v2/src/Amazon.Lambda.TestTool/Amazon.Lambda.TestTool.csproj

+    <ItemGroup>
+      <None Include="$(MSBuildThisFileDirectory)..\..\..\..\Libraries\src\Amazon.Lambda.RuntimeSupport\bin\$(Configuration)\%(TargetFrameworks.Identity)\publish\**\*.*">
+        <Pack>true</Pack>
+        <PackagePath>content\%(TargetFrameworks.Identity)</PackagePath>


Can you change content\%(TargetFrameworks.Identity) to content\Amazon.Lambda.RuntimeSupport\%(TargetFrameworks.Identity) to give some name spacing to the content here.

normj · 2025-01-24T01:25:11Z

Tools/LambdaTestTool-v2/tests/Amazon.Lambda.TestTool.UnitTests/PackagingTests.cs

+{
+    private readonly ITestOutputHelper _output;
+    private static readonly string[] ExpectedFrameworks = new[]
+    {


Would be great if instead of having this hardcoded list if you read the TargetFrameworks from the Amazon.Lambda.RuntimeSupport project file. That way we don't have to update this every time we add a new supported version.

You can skip .NET Standard 2.0.

normj · 2025-01-24T01:26:15Z

Tools/LambdaTestTool-v2/src/Amazon.Lambda.TestTool/Amazon.Lambda.TestTool.csproj

+    <Exec Command="dotnet publish &quot;$(MSBuildThisFileDirectory)..\..\..\..\Libraries\src\Amazon.Lambda.RuntimeSupport\Amazon.Lambda.RuntimeSupport.csproj&quot; -c $(Configuration) -f %(TargetFrameworks.Identity) /p:ExecutableOutputType=true -v n" />
+
+    <ItemGroup>
+      <None Include="$(MSBuildThisFileDirectory)..\..\..\..\Libraries\src\Amazon.Lambda.RuntimeSupport\bin\$(Configuration)\%(TargetFrameworks.Identity)\publish\**\*.*">


It would be great if we could skip netstandard2.0 because it doesn't make any sense in this context. The netstandard2.0 target is for class libraries not executables.

ive updated the logic in the csproj and the unit test to dynamically get the targetframeworks and also skip netstandard2.0

normj

Nice job. Some impressive MSBuild work here.

ghost commented Dec 5, 2024

View reviewed changes

Tools/LambdaTestTool-v2/tests/Amazon.Lambda.TestTool.UnitTests/PackagingTests.cs Show resolved Hide resolved

ghost commented Dec 5, 2024

View reviewed changes

Tools/LambdaTestTool-v2/src/Amazon.Lambda.TestTool/Amazon.Lambda.TestTool.csproj Outdated Show resolved Hide resolved

ghost changed the title ~~add packaging of runtime~~ Add Amazon.Lambda.RuntimeSupport dll to the output nuget package Dec 17, 2024

ghost marked this pull request as ready for review December 17, 2024 18:35

ghost requested a review from philasmar December 17, 2024 18:35

philasmar reviewed Dec 17, 2024

View reviewed changes

ghost force-pushed the gcbeatty/runtime branch 2 times, most recently from f8aafa1 to 13a74ae Compare January 14, 2025 20:06

ghost commented Jan 14, 2025

View reviewed changes

ghost requested review from philasmar and normj January 14, 2025 20:13

ghost commented Jan 14, 2025

View reviewed changes

Tools/LambdaTestTool-v2/src/Amazon.Lambda.TestTool/Amazon.Lambda.TestTool.csproj Outdated Show resolved Hide resolved

normj reviewed Jan 14, 2025

View reviewed changes

ghost requested a review from normj January 15, 2025 17:26

ghost force-pushed the gcbeatty/runtime branch from 16f879a to 22180b8 Compare January 15, 2025 18:51

philasmar requested changes Jan 17, 2025

View reviewed changes

ghost force-pushed the gcbeatty/runtime branch from 22180b8 to 616d6b2 Compare January 22, 2025 19:21

ghost requested a review from philasmar January 22, 2025 19:25

normj requested changes Jan 24, 2025

View reviewed changes

ghost added the Release Not Needed Add this label if a PR does not need to be released. label Jan 24, 2025

ghost requested a review from normj January 24, 2025 16:25

normj approved these changes Jan 24, 2025

View reviewed changes

philasmar approved these changes Jan 27, 2025

View reviewed changes

add packaging of runtime

1366b57

ghost force-pushed the gcbeatty/runtime branch from 3187134 to 1366b57 Compare January 27, 2025 16:59

ghost merged commit 6c50e10 into feature/lambdatesttool-v2 Jan 27, 2025
5 checks passed

ghost deleted the gcbeatty/runtime branch January 27, 2025 17:17

This pull request was closed.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Amazon.Lambda.RuntimeSupport dll to the output nuget package #1897

Add Amazon.Lambda.RuntimeSupport dll to the output nuget package #1897

ghost commented Dec 5, 2024 •

edited by ghost

Loading

ghost Dec 5, 2024

philasmar Dec 17, 2024

ghost Dec 17, 2024

normj Jan 14, 2025

ghost Jan 14, 2025

ghost Jan 14, 2025

ghost Jan 14, 2025

ghost Jan 15, 2025

philasmar left a comment

ghost Jan 14, 2025

normj left a comment •

edited

Loading

ghost commented Jan 15, 2025

philasmar Jan 17, 2025

ghost Jan 17, 2025

ghost Jan 22, 2025

ghost Jan 22, 2025

philasmar Jan 17, 2025

ghost Jan 17, 2025

ghost Jan 22, 2025

philasmar Jan 17, 2025

ghost Jan 22, 2025

normj Jan 24, 2025

normj Jan 24, 2025

ghost Jan 24, 2025

normj Jan 24, 2025

ghost Jan 24, 2025

normj Jan 24, 2025

normj Jan 24, 2025

ghost Jan 24, 2025

normj left a comment

		@@ -1,14 +1,13 @@
		<Project Sdk="Microsoft.NET.Sdk.Web">

		<Project Sdk="Microsoft.NET.Sdk.Web">

Add Amazon.Lambda.RuntimeSupport dll to the output nuget package #1897

Add Amazon.Lambda.RuntimeSupport dll to the output nuget package #1897

Conversation

ghost commented Dec 5, 2024 • edited by ghost Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

philasmar left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

normj left a comment • edited Loading

Choose a reason for hiding this comment

ghost commented Jan 15, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

normj left a comment

Choose a reason for hiding this comment

ghost commented Dec 5, 2024 •

edited by ghost

Loading

normj left a comment •

edited

Loading