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

Conversation

normj
Copy link
Member

@normj normj commented Nov 30, 2024

Issue #, if available:
#351

Description of changes:
In the recent release of Amazon.Lambda.Tools we switched to using dotnet msbuild getproperty command to evaluate MSBuild properties like PublishAot. It is possible for users to specify additional MSBuild parameters via the --msbuild-parameters switch that might affect the evaluation. That is the case of the linked issue.

This PR pass along the user specified value for --msbuild-parameters to the dotnet msbuild getproperty command so the evaluation has the full context for evaluation.

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

…arameters include user specified msbuild parameters.
@normj normj force-pushed the normj/use-msbuild-parameters branch from 47d76ee to cac10d2 Compare November 30, 2024 00:08
@@ -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">Additonal MSBuild paramteres passed by the user from the commandline</param>
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
/// <param name="msBuildParameters">Additonal MSBuild paramteres passed by the user from the commandline</param>
/// <param name="msBuildParameters">Additional MSBuild parameters passed by the user from the commandline</param>

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks fixed

[InlineData("TargetFramework", "", "net6.0")]
[InlineData("TargetFramework", "/p:NonExistence=net20.0", "net6.0")]
[InlineData("TargetFramework", "/p:TargetFramework=net20.0", "net20.0")]
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.

@normj normj changed the base branch from master to dev December 16, 2024 17:28
@normj normj merged commit 5c1e258 into dev Dec 16, 2024
2 checks passed
@normj normj deleted the normj/use-msbuild-parameters branch December 16, 2024 17:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants